Using Cursor in DELETE statement

  • kdrymer

    SSC Veteran

    Points: 262

    I have a need to use a SQL Cursor in order to delete data from a table, one row at a time (to prevent issues with a trigger associated with the table being deleted from).  I wrote the following SQL, and while it is not producing any errors, it is running very long and eventually I stopped the query. My concern is that it is in some kind of infinite loop so I wanted to get feedback on whether there are any apparent issues with it.

    Basically I want to delete row(s) that are retrieved in the first SQL Select statement, so I am selecting columns ROLEUSER and ROLENAME (a ROLEUSER may have more then 1 ROLENAME values, so multiple rows per ROLEUSER may occur).

    In the Delete statement I am specifying the ROLEUSER to be equal to the variable @user and ROLENAME to be equal to @name.

    Without the use of the cursor this runs extremely fast, less than 2 seconds typically. With the Cursor, I've let it run for 15 minutes and then cancelled it while still executing, but it's hard to tell if it's stuck in an infinite loop, or if there is another issues. Appreciate any feedback on this.

    DECLARE @user VARCHAR(50)
    DECLARE @role VARCHAR(50)

    DECLARE db_cursor CURSOR FOR
    SELECT ROLEUSER, ROLENAME
    FROM PSROLEUSER PSRO
    INNER JOIN PS_GH_AD_X_WALK B ON B.OPRID = PSRO.ROLEUSER
    INNER JOIN HRDEV01_FOR_BUDGET_MODULE.HRDEV92B.dbo.PS_JOB C ON C.EMPLID = B.GH_AD_EMPLID AND B.GH_AD_EMPLID <> ''
    WHERE C.EFFDT =
    (SELECT MAX(A_ED.EFFDT) FROM HRDEV01_FOR_BUDGET_MODULE.HRDEV92B.dbo.PS_JOB A_ED
    WHERE C.EMPLID = A_ED.EMPLID
    AND C.EMPL_RCD = A_ED.EMPL_RCD
    AND A_ED.EFFDT <= SUBSTRING(CONVERT(CHAR,GETDATE(),121), 1, 10))
    AND C.ACTION = 'TER'

    OPEN db_cursor
    FETCH NEXT FROM db_cursor INTO @user, @role

    WHILE @@FETCH_STATUS = 0
    BEGIN DELETE PSRO
    FROM PSROLEUSER PSRO
    INNER JOIN PS_GH_AD_X_WALK B ON B.OPRID = PSRO.ROLEUSER
    INNER JOIN HRDEV01_FOR_BUDGET_MODULE.HRDEV92B.dbo.PS_JOB C ON C.EMPLID = B.GH_AD_EMPLID AND B.GH_AD_EMPLID <> ''
    WHERE C.EFFDT =
    (SELECT MAX(A_ED.EFFDT) FROM HRDEV01_FOR_BUDGET_MODULE.HRDEV92B.dbo.PS_JOB A_ED
    WHERE C.EMPLID = A_ED.EMPLID
    AND C.EMPL_RCD = A_ED.EMPL_RCD
    AND A_ED.EFFDT <= SUBSTRING(CONVERT(CHAR,GETDATE(),121), 1, 10))
    AND C.ACTION = 'TER'
    AND PSRO.ROLEUSER = @user
    AND PSRO.ROLENAME = @role

    END

    CLOSE db_cursor
    DEALLOCATE db_cursor
  • Luis Cazares

    SSC Guru

    Points: 183573

    You do have an infinite loop going on, but you also have some other problems. I would suggest that you change your trigger to allow you to handle multiple rows at a time.

    DECLARE @user VARCHAR(50)
    DECLARE @role VARCHAR(50)

    DECLARE db_cursor CURSOR LOCAL STATIC --Make cursor static to prevent that changes in the data affect it
    FOR
    SELECT ROLEUSER, ROLENAME
    FROM PSROLEUSER PSRO
    INNER JOIN PS_GH_AD_X_WALK B ON B.OPRID = PSRO.ROLEUSER
    INNER JOIN HRDEV01_FOR_BUDGET_MODULE.HRDEV92B.dbo.PS_JOB C ON C.EMPLID = B.GH_AD_EMPLID AND B.GH_AD_EMPLID <> ''
    WHERE C.EFFDT =
    (SELECT MAX(A_ED.EFFDT) FROM HRDEV01_FOR_BUDGET_MODULE.HRDEV92B.dbo.PS_JOB A_ED
    WHERE C.EMPLID = A_ED.EMPLID
    AND C.EMPL_RCD = A_ED.EMPL_RCD
    AND A_ED.EFFDT <= SUBSTRING(CONVERT(CHAR,GETDATE(),121), 1, 10))
    AND C.ACTION = 'TER';


    OPEN db_cursor
    FETCH NEXT FROM db_cursor INTO @user, @role
    WHILE @@FETCH_STATUS = 0
    BEGIN
    --You don't need the whole query, just use the key
    DELETE
    FROM PSROLEUSER
    WHERE ROLEUSER = @user
    AND ROLENAME = @role

    --You forgot to move forward in the cursor.
    FETCH NEXT FROM db_cursor INTO @user, @role;
    END
    CLOSE db_cursor
    DEALLOCATE db_cursor

    Luis C.
    General Disclaimer:
    Are you seriously taking the advice and code from someone from the internet without testing it? Do you at least understand it? Or can it easily kill your server?

    How to post data/code on a forum to get the best help: Option 1 / Option 2
  • kdrymer

    SSC Veteran

    Points: 262

    Thanks Luis, yes I see that I had forgotten to move the cursor forward. I added that and it now performs the transaction. Just curious what you mean by  "suggest that you change your trigger to allow you to handle multiple rows at a time." ?  Thanks.

  • crow1969

    SSCrazy

    Points: 2889

    Looks like you just need a FETCH NEXT statement in the loop.

    For troubleshooting purposes, you can log the key value(s) to some other table, so you can see if it is moving from key to key, or is stuck on the same one.

  • drew.allen

    SSC Guru

    Points: 76635

    You're missing a FETCH NEXT inside your cursor.  That being said, you'll be better off in the long run if you fix your trigger rather than trying to work around your problems.

    Drew

    J. Drew Allen
    Business Intelligence Analyst
    Philadelphia, PA

  • ScottPletcher

    SSC Guru

    Points: 98174

    Don't force SQL to fully reprocess the conditions before DELETEing each row, instead use WHERE CURRENT OF in the DELETE:

    DECLARE @user VARCHAR(50)
    DECLARE @role VARCHAR(50)

    DECLARE db_cursor CURSOR LOCAL FOR
    SELECT ROLEUSER, ROLENAME
    FROM PSROLEUSER PSRO
    INNER JOIN PS_GH_AD_X_WALK B ON B.OPRID = PSRO.ROLEUSER
    INNER JOIN HRDEV01_FOR_BUDGET_MODULE.HRDEV92B.dbo.PS_JOB C ON C.EMPLID = B.GH_AD_EMPLID AND B.GH_AD_EMPLID <> ''
    WHERE C.EFFDT =
    (SELECT MAX(A_ED.EFFDT) FROM HRDEV01_FOR_BUDGET_MODULE.HRDEV92B.dbo.PS_JOB A_ED
    WHERE C.EMPLID = A_ED.EMPLID
    AND C.EMPL_RCD = A_ED.EMPL_RCD
    AND A_ED.EFFDT <= SUBSTRING(CONVERT(CHAR,GETDATE(),121), 1, 10))
    AND C.ACTION = 'TER'

    OPEN db_cursor

    FETCH NEXT FROM db_cursor INTO @user, @role
    WHILE @@FETCH_STATUS = 0
    BEGIN
    DELETE FROM PSROLEUSER
    WHERE CURRENT OF db_cursor
    END /*WHILE*/
    CLOSE db_cursor
    DEALLOCATE db_cursor

    SQL DBA,SQL Server MVP(07, 08, 09) Prosecutor James Blackburn, in closing argument in the Fatal Vision murders trial: If in the future, you should cry a tear, cry one for them [the murder victims]. If in the future, you should say a prayer, say one for them. And if in the future, you should light a candle, light one for them.

  • kdrymer

    SSC Veteran

    Points: 262

    Being new to cursors, what exactly does the WHERE CURRENT OF db_cursor do? It will keep the SQL criteria in memory on each iteration of the loop?

  • kdrymer

    SSC Veteran

    Points: 262

    ScottPletcher wrote:

    Don't force SQL to fully reprocess the conditions before DELETEing each row, instead use WHERE CURRENT OF in the DELETE:

    DECLARE @user VARCHAR(50)
    DECLARE @role VARCHAR(50)

    DECLARE db_cursor CURSOR LOCAL FOR
    SELECT ROLEUSER, ROLENAME
    FROM PSROLEUSER PSRO
    INNER JOIN PS_GH_AD_X_WALK B ON B.OPRID = PSRO.ROLEUSER
    INNER JOIN HRDEV01_FOR_BUDGET_MODULE.HRDEV92B.dbo.PS_JOB C ON C.EMPLID = B.GH_AD_EMPLID AND B.GH_AD_EMPLID <> ''
    WHERE C.EFFDT =
    (SELECT MAX(A_ED.EFFDT) FROM HRDEV01_FOR_BUDGET_MODULE.HRDEV92B.dbo.PS_JOB A_ED
    WHERE C.EMPLID = A_ED.EMPLID
    AND C.EMPL_RCD = A_ED.EMPL_RCD
    AND A_ED.EFFDT <= SUBSTRING(CONVERT(CHAR,GETDATE(),121), 1, 10))
    AND C.ACTION = 'TER'

    OPEN db_cursor

    FETCH NEXT FROM db_cursor INTO @user, @role
    WHILE @@FETCH_STATUS = 0
    BEGIN
    DELETE FROM PSROLEUSER
    WHERE CURRENT OF db_cursor
    END /*WHILE*/
    CLOSE db_cursor
    DEALLOCATE db_cursor

      Being new to cursors, what exactly does the WHERE CURRENT OF db_cursor do? It will keep the SQL criteria in memory on each iteration of the loop?

  • ScottPletcher

    SSC Guru

    Points: 98174

    What row was just FETCHed from, that is the row that SQL will delete, since that is the current position of the cursor.

    SQL DBA,SQL Server MVP(07, 08, 09) Prosecutor James Blackburn, in closing argument in the Fatal Vision murders trial: If in the future, you should cry a tear, cry one for them [the murder victims]. If in the future, you should say a prayer, say one for them. And if in the future, you should light a candle, light one for them.

  • kdrymer

    SSC Veteran

    Points: 262

    Thanks!

  • kdrymer

    SSC Veteran

    Points: 262

    Thanks all for the help!

  • Luis Cazares

    SSC Guru

    Points: 183573

    kdrymer wrote:

    Thanks Luis, yes I see that I had forgotten to move the cursor forward. I added that and it now performs the transaction. Just curious what you mean by  "suggest that you change your trigger to allow you to handle multiple rows at a time." ?  Thanks.

    Well, you mentioned: "I have a need to use a SQL Cursor in order to delete data from a table, one row at a time (to prevent issues with a trigger associated with the table being deleted from)"

    Triggers shouldn't cause problems when data is affected multiple rows at a time. When triggers can only handle one row, it's commonly because they're using scalar variables instead of set-based code using the inserted and deleted tables. Correcting the trigger will allow you to delete all the rows that you want at once without using cursors.

    Luis C.
    General Disclaimer:
    Are you seriously taking the advice and code from someone from the internet without testing it? Do you at least understand it? Or can it easily kill your server?

    How to post data/code on a forum to get the best help: Option 1 / Option 2

Viewing 12 posts - 1 through 12 (of 12 total)

You must be logged in to reply to this topic. Login to reply