1
\$\begingroup\$

So I got this query from another analyst here and I'm a little stumped as to why a cursor was used instead of just joins. Could I please have some help trying to refactor it? I am using MSSQL 2008.

SET NOCOUNT ON;

DECLARE @myCur AS CURSOR;
DECLARE @totalCdes AS INT;
DECLARE @SysOjb AS SMALLINT;
DECLARE @prinOjb AS SMALLINT;
DECLARE @JobClass AS VARCHAR (25);
DECLARE @JobNo AS INT;
DECLARE @JobType AS VARCHAR (2);
DECLARE @JobDescr AS VARCHAR (30);
DECLARE @JobComplCde AS VARCHAR (18);
DECLARE @OrderNo AS VARCHAR (16);
DECLARE @SchedDate AS DATETIME;
DECLARE @JobResolution AS VARCHAR (200);
DECLARE @startingRow AS INT;

CREATE TABLE #JobsFixCodes
(
    SYS_OJB        BIGINT       ,
    PRIN_OJB       SMALLINT     ,
    JOB_NO         INT          ,
    ORDER_NO       BIGINT       ,
    JOB_CLASS      VARCHAR (25) ,
    JOB_TYPE       VARCHAR (30) ,
    JOB_RESOLUTION VARCHAR (200),
    SCHED_DATE     DATE         
);

SET @myCur = CURSOR
    FOR SELECT [SYS_OJB],
            [PRIN_OJB],
            CASE [JOB_CLASS_OJB] 
WHEN 'C' THEN 'New Connect' 
WHEN 'D' THEN 'Disco' 
WHEN 'R' THEN 'Restart' 
WHEN 'S' THEN 'Service Change' 
WHEN 'T' THEN 'Trouble Call' 
WHEN 'Z' THEN 'Special Request' ELSE 'Unknown' 
END AS JobClass,
            [JOB_NO_OJB],
            [JOB_TYP_OJB],
            COALESCE (cagent.DESCR_CTD, cprin.DESCR_CTD, csys.DESCR_CTD, 'Unknown') AS Job_Descr,
            COMPL_CDE_OJB,
            [ORDER_NO_OJB],
            [SCHED_DTE_OJB]
        FROM   [ExternalUser].[Vantage].[OJB_JOBS] AS j
            LEFT OUTER JOIN
            [ExternalUser].[Vantage].[CTD_DISPLAY] AS cagent
            ON cagent.SYS_CTD = j.SYS_OJB
                AND cagent.PRIN_CTD = j.PRIN_OJB
                AND cagent.AGNT_CTD = j.AGNT_OJB
                AND cagent.CDE_TBL_NO_CTD = '32'
                AND cagent.CDE_VALUE_CTD = j.JOB_TYP_OJB
                AND cagent.SPA_FLG_CTD = 'A'
            LEFT OUTER JOIN
            [ExternalUser].[Vantage].[CTD_DISPLAY] AS cprin
            ON cprin.SYS_CTD = j.SYS_OJB
                AND cprin.PRIN_CTD = j.PRIN_OJB
                AND cprin.CDE_TBL_NO_CTD = '32'
                AND cprin.CDE_VALUE_CTD = j.JOB_TYP_OJB
                AND cprin.SPA_FLG_CTD = 'P'
            LEFT OUTER JOIN
            [ExternalUser].[Vantage].[CTD_DISPLAY] AS csys
            ON csys.SYS_CTD = j.SYS_OJB
                AND csys.CDE_TBL_NO_CTD = '32'
                AND csys.CDE_VALUE_CTD = j.JOB_TYP_OJB
                AND csys.SPA_FLG_CTD = 'S'
        WHERE  JOB_STAT_OJB = 'C'
            AND SYS_OJB = '8155'
            --and SCHED_DTE_OJB = @DateParm
            AND SCHED_DTE_OJB = CONVERT (VARCHAR (20), GETDATE()-3, 101); /*Use this to run 3 days in arrears*/
            --and JOB_CLASS_OJB in (@JobClassParm) **Keep this commented out if you want *all* job classes
OPEN @myCur;

FETCH NEXT FROM @myCur INTO @SysOjb, @prinOjb, @JobClass, @JobNo, @JobType, @JobDescr, @JobComplCde, @OrderNo, @SchedDate;

WHILE @@FETCH_STATUS = 0
    BEGIN
        SET @totalCdes = len(@JobComplCde);
        --if @totalCdes is = 0 then don't loop
        IF @totalCdes > 0
            BEGIN
                IF @totalCdes = 3
                OR @totalCdes = 6
                OR @totalCdes = 9
                OR @totalCdes = 12
                OR @totalCdes = 15
                OR @totalCdes = 18
                    SET @totalCdes = @totalCdes / 3; --this is the true loop count
                ELSE
                    SET @totalCdes = @totalCdes / 2;
            END
        ELSE
            GOTO QuitMe;
        -- print 'Total job resolutions for order (: ' + convert(varchar(16), @OrderNo) + '): ' + convert(varchar(10), @totalCdes)
        IF @JobClass = 'Service Change'
        OR @JobClass = 'Special Request'
            WHILE @totalCdes > 0
                BEGIN
                    SET @startingRow = (@totalCdes * 3) - 2;
                    -- print 'Service Change in prin ' + convert(varchar(4),@prinOjb) + ' found with order: ' + convert(varchar(16), @OrderNo) + ' with ' + convert(varchar(16), @totalCdes) + ' resolutions of ' + @JobComplCde
                    -- print 'starting row is ' + convert(varchar(4),@startingRow)
                    SET @JobResolution = (SELECT descr_ctd
                                        FROM   [ExternalUser].[Vantage].[CTD_DISPLAY]
                                        WHERE  CDE_TBL_NO_CTD = '19'
                                                AND CDE_VALUE_CTD = SUBSTRING(@JobComplCde, @startingRow, 3)
                                                AND SYS_CTD = '8155');
                    INSERT  INTO #JobsFixCodes
                    VALUES (@SysOjb, @prinOjb, @JobNo, @OrderNo, @JobClass, @JobDescr, @JobResolution, @SchedDate);
                    SET @totalCdes = @totalCdes - 1;
                END
        IF @JobClass = 'Trouble Call'
            WHILE @totalCdes > 0
                BEGIN
                    SET @startingRow = (@totalCdes * 3) - 2;
                    -- print 'Service Change in prin ' + convert(varchar(4),@prinOjb) + ' found with order: ' + convert(varchar(16), @OrderNo) + ' with ' + convert(varchar(16), @totalCdes) + ' resolutions of ' + @JobComplCde
                    -- print 'starting row is ' + convert(varchar(4),@startingRow)
                    SET @JobResolution = (SELECT descr_ctd
                                        FROM   [ExternalUser].[Vantage].[CTD_DISPLAY]
                                        WHERE  CDE_TBL_NO_CTD = '08'
                                                AND CDE_VALUE_CTD = SUBSTRING(@JobComplCde, @startingRow, 3)
                                                AND SYS_CTD = '8155');
                    INSERT  INTO #JobsFixCodes
                    VALUES (@SysOjb, @prinOjb, @JobNo, @OrderNo, @JobClass, @JobDescr, @JobResolution, @SchedDate);
                    SET @totalCdes = @totalCdes - 1;
                END
        QuitMe:
        FETCH NEXT FROM @myCur INTO @SysOjb, @prinOjb, @JobClass, @JobNo, @JobType, @JobDescr, @JobComplCde, @OrderNo, @SchedDate;
    END

CLOSE @myCur;

DEALLOCATE @myCur;

SELECT   *
FROM     #JobsFixCodes AS f
ORDER BY f.JOB_CLASS, f.ORDER_NO, f.SCHED_DATE;

DROP TABLE #JobsFixCodes;
\$\endgroup\$
3
  • \$\begingroup\$ Check out Jeff Atwood's post on Rubber Duck problem solving: codinghorror.com/blog/2012/03/rubber-duck-problem-solving.html \$\endgroup\$ – Tom Halladay May 8 '12 at 22:35
  • \$\begingroup\$ Refactoring complex Cursor procedures into proper SQL queries is Hard. That's why so many people here would rather not have to think about it. Take a look at this series of article (There Must Be 50 Ways to Lose Your Cursors), written explicitly to help developers refactor thier Cursors into good SQL queries. Sadly, the notoriously opinionated author never finished the series, but these first two article should be perfect for helping you to get started with the refactoring process. \$\endgroup\$ – RBarryYoung May 9 '12 at 1:27
  • \$\begingroup\$ So, the question was migrated here, after and answer was already posted, and then my answer was turned into a comment and I was never notified? Nice. Sadly, this is the type of classless, high-handed behavior I have come to expect from StackExchange. \$\endgroup\$ – RBarryYoung May 9 '12 at 20:04
2
\$\begingroup\$

Refactoring complex cursor procedures into proper SQL queries is Hard. That's why so many people here would rather not have to think about it. Take a look at this series of article (There Must Be 15 Ways to Lose Your Cursors), written explicitly to help developers refactor their cursors into good SQL queries. Sadly, the notoriously opinionated author never finished the series, but these first two article should be perfect for helping you to get started with the refactoring process.

\$\endgroup\$
1
\$\begingroup\$

Back in the dark ages, when I was a newbie at SQL 2000, I had a mentor who coached me on cursors. As a result I created a template which will run as fast as possible for a simple cursor. This template is designed to run the looping process as fast as possible. This simple cursor rivals the newer forms for speed.

DECLARE @Name1 VARCHAR(50), @Name2 VARCHAR(50), @Name3 VARCHAR(50)
DECLARE CurName CURSOR FAST_FORWARD READ_ONLY FOR
SELECT statement

OPEN CurName
FETCH NEXT FROM CurName INTO @Name1, @Name2, @Name3
WHILE @@FETCH_STATUS = 0 BEGIN

    -- Do Something

    FETCH NEXT FROM CurName INTO @Name1, @Name2, @Name3
END
CLOSE CurName
DEALLOCATE CurName

The part that makes it fast is the "FAST_FORWARD READ_ONLY" portion. It only works for the simple style. (I never update in a cursor. Too slow.)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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