May 4, 2016 at 1:32 pm
Hello all,
Forgive my ignorance but I am new to using Cursors in T-SQL. I thank you in advance for the input and direction you provide.
My problem is that my code is not returning all of the records.
Here is the code:
IF OBJECT_ID('tempdb..#TempRY') IS NOT NULL
BEGIN
DROP TABLE #TempRY
END
GO
DECLARE @tempRY TABLE(
rYear nvarchar(4),
rmonth nvarchar(2),
tblclaimstatusactionID bigint,
tbllineID bigint,
tbllinestatusID int,
companyname nvarchar(50),
ronumber nvarchar(50),
line char(4),
cstatus nvarchar(60),
LastUpdatedBy int,
name nvarchar(50),
dStarted datetime,
Dfinsished datetime,
Duration_sec int,
RY nvarchar(3))
DECLARE @employee int
DECLARE Employee_Cursor CURSOR FOR
SELECT .tbluserID
FROM INNER JOIN
tbljfunctiontable ON .tbluserID = tbljfunctiontable.tbluserid
WHERE (tbljfunctiontable.jfunction = N'closing') AND (.notactive = 0) AND (.tbluserID <> 288) AND (.tbluserID <> 643);
OPEN Employee_Cursor;
FETCH FROM Employee_Cursor
INTO @employee;
Insert into @tempRY (ryear, rmonth, tblclaimstatusactionID,tbllineID, tbllinestatusID, companyname, ronumber, line, cstatus, LastUpdatedBy, name, dStarted, Dfinsished, Duration_sec, RY)
SELECT DATEPART(YYYY, derivedtbl_1.LastUpdatedOn) as Year, DATEPART(MM, derivedtbl_1.LastUpdatedOn) as Month, derivedtbl_1.tblclaimstatusactionID, derivedtbl_1.tbllineID, derivedtbl_1.tbllinestatusId, tblcustomer.companyname, tblro.ronumber, tblline.line, tbllinestatus.status, derivedtbl_1.LastUpdatedBy, .name, derivedtbl_2.LastUpdatedOn AS Started, derivedtbl_1.LastUpdatedOn as Finished,
DATEDIFF(ss, derivedtbl_2.LastUpdatedOn, derivedtbl_1.LastupdatedOn) AS Duration_sec, CASE When DATEDIFF(ss, derivedtbl_2.LastUpdatedOn, derivedtbl_1.LastupdatedOn) <= 899 Then 'OK'
When DATEDIFF(ss, derivedtbl_2.LastUpdatedOn, derivedtbl_1.LastupdatedOn) >= 900 AND DATEDIFF(ss, derivedtbl_2.LastUpdatedOn, derivedtbl_1.LastupdatedOn) <=1199 Then 'Y'
When DATEDIFF(ss, derivedtbl_2.LastUpdatedOn, derivedtbl_1.LastupdatedOn) >= 1200 AND derivedtbl_1.tbllinestatusId <> 78 Then 'R'
When DATEDIFF(ss, derivedtbl_2.LastUpdatedOn, derivedtbl_1.LastupdatedOn) <= 899 AND derivedtbl_1.tbllinestatusId = 78 Then 'OK'
ELSE
'OK'END AS RY
FROM (SELECT TOP (100) PERCENT ROW_NUMBER() over (Order by LastUpdatedON DESC) AS RowNum, tblclaimstatusactionID, tbllineID, tbllinestatusId, LastUpdatedBy, LastUpdatedOn
FROM tblclaimstatusaction
WHERE (LastUpdatedBy = @employee) AND (tbllinestatusId <> 23)
ORDER BY LastUpdatedBy, LastUpdatedOn DESC) AS derivedtbl_1 INNER JOIN
tbllinestatus ON derivedtbl_1.tbllinestatusId = tbllinestatus.tbllinestatusID INNER JOIN
tblline ON derivedtbl_1.tbllineID = tblline.tbllineID INNER JOIN
tblro ON tblline.tblroID = tblro.tblroID INNER JOIN
tblcustomer ON tblro.tblcustomerID = tblcustomer.tblcustomerID INNER JOIN
ON derivedtbl_1.LastUpdatedBy = .tbluserID INNER JOIN
(SELECT TOP (100) PERCENT ROW_NUMBER() over (Order by LastUpdatedON DESC)-1 AS RowNum, tblclaimstatusactionID, tbllineID, tbllinestatusId, LastUpdatedBy, LastUpdatedOn
FROM tblclaimstatusaction AS tblclaimstatusaction_1
WHERE (LastUpdatedBy = @employee) AND (tbllinestatusId <> 23) AND (LastUpdatedOn > DATEADD(month, DATEDIFF(month, -2, getdate()) - 7, 0))
ORDER BY LastUpdatedBy, LastUpdatedOn DESC) AS derivedtbl_2 ON derivedtbl_1.LastUpdatedBy = derivedtbl_2.LastUpdatedBy AND
derivedtbl_1.RowNum = derivedtbl_2.RowNum
WHILE @@FETCH_STATUS = 0
BEGIN
FETCH NEXT FROM Employee_Cursor
INTO @employee;
Insert into @tempRY (ryear, rmonth, tblclaimstatusactionID,tbllineID, tbllinestatusID, companyname, ronumber, line, cstatus, LastUpdatedBy, name, dStarted, Dfinsished, Duration_sec, RY)
SELECT DATEPART(YYYY, derivedtbl_1.LastUpdatedOn) as Year, DATEPART(MM, derivedtbl_1.LastUpdatedOn) as Month, derivedtbl_1.tblclaimstatusactionID, derivedtbl_1.tbllineID, derivedtbl_1.tbllinestatusId, tblcustomer.companyname, tblro.ronumber, tblline.line, tbllinestatus.status, derivedtbl_1.LastUpdatedBy, .name, derivedtbl_2.LastUpdatedOn AS Started, derivedtbl_1.LastUpdatedOn as Finished,
DATEDIFF(ss, derivedtbl_2.LastUpdatedOn, derivedtbl_1.LastupdatedOn) AS Duration_sec, CASE When DATEDIFF(ss, derivedtbl_2.LastUpdatedOn, derivedtbl_1.LastupdatedOn) <= 899 Then 'OK'
When DATEDIFF(ss, derivedtbl_2.LastUpdatedOn, derivedtbl_1.LastupdatedOn) >= 900 AND DATEDIFF(ss, derivedtbl_2.LastUpdatedOn, derivedtbl_1.LastupdatedOn) <=1199 Then 'Y'
When DATEDIFF(ss, derivedtbl_2.LastUpdatedOn, derivedtbl_1.LastupdatedOn) >= 1200 AND derivedtbl_1.tbllinestatusId <> 78 Then 'R'
When DATEDIFF(ss, derivedtbl_2.LastUpdatedOn, derivedtbl_1.LastupdatedOn) >= 1200 AND derivedtbl_1.tbllinestatusId = 78 Then 'OK'
ELSE
'OK'END AS RY
FROM (SELECT TOP (100) PERCENT ROW_NUMBER() over (Order by LastUpdatedON DESC) AS RowNum, tblclaimstatusactionID, tbllineID, tbllinestatusId, LastUpdatedBy, LastUpdatedOn
FROM tblclaimstatusaction
WHERE (LastUpdatedBy = @employee) AND (tbllinestatusId <> 23)
ORDER BY LastUpdatedBy, LastUpdatedOn DESC) AS derivedtbl_1 INNER JOIN
tbllinestatus ON derivedtbl_1.tbllinestatusId = tbllinestatus.tbllinestatusID INNER JOIN
tblline ON derivedtbl_1.tbllineID = tblline.tbllineID INNER JOIN
tblro ON tblline.tblroID = tblro.tblroID INNER JOIN
tblcustomer ON tblro.tblcustomerID = tblcustomer.tblcustomerID INNER JOIN
ON derivedtbl_1.LastUpdatedBy = .tbluserID INNER JOIN
(SELECT TOP (100) PERCENT ROW_NUMBER() over (Order by LastUpdatedON DESC)-1 AS RowNum, tblclaimstatusactionID, tbllineID, tbllinestatusId, LastUpdatedBy, LastUpdatedOn
FROM tblclaimstatusaction AS tblclaimstatusaction_1
WHERE (LastUpdatedBy = @employee) AND (tbllinestatusId <> 23) AND (LastUpdatedOn > DATEADD(month, DATEDIFF(month, -2, getdate()) - 7, 0))
ORDER BY LastUpdatedBy, LastUpdatedOn DESC) AS derivedtbl_2 ON derivedtbl_1.LastUpdatedBy = derivedtbl_2.LastUpdatedBy AND
derivedtbl_1.RowNum = derivedtbl_2.RowNum
FETCH NEXT FROM Employee_Cursor
INTO @employee;
END;
Select *
From @tempRY
CLOSE Employee_Cursor;
DEALLOCATE Employee_Cursor;
GO
When this is run it returns data for 4 different employees.
IF I run this part of the code only, it returns 6 different employee numbers.
SELECT .tbluserID
FROM INNER JOIN
tbljfunctiontable ON .tbluserID = tbljfunctiontable.tbluserid
WHERE (tbljfunctiontable.jfunction = N'closing') AND (.notactive = 0) AND (.tbluserID <> 288) AND (.tbluserID <> 643)
Why is it not fetching the records for the other 2 employees? I have verified that there are records for the missing employees that meet the same criteria as the employees that it is returning data for.
May 4, 2016 at 1:38 pm
1) Cursors (actually almost ANY row-by-agonizing-row or iterative processing) in SQL Server is horribly inefficient.
2) You need to provide us with table definitions and actual data for them (in the form of inserts) to be able to help you most effectively.
3) You should evaluate the query plans to see where you are going from expected row counts to unexpected ones. That may key you in on the problem without us needing to assist.
Best,
Kevin G. Boles
SQL Server Consultant
SQL MVP 2007-2012
TheSQLGuru on googles mail service
May 4, 2016 at 2:52 pm
This is my best guess at doing this WITHOUT A CURSOR.
I combined your two derived tables into one by using the LAG function and removed unnecessary conditions in your CASE expression.
Insert into @tempRY (ryear, rmonth, tblclaimstatusactionID,tbllineID, tbllinestatusID, companyname, ronumber, line, cstatus, LastUpdatedBy, name, dStarted, Dfinsished, Duration_sec, RY)
SELECT
DATEPART(YYYY, derivedtbl_1.LastUpdatedOn) as Year,
DATEPART(MM, derivedtbl_1.LastUpdatedOn) as Month,
derivedtbl_1.tblclaimstatusactionID,
derivedtbl_1.tbllineID,
derivedtbl_1.tbllinestatusId,
tblcustomer.companyname,
tblro.ronumber,
tblline.line,
tbllinestatus.status,
derivedtbl_1.LastUpdatedBy,
.name,
derivedtbl_1.prevLastUpdatedOn AS Started,
derivedtbl_1.LastUpdatedOn as Finished,
DATEDIFF(ss, derivedtbl_1.prevLastUpdatedOn, derivedtbl_1.LastupdatedOn) AS Duration_sec,
CASE
WHEN DATEDIFF(ss, derivedtbl_1.prevLastUpdatedOn, derivedtbl_1.LastupdatedOn) <= 899 Then 'OK'
WHEN DATEDIFF(ss, derivedtbl_2.LastUpdatedOn, derivedtbl_1.LastupdatedOn) <=1199 Then 'Y'
WHEN derivedtbl_1.tbllinestatusId <> 78 Then 'R'
ELSE 'OK'
END AS RY
FROM
(
SELECT tblclaimstatusactionID, tbllineID, tbllinestatusId, LastUpdatedBy, LastUpdatedOn, LAG(LastUpdatedOn) OVER( PARTITION BY LastUpdatedBy ORDER BY LastUpdatedOn) prevLastUpdateOn
FROM tblclaimstatusaction
WHERE tbllinestatusId <> 23
AND LastUpdatedOn > DATEADD(MONTH, -6, GETDATE())
) AS derivedtbl_1
ON .tbluserID = derivedtbl_1.LastUpdatedBy
INNER JOIN tbllinestatus
ON derivedtbl_1.tbllinestatusId = tbllinestatus.tbllinestatusID
INNER JOIN tblline
ON derivedtbl_1.tbllineID = tblline.tbllineID
INNER JOIN tblro
ON tblline.tblroID = tblro.tblroID
INNER JOIN tblcustomer
ON tblro.tblcustomerID = tblcustomer.tblcustomerID
Remember that CASE WHEN clauses are evaluated in order, so you don't have to test for things that can be ruled out by previous WHEN clauses. For instance your first test is whether the difference is <= 899. If that test fails, you know that the difference has to be >= 900, so you don't need to test for it.
Drew
J. Drew Allen
Business Intelligence Analyst
Philadelphia, PA
May 9, 2016 at 9:33 am
The other replies about cursors being slow are all completely correct, so better steer away from them using almost any alternative given. But the original problem is probably rather easily addressed. Have a look at Books online and lookup the page for @@fetch_status (https://msdn.microsoft.com/en-us/library/ms187308.aspx).
@@fetch_status does not indicate the end of the loop by returning <> 0, it indicates the end of the loop by returning = -1. Better use something like:
declare Employee_Cursor Cursor LOCAL FAST_FORWARD
for
....
open Employee_Cursor;
while (1=1)
begin
FETCH FROM Employee_Cursor
INTO @employee;
if @@fetch_status = -1
break;
if @@fetch_status = 0
begin
-- do your thing here
end
end
close Employee_Cursor;
deallocate Employee_Cursor;
Furthermore you should have a look at the options for declare cursor. I suggest adding "local" and "fast_forward" for performance and stability reasons. (https://msdn.microsoft.com/en-us/library/ms180169.aspx)
May 12, 2016 at 9:49 am
Thank you, that fixed my problem!
May 13, 2016 at 7:18 am
and yet it is still lots better if you would get rid of the cursor. I know it is easy to stop now, but you should REALY do without that cursor. It may seem to work well now, but in some time, for example when the number of rows has grown, it WILL bite you.
May 13, 2016 at 7:49 am
R.P.Rozema (5/13/2016)
and yet it is still lots better if you would get rid of the cursor. I know it is easy to stop now, but you should REALY do without that cursor. It may seem to work well now, but in some time, for example when the number of rows has grown, it WILL bite you.
+1000
Best,
Kevin G. Boles
SQL Server Consultant
SQL MVP 2007-2012
TheSQLGuru on googles mail service
Viewing 7 posts - 1 through 6 (of 6 total)
You must be logged in to reply to this topic. Login to reply