June 29, 2007 at 12:24 am
Having an issue with the @Count parameter in the following and I'm rather new to this so I may have missed something:
Create Procedure AAA
As
Declare Customer Cursor
For
Select ID, DateFld from Table1 where type = 4 and ID In
(Select ID from Table1 where type_ID = 8)
Declare @Expiry Datetime
Declare @CustomerID Varchar(30)
Declare @Count int
Open Customer
Fetch Customer into @CustomerID, @Expiry
while (@@Fetch_Status <> -1)
Begin
Set @Count = (Select Count(*) from Followuptbl where
Datediff(Day,Create_Date,@Expiry) < 366 and ID = @CustomerID)
Update Table1 Set totalfollowupfld = @Count where ID = @CustomerID and Type = 5
Fetch Customer INTO @CustomerID, @Expiry
End
Close Cursor
Deallocate Cursor
I printed the values of @CustomerID, @Expiry and @Count. The procedure pulls in the correct number of Rows, the values in @CustomerID & @Expiry are as expected but @Count is always 0 so nothing changes
Tried running the Select Count statement by itself and filled in the values for @Expiry and @CustomerID from the first row pulled in by the cursor. I got the expected Count. This is supposed to track the number of inquiries made during the Customers subscription and update an Integer Column. The Database is not properly normalized hence the funny select statement at the top to get ID's tagged as Active and having an expiry date (Exists on 2 lines in the Table) Any ideas?
June 29, 2007 at 3:10 am
i think you are missing a NEXT keyword in you FETCH statement
FETCH
NEXT Customer into @CustomerID, @Expiry
WHILE
(@@FETCH_STATUS <> -1)
Begin
Set
@Count = (Select Count(*) from Followuptbl where
Datediff
(Day,Create_Date,@Expiry ) < 366 and ID = @CustomerID)
Update
Table1 Set totalfollowupfld = @Count where ID = @CustomerID and Type = 5
FETCH
NEXT Customer INTO @CustomerID, @Expiry
END
CLOSE
CURSOR
June 29, 2007 at 3:16 am
Hi Paul,
Heres a bunch of suggestions: Use Select/print statements inside the cursor to see whats going on
e.g. -- see if the count is non-zero
SELECT COUNT(*) FROM FollowUpTbl WHERE ...
If the count is zero check for nulls etc on Create_Date - if there are likly to be nulls on Create_Date - default it to something sensible
Datediff(Day,Create_Date,@Expiry) < 366
Did you mean that create date is less than 366 days before expiry date?
-- and see which records should be updated
SELECT * FROM Table1 WHERE ID= @CustomerID AND Type=5
and I generally use slightly different syntax to yours, as follows - I'm not sure if it will make any difference to the logic.
SELECT @Count = COUNT(1) FROM FollowUpTbl WHERE ...
AND
FETCH NEXT FROM Customer INTO @CustomerID, @Expiry
Hope something is of use
June 29, 2007 at 5:55 am
Will no one ask THE question ? No? Well, then it is up to me : Why do you use cursor? Is there something that forces you to use it?
I'm not sure whether I understood the requirements and specifics of your data structure correctly, but something similar to this could do the job without cursor. If you need more info, please post table structure (CREATE TABLE statements) and some sample data (INSERT INTO...).
UPDATE tbl_upd
SET totalfollowupfld = calc.cnt
FROM Table1 tbl_upd
JOIN
(SELECT tbl.[ID] as customer, COUNT(*) as cnt
FROM Table1 tbl
JOIN (select [ID] from Table1 where type_ID = 8 group by [ID]) as has_expiry ON has_expiry.[ID] = tbl.[ID]
JOIN followuptbl f ON f.[ID]=tbl.[ID] AND DATEDIFF(day, f.create_date, tbl.DateFld) < 366
WHERE tbl.type = 4
GROUP BY tbl.[ID]) as calc ON calc.customer=tbl_upd.[ID]
WHERE tbl_upd.type = 5
June 29, 2007 at 6:17 am
June 29, 2007 at 6:31 am
Those of us used to procedural languages like cursors in SQL - at least to start with. They are easy to understand, when the set-based solution isn't always obvious. Cursors can be the best solution in situations where the people who are going to inherit your code, when you move on, aren't experts in the field.
"I can imagine a world where cursors are cool"
June 29, 2007 at 8:45 am
Tom,
I can understand your position (although I don't agree), and I also know that there are certain situations where you simply have to use cursors (sometimes because the database has poor design and you can't change it). That's why I was asking... if cursor was used just because it seemed "easy to write and maintain", then I would suggest replacing it with a set-based approach.
Why? Well, mainly because of performance (cursors often perform VERY badly in SQL Server). And performance should be preferred over other things, including simplicity of the code - also because "obvious" is very subjective term.
How do you know the people who inherit the code will understand cursors better than set-based approach? For me, it is harder to understand the cursor... IMHO the solution here should be cleanly written and well commented SQL (then it makes little difference to understandability whether it is a cursor or set-based solution).
Of course it is up to everyone to decide whether to use cursors or not (and where). I'm just saying that in some places, by replacing cursor with set-based solution, you can shorten the execution time a lot (like from 10 minutes to 10 seconds). In my opinion well worth trying, and I avoid cursors everywhere possible.
June 29, 2007 at 11:42 am
Thank you for all the suggestions. I tried the Fetch next option and the Count(1) but the result was the same. Also, I did print out the values for the @ClientID, @Expiry and @Count Values. The Cursor is returning 3 rows as expected with the correct ClientID and Expiry date for each row but the Count is 0 for each Row and it should be 5, 4, 4. This is where it is failing. When I run the Select Count Manually & swap in the values the Cursor returns for ClientID & Expiry, I do get the correct numbers so I'm not sure it is failing here. This is supposed to return a count of all the inquires for a specific Customer type for each Customer during the period of their Subscription and write this to a table
I used a Cursor as I am more familiar with a procedural language than SQL Sets & this made more since to me. Also, this will only be applied against 1000 records in production off peak hours so I didn't think it would greatly affect performance. I'll have to test that. I'll try the set based approach and see what happens. Also the database is unusual in that the Expiry value is in a different row than the type value for each ClientID in the table.
June 29, 2007 at 3:59 pm
July 2, 2007 at 7:04 am
when you say it is returning 0 as your count verify that you are updating the correct thing. How are you verifying that it is 0?
Something I noticed is you select where type = 4 but you update where type = 5.
totalfollowupfld = @Count where ID = @CustomerID and Type = 5
When you are verifying the amount is 0 are you looking where type is 5?
Viewing 10 posts - 1 through 9 (of 9 total)
You must be logged in to reply to this topic. Login to reply