-2

I have wrote this cursor for commission report. What happens is commission comes in one table, the records are another table. I match two based on certain critera (there is not exact match available). The problem is there are duplicates where records exist. When I match commission with the records table, it can result picking up these duplicates. Thus the rep gets paid more. On the other hand, there are duplicates in commission table also but those are valid beause they simple mean an account got paid for 2 months.

I wrote this query but it takes 5+ minutes to run. I have 50,000 records in records table and 100,000 in commission table. Is there any way I an improve this cursor?

/* just preparation of cursor, this is not time consuming */
CREATE TABLE #result
  (
     repid         INT,
     AccountNo     VARCHAR(100),
     supplier      VARCHAR(15),
     CompanyName   VARCHAR(200),
     StartDate     DATETIME,
     EndDate       DATETIME,
     Product       VARCHAR(25),
     commodity     VARCHAR(25),
     ContractEnd   DATETIME,
     EstUsage      INT,
     EnrollStatus  VARCHAR(10),
     EnrollDate    DATETIME,
     ActualEndDate DATETIME,
     MeterStart    DATETIME,
     MeterEnd      DATETIME,
     ActualUsage   INT
  )

DECLARE @AccountNo VARCHAR(100)
DECLARE @supplier VARCHAR(10)
DECLARE @commodity VARCHAR(15)
DECLARE @meterstart DATETIME
DECLARE @meterEnd DATETIME
DECLARE @volume FLOAT
DECLARE @RepID INT
DECLARE @Month INT
DECLARE @Year INT

SET @repID = 80
SET @Month = 1
SET @year = 2012

/* the actual cursor */
DECLARE commission_cursor CURSOR FOR
  SELECT AccountNo,
         supplier,
         commodity,
         meterStart,
         MeterEnd,
         Volume
  FROM   commission
  WHERE  Datepart(m, PaymentDate) = @Month
         AND Datepart(YYYY, PaymentDate) = @Year

OPEN commission_cursor

FETCH next FROM commission_cursor INTO @AccountNo, @supplier, @commodity, @MeterStart, @MeterEnd, @Volume;

WHILE @@fetch_status = 0
  BEGIN
      IF EXISTS (SELECT id
                 FROM   Records
                 WHERE  AccountNo = @AccountNo
                        AND supplier = @supplier
                        AND Commodity = @commodity
                        AND RepID = @repID)
        INSERT INTO #result
        SELECT TOP 1 RepID,
                     AccountNo,
                     Supplier,
                     CompanyName,
                     [Supplier Start Date],
                     [Supplier End Date],
                     Product,
                     Commodity,
                     [customer end date],
                     [Expected Usage],
                     EnrollStatus,
                     ActualStartDate,
                     ActualEndDate,
                     @meterstart,
                     @MeterEnd,
                     @volume
        FROM   Records
        WHERE  AccountNo = @AccountNo
               AND supplier = @supplier
               AND Commodity = @commodity
               AND RepID = @repID
               AND @MeterStart >= Dateadd(dd, -7, ActualStartDate)
               AND @meterEnd <= Isnull(Dateadd(dd, 30, ActualEndDate), '2015-12-31')

      FETCH next FROM commission_cursor INTO @AccountNo, @supplier, @commodity, @MeterStart, @MeterEnd, @Volume;
  END

SELECT *
FROM   #result

/* clean up */
CLOSE commission_cursor

DEALLOCATE commission_cursor

DROP TABLE #result 

I have read answer to How to make a T-SQL Cursor faster?, for that what I get is rewrite this query in table form. But I do have another query which uses join and is lightening fast. The problem is, it can not differentiate between the dups in my records table.

Is there anything I can do to make is faster. This is primary question. If not, do you have any alternative way to do it.

I specifically need help with

  • Will using Views or store procedure help
  • I there a way I can use cache in Cursor to make it faster
  • Any other option in syntax
10
  • 2
    Have you noticed that the cursor in the linked question was declared with the FAST_FORWARD option?
    – Andriy M
    Jun 15, 2012 at 17:50
  • I did which I dont know what that means. Coz he was using cursor in a cursor which probably does not apply here.
    – TheTechGuy
    Jun 15, 2012 at 17:52
  • 3
    The main problem with cursor performance is when the default options are used (global, dynamic, read/write, scrollable). Please read this article: sqlblog.org/blogs/aaron_bertrand/archive/2012/01/26/… That doesn't mean that adding LOCAL STATIC READ_ONLY FORWARD_ONLY is the only way to improve efficiency here (it is quite likely you can do this without a loop at all), but it's a good start. Jun 15, 2012 at 17:53
  • 2
    I would get rid of the cursor all together and write a query that gives you single record you want for your commision and then use it as a sub query or dump it in a temp table. you should be able to get that many records in seconds, not minutes
    – Limey
    Jun 15, 2012 at 17:55
  • 1
    Also can you explain TOP 1 without ORDER BY? Do you really not care which matching row in Records you get? Jun 15, 2012 at 17:58

2 Answers 2

8

The very first option is to set the least resource intensive options for your cursor:

declare commission_cursor cursor
local static read_only forward_only
for 

Next is to investigate whether you need a cursor at all. In this case I think you can do the same with a single pass and no loops:

;WITH x AS 
(
  SELECT 
    rn = ROW_NUMBER() OVER (PARTITION BY r.AccountNo, r.Supplier, r.Commodity, r.RepID 
      ORDER BY r.ActualEndDate DESC),
    r.RepID, 
    r.AccountNo, 
    r.Supplier, 
    r.CompanyName, 
    StartDate = r.[Supplier Start Date], 
    EndDate = r.[Supplier End Date],
    r.Product, 
    r.Commodity, 
    ContractEnd = r.[customer end date], 
    EstUsage = r.[Expected Usage], 
    r.EnrollStatus, 
    EnrollDate = r.ActualStartDate,
    r.ActualEndDate, 
    c.MeterStart, 
    c.MeterEnd, 
    ActualUsage = c.Volume 
  FROM dbo.commission AS c 
  INNER JOIN dbo.Records AS r
    ON c.AccountNo = r.AccountNo
    AND c.Supplier = r.Supplier
    AND c.Commodity = r.Commodity
    AND c.RepID = r.RepID
  WHERE 
    c.PaymentDate >= DATEADD(MONTH, @Month-1, CONVERT(CHAR(4), @Year) + '0101')
    AND c.PaymentDate < DATEADD(MONTH, 1, CONVERT(CHAR(4), @Year) + '0101')
    AND r.RepID = @RepID
)
SELECT RepID, AccountNo, Supplier, CompanyName, StartDate, EndDate, 
  Product, Commodity, ContractEnd, EstUsage, EnrollStatus, EnrollDate, 
  ActualEndDate, MeterStart, MeterEnd, ActualUsage 
FROM x 
WHERE rn = 1 --ORDER BY something;

If this is still slow, then the cursor probably wasn't the problem - the next step will be investigating what indexes might be implemented to make this query more efficient.

3
  • 1
    +1 Cursors are the devil, there is very high likelihood it can be with a regular sql statement. Jun 15, 2012 at 18:05
  • This query runs fast, almost instantly but returns 9,000 records where it would return only 400. Need to work on this.
    – TheTechGuy
    Jun 15, 2012 at 18:30
  • Sorry, I simply missed the @RepID variable in the where clause inside the cursor (I think it was lost in the formatting before Martin fixed it). Please try my updated code. Your first attempt at a query should never be a cursor, though I do feel a little less strongly about whether they should never be used - they do have their place (e.g. running totals). Jun 15, 2012 at 20:21
0

Temp tables are your friend

The way I solved my problem, merging data from two tables, removed duplicates in complex fashion and everything extremely fast was to use temporary table. This is what I did

Create a #temp table, fetch the merged data from both the tables. Make sure you include ID fields in both tables even if you do not required it. This will help remove duplicates.

Now you can do all sort of calculation on this table. Remove duplicates from table B, just remove duplicate table B IDs. Remove duplicates from table A, just remove duplicate table A Ids. There is more complexity to the problem but at least this is probably the best way to solve your problem and make it considerably faster if cursors are too expensive and takes considerable time to calculate. In my case it was taking +5 min. The #temp table query about about 5 sec, which had a lot more calculations in it.


While applying Aaron solution, the cursor did not get any faster. The second query was faster but it did not give me the correct answer, so finally I used temp tables. This is my own answer.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.