Speeding up a stored procedure

Is there any way to optimize this stored procedure? Maybe something instead of so many joins? It takes some time to execute. Maybe there are other options that I could look into?

```
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO

ALTER PROCEDURE [stimulator].[GetLastMessages2]
@serviceId bigint,
@stimulatorId int,
@from datetime,
@to datetime,
@atLeast int = null,
@lessThan int = null,
@lastHourLessThan int = null,
@hourFactor int = 24,
@lastXHourLessThan int = null,
@totalLessThan int = null
AS
BEGIN

SET NOCOUNT ON;

DECLARE @H1Ago datetime
DECLARE @HXAgo datetime
DECLARE @Now datetime
DECLARE @Today datetime

SET @H1Ago = DATEADD(hh, -1, getdate())
SET @HXAgo = DATEADD(hh, -@hourFactor, getdate())
SET @Now = getdate()
SET @Today = DATEADD(dd, 0, DATEDIFF(dd, 0, @Now))

SELECT tel_telewidz,
acn_id,
guid,
[First],
[Last],
[inCount],
isNULL(lastHourCount, 0) lastHourCount,
isNULL(lastXCount, 0) lastXCount,
isNULL(allCount, 0) allCount,
lastStimulation,
body,
STUFF((SELECT '|' + body
FROM SP_BILLING_DW.hurt.MESSAGE_In inmsg
WHERE inmsg.tel_telewidz = d.tel_telewidz
AND service_id = @serviceId
AND creation_time > @from
AND creation_time @from
AND
creation_time @H1Ago
GROUP BY msisdn
) lh ON d.tel_telewidz = lh.msisdn
LEFT JOIN
(
SELECT msisdn, count(*) lastXCount
FROM stimulator.SentMessages
WHERE serviceId = @serviceId
AND stimulatorId = @stimulatorId

A couple of things to think about while writing your query....

Variable Naming

ALTER PROCEDURE [stimulator].[GetLastMessages2]
    @serviceId bigint,
    @stimulatorId int,
    @from datetime,
    @to datetime,
    @atLeast int = null,
    @lessThan int = null,
    @lastHourLessThan int = null,
    @hourFactor int = 24,
    @lastXHourLessThan int = null,
    @totalLessThan int = null
AS
BEGIN

    SET NOCOUNT ON;

    DECLARE @H1Ago datetime
    DECLARE @HXAgo datetime
    DECLARE @Now datetime
    DECLARE @Today datetime

    SET @H1Ago = DATEADD(hh, -1, getdate())
    SET @HXAgo = DATEADD(hh, -@hourFactor, getdate())
    SET @Now = getdate()
    SET @Today = DATEADD(dd, 0, DATEDIFF(dd, 0, @Now))

What are these variables?

• @from

• @to

• @atLeast

• @lessThan

• @lastHourLessThan

• ...

What is their purpose? you should see if you can find more descriptive names for these, don't try to make them as short as possible because that makes the code obscure.

Extraneous Variables

Also, get rid of @Now you are not really using it for anything, instead write @Today like this

DECLARE @Today DATETIME
SET @Today = DATEADD(dd, 0, DATEDIFF(dd, 0, GETDATE())

Formatting

I noticed that not all the reserved keywords were capitalized, Be Consistent with formatting otherwise you will get lost in your code.

I am talking about...

-
The type on your variable declarations

DECLARE @Today datetime

should be

DECLARE @Today DATETIME

-
Database functions

SET @Now = getdate()

should be

SET @Now = GETDATE()

I only bring this up because of your inconsistent use of CAPS

List of keywords from your code that should be in all caps:

• BIGINT

• INT

• DATETIME

• NULL

• GETDATE()

• ISNULL()

• COUNT()

• MAX()

You also have a spot where the code is not indented, I am not sure if that is copy paste to CodeReview or not though. On the same piece of code you change your style on the next nested block.

FROM (
    SELECT tel_telewidz, acn_id, guid, creation_time, service_id
        ,ROW_NUMBER() OVER(PARTITION BY tel_telewidz ORDER BY creation_time) [No]
        ,COUNT(*) OVER(PARTITION BY tel_telewidz) [inCount]
        ,MIN(creation_time) OVER(PARTITION BY tel_telewidz) [First]
        ,MAX(creation_time) OVER(PARTITION BY tel_telewidz) [Last]
        ,body
    FROM SP_BILLING_DW.hurt.MESSAGE_In
    WHERE 
        creation_time > @from
    AND
        creation_time  @H1Ago
        GROUP BY msisdn
    ) lh ON d.tel_telewidz = lh.msisdn
    LEFT JOIN 
    ....

this should look more like this:

FROM (
        SELECT tel_telewidz, acn_id, guid, creation_time, service_id
            ,ROW_NUMBER() OVER(PARTITION BY tel_telewidz ORDER BY creation_time) [No]
            ,COUNT(*) OVER(PARTITION BY tel_telewidz) [inCount]
            ,MIN(creation_time) OVER(PARTITION BY tel_telewidz) [First]
            ,MAX(creation_time) OVER(PARTITION BY tel_telewidz) [Last]
            ,body
        FROM SP_BILLING_DW.hurt.MESSAGE_In
        WHERE creation_time > @from
            AND creation_time  @H1Ago
        GROUP BY msisdn
    ) lh ON d.tel_telewidz = lh.msisdn
    LEFT JOIN 
    ...

that first nested select statement was very different from the rest.

Things I might change in the code

I would probably turn the queries inside of your LEFT JOINs into Temporary Tables and select from them inside the joins. It might speed up the query a little bit, but don't quote me on that (unless I am right).

I know that it will make your query a lot cleaner and easier to read, which should always translate into efficiency somewhere in the whole process.

StackExchange Code Review Q#55774, answer score: 11