April 24, 2019 at 6:16 pm
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
April 24, 2019 at 6:33 pm
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
April 24, 2019 at 6:35 pm
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.
April 24, 2019 at 6:36 pm
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.
April 24, 2019 at 6:37 pm
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
April 24, 2019 at 6:41 pm
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) A socialist is someone who will give you the shirt off *someone else's* back.
April 24, 2019 at 6:43 pm
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?
April 24, 2019 at 6:44 pm
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?
April 24, 2019 at 6:58 pm
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) A socialist is someone who will give you the shirt off *someone else's* back.
April 24, 2019 at 7:13 pm
Thanks!
April 24, 2019 at 7:14 pm
Thanks all for the help!
April 24, 2019 at 8:29 pm
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.
Viewing 12 posts - 1 through 11 (of 11 total)
You must be logged in to reply to this topic. Login to reply