January 19, 2010 at 10:26 am
I have a stored procedure that uses about 15 different cursors that look like this:
DECLARE file_cursor CURSOR FOR
SELECT fileID FROM fileList
WHERE patientID = @patientID
AND (fileType = '1' OR fileType = '5')
OPEN file_cursor
FETCH NEXT FROM file_cursor
INTO @fileID
WHILE @@FETCH_STATUS = 0
BEGIN
SET @newfileID = NEWID()
INSERT INTO XMLtemppatientMap(oldpatientID, newpatientID, oldfileID, newfileID) VALUES
(@patientID, @newpatientID, @fileID, @newfileID)
FETCH NEXT FROM file_cursor
INTO @fileID
END
CLOSE file_cursor
DEALLOCATE file_cursor
I have read that using cursors can hurt performance, and I was wondering if there was a better way of re-writing my cursors.
I've seen some examples of using loops instead of cursors, but I don't think I can use a loop for the above cursor because I am using the cursor to fill a temporary table.
If anyone has any suggestions, please let me know...
Thanks!
January 19, 2010 at 10:45 am
It would help to see all the code, not just a snippet. Based on the snippet alone, hard to say as there are variables being used in the INSERT that aren't defined anywhere in the snippet.
January 19, 2010 at 11:03 am
Based on what you provided so far the cursor could be elminated by a single insert statement like this:
INSERT INTO XMLtemppatientMap(oldpatientID, newpatientID, oldfileID, newfileID)
SELECT patientID,'@newpatientID',fileID,NEWID()
FROM fileList
WHERE patientID = @patientID
AND (fileType = '1' OR fileType = '5')
Please note that due to the missing declaration of @newpatientID I used it as a string.
To extend it even further:
If this cursor is called from another cursor providing the value for @patientID this outer cursor most probably could be removed as well.
But like Lynn stated before: there's not enough information...
Best case: remaining number of cursors after improvement = Zero 😉
January 19, 2010 at 11:48 am
Here is one block of code containing multiple cursors.
/*Variable declarations*/
DECLARE @newpatientID uniqueidentifier
DECLARE @newfileID NVARCHAR(50)
DECLARE @newLinkID NVARCHAR(50)
DECLARE @fileID NVARCHAR(50)
DECLARE @linkID NVARCHAR(50)
DECLARE @tempprocedureID UNIQUEIDENTIFIER
DECLARE @parentfileID NVARCHAR(50)
DECLARE @childfileID NVARCHAR(50)
DECLARE @orderID INT
DECLARE @linkText NVARCHAR(MAX)
DECLARE @xraysID UNIQUEIDENTIFIER
DECLARE @newxraysID UNIQUEIDENTIFIER
DECLARE @examID UNIQUEIDENTIFIER
DECLARE @newexamID UNIQUEIDENTIFIER
DECLARE @newprocedureID UNIQUEIDENTIFIER
DECLARE @procedureID UNIQUEIDENTIFIER
DECLARE @procedureText NVARCHAR(MAX)
DECLARE @fileType TINYINT
/*temporary fileID Mapping*/
DECLARE file_cursor CURSOR FOR
SELECT fileID FROM fileList
WHERE patientID = @patientID
AND (fileType = '1' OR fileType = '5')
OPEN file_cursor
FETCH NEXT FROM file_cursor
INTO @fileID
WHILE @@FETCH_STATUS = 0
BEGIN
SET @newfileID = NEWID()
INSERT INTO XMLtemporarypatientMap(oldpatientID, newpatientID, oldfileID, newfileID) VALUES
(@patientID, @newpatientID, @fileID, @newfileID)
FETCH NEXT FROM file_cursor
INTO @fileID
END
CLOSE file_cursor
DEALLOCATE file_cursor
/*linkID Mapping*/
DECLARE link_cursor CURSOR FOR
SELECT fileLinkID, parentfileID, childfileID, procedureID FROM fileLinks
WHERE parentfileID IN(SELECT oldfileID FROM XMLtemporarypatientMap)
OR childfileID IN(SELECT oldfileID FROM XMLtemporarypatientMap)
OPEN link_cursor
FETCH NEXT FROM link_cursor
INTO @linkID, @parentfileID, @childfileID, @tempprocedureID
WHILE @@FETCH_STATUS = 0
BEGIN
SET @newLinkID = NEWID()
INSERT INTO XMLtemporaryLinkMap(oldpatientID, newpatientID, oldLinkID, newLinkID, oldprocedureID)
VALUES (@patientID, @newpatientID, @linkID, @newLinkID, @tempprocedureID)
FETCH NEXT FROM link_cursor
INTO @linkID, @parentfileID, @childfileID, @tempprocedureID
END
CLOSE link_cursor
DEALLOCATE link_cursor
/*xraysID Mapping*/
DECLARE xrays_cursor CURSOR FOR
SELECT xraysID, fileID, examID
FROM fileList WHERE fileID IN (SELECT fileID FROM fileList WHERE patientID = @patientID)
OPEN xrays_cursor
FETCH NEXT FROM xrays_cursor
INTO @xraysID, @fileID, @examID
WHILE @@FETCH_STATUS = 0
BEGIN
/*check for NULL values*/
IF @xraysID IS NULL
BEGIN
SET @newxraysID = NULL
END
ELSE
SET @newxraysID = NEWID()
IF @examID IS NULL
BEGIN
SET @newexamID = NULL
END
ELSE
SET @newexamID = NEWID()
INSERT INTO XMLtemporaryexamMap(oldexamID, newexamID, fileID, oldxraysID, newxraysID)
VALUES (@examID, @newexamID, @fileID, @xraysID, @newxraysID)
FETCH NEXT FROM xrays_cursor
INTO @xraysID, @fileID, @examID
END
CLOSE xrays_cursor
DEALLOCATE xrays_cursor
/*procedureID Mapping*/
DECLARE procedure_cursor CURSOR FOR
SELECT a.procedureID, a.examID, b.newexamID, b.fileID, a.procedureText
FROM examprocedures a, XMLtemporaryexamMap b
WHERE a.examID = b.oldexamID
OPEN procedure_cursor
FETCH NEXT FROM procedure_cursor
INTO @procedureID, @examID, @newexamID, @fileID, @procedureText
WHILE @@FETCH_STATUS = 0
BEGIN
SET @newprocedureID = NEWID()
INSERT INTO XMLtemporaryprocedureMap(sessionID, oldfileID, oldprocedureID, newprocedureID, procedureText)
VALUES (@mySessionID, @fileID, @procedureID, @newprocedureID, @procedureText)
FETCH NEXT FROM procedure_cursor
INTO @procedureID, @examID, @newexamID, @fileID, @procedureText
END
CLOSE procedure_cursor
DEALLOCATE procedure_cursor
Thanks!
January 19, 2010 at 11:16 pm
Step 1: Document what the requirements are for each cursor at a business level.
Step 2: Write new set based code as if you never saw the cursor... just the requirements.
--Jeff Moden
Change is inevitable... Change for the better is not.
January 20, 2010 at 7:20 am
Oh ok, so I should just replace the cursors with statements like Imu92 suggested above? How would that work when I need to insert a new variable value into each row? With the cursor, I just used something like: SET @newfileID = NEWID() before each row insert....
Thanks!
January 20, 2010 at 7:34 am
NewID gives a new value per row, so you can just put that in the INSERT .. SELECT statement, exactly as Lutz did in his post.
If it's something other than NewID, give us the details and we can advise.
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
January 21, 2010 at 3:58 am
Try this to illustrate Gail's explanation:
SELECT TOP 10 NEWID() FROM sys.objects
January 21, 2010 at 7:21 am
Oh ok, I see, that makes sense
Thanks for all the help!
January 21, 2010 at 9:26 am
Seems that most of your code follows a similar pattern for each of the cursors, so can be replaced by code similar to Lutz's example earlier.
The section marked "xraysID Mapping" differs slightly but can be handled easily by using CASE something like this:
INSERT
XMLtemporaryexamMap
(
oldexamID,
newexamID,
fileID,
oldxraysID,
newxraysID
)
SELECT
examID,
CASE WHEN examID IS NULL THEN NULL ELSE NEWID() END,
fileID,
xraysID,
CASE WHEN xraysID IS NULL THEN NULL ELSE NEWID() END
FROM
fileList
WHERE
fileID IN
(
SELECT
fileID
FROM
fileList
WHERE
patientID = @patientID
)
Hope this helps.
Please note that I've not tested the code.
January 21, 2010 at 11:19 am
One thing I will add. If you have values (such as the GUIDs) from previous inserted data that you do in a set-based statement that you need to use for subsequent set-based operations take a look at the OUTPUT clause, which you can use to get at the newly inserted data.
Best,
Kevin G. Boles
SQL Server Consultant
SQL MVP 2007-2012
TheSQLGuru on googles mail service
Viewing 11 posts - 1 through 10 (of 10 total)
You must be logged in to reply to this topic. Login to reply