Count parameter not working Help!

  • 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?

  • 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


    Everything you can imagine is real.

  • 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

     

  • 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

  • i think Vladan is spot on with the best solution. after all sql is optimised for set based operations


    Everything you can imagine is real.

  • 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"

  • 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.

  • 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.

  • put this statement at the top of your query

    SET NOCOUNT OFF;


    Everything you can imagine is real.

  • 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