1

I have following piece of sql, whilst it is functioning as expected, it is a bit slow to return results (by slow I'm talking about 10 seconds to return 1000 results from a months date range). Is it possible for it to be made more efficient and/or quicker? Just to add these are the following indexes I have on the tables:-

  • RecordID - Primary Key Unique Clustered
  • Department - Non clustered
  • Direction - Non clustered
  • LocalCallGroup - Non clustered
  • ServiceProvider - Non clustered
  • StartTime- Non clustered
  • UserID- Non clustered
  • UserLocalStartTime - Non clustered
  • UserLocalTimeOffset - Non clustered
  • UserNumber - Non clustered

Declaration of variables here, removed for post

    SET @TerminatingSQL = '
    SELECT 
        UserNumber, 
        ImageDirection = ''in'', 
        CallingNumber = CASE WHEN callingnumber IN(''Unavailable'',''Unknown'',''+44anonymous@10.81.253.12'',''0anonymous@10.81.253.12'') THEN ''Anonymous'' ELSE callingnumber END,
        CalledNumber,
        StartTime = dateadd(ms,(-datepart(ms,(startTime))),(startTime)),
        AnswerTime = dateadd(ms,(-datepart(ms,(answerTime))),(answerTime)),
        ReleaseTime =  dateadd(ms,(-datepart(ms,(releaseTime))),(releaseTime)),         
        CallDuration =  dateadd(ms,(-datepart(ms,(releaseTime))),(releaseTime)) - dateadd(ms,(-datepart(ms,(answerTime))),(answerTime)),
        TotalDuration = dateadd(ms,(-datepart(ms,(releaseTime))),(releaseTime)) - dateadd(ms,(-datepart(ms,(startTime))),(startTime)),
        terminationCause,
        recordID
    FROM
        dbo.TABLEA' + @Table +'
    WHERE
        serviceProvider IN ( SELECT serviceProvider FROM ccNumbers WHERE CRMID = ' + CONVERT(VARCHAR(10),@CrmId,103) + ')
        AND startTime between ''' + CONVERT(VARCHAR(10),@Fromdate,112) + ''' AND ''' + CONVERT(VARCHAR(10), @ToDate, 112) + '''
        AND Direction = ''terminating''
        AND (Department = ''' + @Department + ''' OR ''' + @Department + ''' = ''ALL'')
        AND (userid = ''' + @Userid + ''' OR ''' + @Userid + ''' = ''ALL'')'

    SET @OriginatingSQL = '
    SELECT 
        UserNumber, 
        ImageDirection = ''out'', 
        CallingNumber = CASE WHEN callingnumber IN(''Unavailable'',''Unknown'',''+44anonymous@10.81.253.12'',''0anonymous@10.81.253.12'') THEN ''Anonymous'' ELSE callingnumber END,
        CalledNumber,
        StartTime = dateadd(ms,(-datepart(ms,(startTime))),(startTime)),
        AnswerTime = dateadd(ms,(-datepart(ms,(answerTime))),(answerTime)),
        ReleaseTime =  dateadd(ms,(-datepart(ms,(releaseTime))),(releaseTime)),         
        CallDuration =  dateadd(ms,(-datepart(ms,(releaseTime))),(releaseTime)) - dateadd(ms,(-datepart(ms,(answerTime))),(answerTime)),
        TotalDuration = dateadd(ms,(-datepart(ms,(releaseTime))),(releaseTime)) - dateadd(ms,(-datepart(ms,(startTime))),(startTime)),
        terminationCause,
        recordID
    FROM
        dbo.TABLEA' + @Table +'
    WHERE
        serviceProvider IN ( SELECT serviceProvider FROM ccNumbers WHERE CRMID = ' + CONVERT(VARCHAR(10),@CrmId,103) + ')
        AND startTime between ''' + CONVERT(VARCHAR(10),@Fromdate,112) + ''' AND ''' + CONVERT(VARCHAR(10), @ToDate, 112) + '''
        AND Direction = ''originating''
        AND (Department = ''' + @Department + ''' OR ''' + @Department + ''' = ''ALL'')
        AND (userid = ''' + @Userid + ''' OR ''' + @Userid + ''' = ''ALL'')'

    SET @MainSelectSQL = @TerminatingSQL + ' Union '  + @OriginatingSQL

    SET @MainSQL = 'SELECT TOP (' + @PageSize + ') 
        [t1].CalledNumber,
        [t1].CallingNumber,
        [t1].UserNumber, 
        [t1].StartTime,
        [t1].AnswerTime,
        [t1].ReleaseTime,
        [t1].ImageDirection,
        [t1].CallDuration,
        [t1].TotalDuration,
        [t1].TerminationCause
    FROM (
        SELECT ROW_NUMBER() OVER (
            ORDER BY [t0].startTime) as [row_number], 
            [t0].CalledNumber,
            [t0].CallingNumber,
            [t0].UserNumber,
            [t0].StartTime,
            [t0].AnswerTime,
            [t0].ReleaseTime,
            [t0].ImageDirection,
            [t0].CallDuration,
            [t0].TotalDuration,
            [t0].TerminationCause
        FROM
            (' + @MainSelectSQL + ')  AS [t0] 
          )  AS [t1]

    WHERE [t1].[row_number] > ' + @Page + ' * ' + @PageSize +';'

    EXEC (@MainSQL)

-- Work out the total number of rows, but don't bother if we have the number already (i.e. when they keep the same parameters and just click paging.

IF  (@CurrentCount IS NULL)
    BEGIN
        DECLARE @TotalCountSQL nvarchar(4000)
        DECLARE @ParameterList NVARCHAR(4000)   

        SET @ParameterList = '@TotalCount int OUTPUT'
        SET @TotalCountSQL = 'SELECT @TotalCount = COUNT(recordId) FROM (' + @MainSelectSQL + ') as a'

        EXEC SP_EXECUTESQL @TotalCountSQL,@ParameterList,@TotalCount=@TotalCount OUTPUT
    END
ELSE
    BEGIN
        SET @TotalCount = @CurrentCount;
    END
END
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Vince Ashby-Smith
  • 1,152
  • 4
  • 18
  • 36

2 Answers2

4

Things to improve

  • UNION ALL will remove an implied DISTINCT. The ImageDirection column ensures that there is no overlap anyway, so UNION adds an extra step in the plan

  • Where you have ROW_NUMBER(), add COUNT(*) OVER () to get the total record count. This removes the need for the 2nd call

Thoughts:

  • Do you have dynamic table names that requires such ugly concatenation?
    Except for that, I see no need for dynamic SQL

  • Consider using a temp table to stage results to simplify complexity

gbn
  • 422,506
  • 82
  • 585
  • 676
  • Yes unfortunately i have dynamic table names based on month and year. So for example it is in the format of Table_2011_11 – Vince Ashby-Smith Nov 28 '11 at 10:09
  • 5
    @Vince Ashby-Smith: that's why RDBMS have partitioning so you don't end up in this mess... – gbn Nov 28 '11 at 10:10
  • Yeah i looked into partitioning however we dont have the correct version of sql to allow partitioning and we're not in the position to buy the version :( – Vince Ashby-Smith Nov 28 '11 at 10:13
  • Any chance you could explain the count(*) over() suggestion, dont really understand? – Vince Ashby-Smith Nov 28 '11 at 10:18
  • The OVER clause changes the scope of the count to "entire record set". It also replaces the need for a GROUP BY clause. So consider it as a "in-line aggregate". See http://stackoverflow.com/q/6218902/27535 for more – gbn Nov 28 '11 at 10:21
  • The Union shaved off about 2 seconds but the count(*)over() actually added a few more seconds as i only need to get the count once, then i'm storing that in .net. I'm going to try some temporary tables, see if that improves it. I'm hoping a can shave off another few seconds somewhere! – Vince Ashby-Smith Nov 28 '11 at 13:19
0

If you only need dynamic SQL because of the variable table names, consider only using dynamic SQL to pull the data you need into a temp table, and then write all your SQL as regular SQL (non-dynamic) using the temp table as the data source.

DECLARE @Table_Name VARCHAR(255) = 'Table_2011_11'

CREATE TABLE #temp (
    Number INT,
    Name VARCHAR(64)
)

DECLARE @sql VARCHAR(512) = 'INSERT INTO #temp(Number) SELECT UserNumber FROM ' + @Table_Name
exec sp_sqlexec @sql

select * from #temp
simon
  • 3,380
  • 2
  • 22
  • 23