SQL Taking too long.

I realise some of you guys are well averse to Dynamic SQL and in this instance I wholly agree however the legacy of the design of this application dictates its use. The SQL procedure below is inherited and not my design. I need to speed it up from its current execution time of 25 seconds as it's err supposed to be a real time display. By the way, Query Execution plan denotes 78% table scan in proc bit.

I'm in the middle of re-structuring it myself but fear all I'm doing is tidying. Any suggestions, given that I'm unable to change the table architecture would be gratefully received.

Cheers.

Proc:

Code:

---


CREATE PROCEDURE sd_rtd_cn  AS DECLARE @Trans VARCHAR(20) DECLARE @CampaignName VARCHAR(40) DECLARE @active BIT DECLARE @sqlquery VARCHAR(4000)
SELECT @Trans = 'ArchiveTransaction'
BEGIN TRANSACTION @Trans
DELETE FROM sd_rtd_campaigns
DECLARE campaignName_Cursor CURSOR FOR SELECT campaignName, active FROM campaigns
OPEN campaignName_Cursor
FETCH NEXT FROM campaignName_Cursor INTO  @CampaignName, @active
WHILE @@FETCH_STATUS = 0
BEGIN
        SET @sqlquery = ' DECLARE @fromRetry decimal(9,4) DECLARE @toRetry decimal(9,4) DECLARE @nowRetry decimal(9,4)' +
        ' DECLARE @a INT DECLARE @b INT DECLARE @c INT DECLARE @d INT' +
        ' DECLARE @e INT DECLARE @f INT DECLARE @g INT DECLARE @h INT' +
        ' DECLARE @i INT DECLARE @j INT DECLARE @k INT' +
        ' DECLARE @DPNOW varchar(10) DECLARE @DPWRITE varchar(10) DECLARE @cToday INT' +
        ' SELECT @DPNOW = cast(datepart(yyyy,getdate()) as varchar) + cast(datepart(mm,getdate()) as varchar) + cast(datepart(dd,getdate()) as varchar)' +
        ' SELECT @k = count(recID) FROM [' + @CampaignName + '] WHERE retryTime = 70000.3 AND status=0' +
        ' SELECT @a = COUNT(recID) FROM [' + @CampaignName + ']  WHERE status=0' +
        ' SELECT @b = count(recID) FROM [' + @CampaignName + '] WHERE status=1' +
        ' SELECT @c = count(recID) FROM [' + @CampaignName + '] WHERE status=2' +
        ' SELECT @cToday = count(recID) FROM [' + @CampaignName + '] WHERE status=2  AND  cast(datepart(yyyy,WriteTime) as varchar) + cast(datepart(mm,WriteTime) as varchar) + cast(datepart(dd,WriteTime) as varchar) = @DPNOW ' +
        ' SET @nowRetry = CONVERT( decimal(9,4),getDate(),2) + 2' +
        ' SET @toRetry = @nowRetry + (0.5/24.0) SET @fromRetry = @nowRetry ' +
        ' SELECT @d = count(recID) FROM [' + @CampaignName + '] WHERE (retryTime > @fromRetry) AND  (retryTime <= @toRetry) AND (ISNULL(agentID,'''')='''') AND status=0' +
        ' SET @toRetry = @nowRetry + (1.0/24.0) SET @fromRetry = @nowRetry + (0.5/24.0) ' +
        ' SELECT @e = count(recID) FROM [' + @CampaignName + '] WHERE (retryTime > @fromRetry) AND  (retryTime <= @toRetry) AND (ISNULL(agentID,'''')='''') AND status=0' +
        ' SET @toRetry = @nowRetry + (2.0/24.0) SET @fromRetry = @nowRetry + (1.0/24.0) ' +
        ' SELECT @f = count(recID) FROM [' + @CampaignName + '] WHERE (retryTime > @fromRetry) AND  (retryTime <= @toRetry) AND (ISNULL(agentID,'''')='''') AND status=0' +
        ' SET @toRetry = @nowRetry + (3.0/24.0) SET @fromRetry = @nowRetry + (2.0/24.0) ' +
        ' SELECT @g = count(recID) FROM [' + @CampaignName + '] WHERE (retryTime > @fromRetry) AND  (retryTime <= @toRetry) AND (ISNULL(agentID,'''')='''') AND status=0' +
        ' SET @toRetry = @nowRetry + (4.0/24.0) SET @fromRetry = @nowRetry + (3.0/24.0) ' +
        ' SELECT @h = count(recID) FROM [' + @CampaignName + '] WHERE (retryTime > @fromRetry) AND  (retryTime <= @toRetry) AND (ISNULL(agentID,'''')='''') AND status=0' +
        ' SET @fromRetry = @nowRetry + (4.0/24.0) ' +
        ' SELECT @i = count(recID) FROM [' + @CampaignName + '] WHERE  (retryTime > @fromRetry) AND (retryTime < 70000.3) AND (ISNULL(agentID,'''')='''') AND status=0' +
        ' SET @toRetry = @nowRetry' +
        ' SELECT @j = count(recID) FROM [' + @CampaignName + '] WHERE retryTime <= @toRetry AND (ISNULL(agentID,'''')='''') AND status=0' +
        ' INSERT INTO sd_rtd_campaigns VALUES(''' + @CampaignName + ''',@c,@cToday,@b,@a,@k,@j,@d,@e,@f,@g,@h,@i,' + CAST(@active AS VARCHAR(16)) + ')'
       

Exec (@sqlQuery)
    FETCH NEXT FROM campaignName_Cursor INTO @CampaignName, @active
END
CLOSE  campaignName_Cursor
COMMIT TRANSACTION ArchiveTransaction
DEALLOCATE campaignName_Cursor
GO

---

Well - that basically breaks all the rules :)

Sorry to say that - but it does.

CURSOR loop with DYNAMIC SQL is not ever a way to process data.

INSERT INTO @TABLEVARIABLE SELECT SUM(column)... should be the basic technique used here.

That's going to take you fully understanding what the purpose of the CURSOR loop is so that a TABLE VARIABLE can be built in a SINGLE SQL statement.

If that first TABLE VARIABLE only has the "raw" rows, then process it with another INSERT INTO...SELECT... into another table variable - using more SUM and other AGGREGATE functions.

The final result should be a TABLE VARIABLE that can be INSERT SD_RTD_CAMPAIGNS source.

For example this part:

Code:

---

SELECT @k = count(recID) FROM [' + @CampaignName + '] WHERE retryTime = 70000.3 AND status=0' +
        ' SELECT @a = COUNT(recID) FROM [' + @CampaignName + ']  WHERE status=0' +
        ' SELECT @b = count(recID) FROM [' + @CampaignName + '] WHERE status=1' +
        ' SELECT @c = count(recID) FROM [' + @CampaignName + '] WHERE status=2' +

---

Could be replaced by:

Code:

---

Declare @TableCounts Table (CName varchar(100),KCount,ACount,BCount,CCount)

Insert into @TableCounts
  Select 'CampName',Sum(Case When RetryTime=70000.3 and Status=0 Then 1 Else 0 End)
          ,Sum(Case When Status=0 Then 1 Else 0 End)
          ,Sum(Case When Status=1 Then 1 Else 0 End)
          ,Sum(Case When Status=2 Then 1 Else 0 End) From CampName

---

Not sure how many campaign name tables you have - the design is a problem - should really be one table with CampaignName as a key.

szlamany,

Thanks for the suggestions, appreciate your time and effort.
Will get back to you shortly when I've worked your comments thru...

Cheers.

Chub..

Weeeelll, had a bit of time reviewing the SP & your comments szlamany, I'm still not sure what this gains me ? I've still got to execute a cursor (or alternatively construct 1 - n SP's (1 per table). Even placing the data into a temporary table type variable and exec'ing more calculations on it, the definition of the table would merely bring together raw data from each table into 1 table and without being able to further define what should be in the table for each of the campaigns table the data manipulation would be extensive and I'm not quite sure it would be much quicker...

I do fully agree that the SP breaks all the rules but worse is thedesign in the first place ! :rolleyes:

Or am I getting the wrong end of this ?

Cheers

Chub.

Now that we both fully agree that the design stinks and you have nothing but poor choices - where do you go from here...

How many campaign tables are there?

Ach but we agreed at the start.... wasn't my design so happy to call it cack :bigyello: ..... still where to go from here is right..... this code is live in somewhere in the region of 150 companies. there are rarely more than 20 active campaigns but the issue is that the campaign tables are all dynamically created (and no, there are no Primary Keys or even indexes !!), some of these tables can have upwards of 500,000 records in each, hence the table scanning issue.

I need to see a return on this bringing it closer to 5 seconds as opposed to between 25 and 50 seconds......

I have just got agreement to sub-set the data to use only today's data so will begin from there.... whether to stack it all into a temp table is the next part as the Where statement simply compounds the tablescan....

Suggestions ??

Cheers

Chub..

I have to say the quicker you can get the data out of the garbage non-indexed table into something you can work with, such as a temp table or a table variable (they run fast - all memory) - the better.

So I would somehow start the SPROC by doing a one-time load of the data from the campaign table into some working space that is better controlled.

So maybe that first QUERY is a dynamic query - all the rest of the queries that follow are off the temp table/tbl variable.

My suggestion from before took 4 seperate queries on the table - each one a table scan of it's own and made it just one single queries (still a table scan - cannot avoid that!).

Since you defined the real problem here in that 500,000 rows exist in a pretty poorly defined table, the least amount of SELECT's against that table the better.

But all in all, this is going to be a lot of experimentation by you.

Good luck!

Was coming around to the temp table...

:eek2: .....brainwave hits..... :eek2: ...

...... If I put all the data for each campaign into thetemp table then run an insert into sd_rtd_campaigns for each campaign 9simply to put in the name then run an update SQL for each of the calc values using a where sd_rtd_campaigns.Campaignname = temptable.campaignname and then pull from that I get my results......well...thats the sort-of-theory.........

Thanks for your time & input. :thumb:

Chub..

Note that my example table variable had the campaign name as the first column - the idea of that was so that you could populate this working structure with more than one campaign and have a more or less single path to follow using this table variable structure in subsequent queries...

At least that was the idea - once again - good luck!