Improve my where loop!

  • I have the following WHILE loop I have put in place to replace a cursor.

    I would appreciate any pointers on making this better, in particular the use of a temporary table to make changes on a live table. Is this the wrong thing to do?

    Much appreciated.

    DECLARE @PNo numeric (10,0)

    CREATE TABLE #PT

    (PT datetime,

    PNo numeric (10,0),

    PVer numeric (2,0),

    PAction char(1),

    PDone bit)

    INSERT INTO #PT

    SELECT PT,

    PNo,

    PVer,

    PAction,

    PDone FROM PT

    WHILE (SELECT count(*) FROM PT where PDone = 0) > 0

    BEGIN

    SELECT @PNo = min(PNo) FROM #PT

    WHERE PDone = 0

    UPDATE #PT SET PDone = 1 WHERE PNo = @PNo

    UPDATE PT SET PDone = 1 WHERE PNo = @PNo

    END

    DROP TABLE #PT

    This is the shell. I want to create a temporary table of 40,000 records from PT. Then I want to throw each record into a stored procedure that reacts to this record. The above is just a pure test to loop round the records. But it feels wrong, specifically the #PT/PT dual usage with the PDone field.

    Put me right....PLEASE!!!

    Thanks loads

  • The same idea using cursors took 4 minutes compared to 16 for the code above, so clearly I'm not doing it properly!

  • Have you tried using a @counter variable that you inciment instead of running a select in you while clause?

    thanks

    Chris

    ----------------------------------------------
    Try to learn something about everything and everything about something. - Thomas Henry Huxley

    :w00t:
    Posting Best Practices[/url]
    Numbers / Tally Tables[/url]

    SQL-4-Life
  • something like this:

    DECLARE @PNo numeric (10,0)

    DECLARE @counter INT

    DECLARE @counterMAX INT

    CREATE TABLE #PT

    (ROWNUM INT PRIMARY KEY

    PT datetime,

    PNo numeric (10,0),

    PVer numeric (2,0),

    PAction char(1),

    PDone bit)

    INSERT INTO #PT

    (PT,

    PNo,

    PVer,

    PAction,

    PDone)

    SELECT PT,

    PNo,

    PVer,

    PAction,

    PDone

    FROM PT

    WHERE PDone = 0 --This will restric the number of rows you loop through

    SELECT @counter = 1

    ,@counterMAX = MAX(ROWNUM)

    FROM #PT

    WHILE (@counter <= @counterMAx)

    BEGIN

    SELECT @PNo = PNo

    FROM #PT

    WHERE ROWNUM = @counter

    UPDATE #PT SET PDone = 1 WHERE PNo = @PNo

    UPDATE PT SET PDone = 1 WHERE PNo = @PNo

    SET @counter = @counter + 1

    END

    DROP TABLE #PT

    ----------------------------------------------
    Try to learn something about everything and everything about something. - Thomas Henry Huxley

    :w00t:
    Posting Best Practices[/url]
    Numbers / Tally Tables[/url]

    SQL-4-Life
  • DECLARE @PNo NUMERIC(10,0),

    @Counter INT,

    @CounterMAX INT

    CREATE TABLE#PT

    (

    RowNum INT IDENTITY(1, 1) PRIMARY KEY,

    PT DATETIME,

    PNo NUMERIC(10,0),

    PVer NUMERIC(2,0),

    PAction CHAR(1),

    PDone BIT

    )

    INSERT#PT

    (

    PT,

    PNo,

    PVer,

    PAction,

    PDone

    )

    SELECTPT,

    PNo,

    PVer,

    PAction,

    PDone

    FROMPT

    WHEREPDone = 0

    SELECT@Counter = 1,

    @CounterMAX = MAX(RowNum)

    FROM#PT

    WHILE @Counter <= @CounterMAX

    BEGIN

    SELECT@PNo = PNo

    FROM#PT

    WHERERowNum = @Counter

    ...

    SET@Counter = @Counter + 1

    END

    UPDATEx

    SETx.PDone = 1

    FROMPT AS x

    INNER JOIN#PT AS y ON y.PNo = x.PNo

    DROP TABLE#PT


    N 56°04'39.16"
    E 12°55'05.25"

  • I'm such a fool. I've used a Counter variable before. Nice one.

    How does that ROWNUM get populated? EDIT: Identity of course! Ignore me.

    Great help guys,. Thanks.

  • DECLARE @PNo NUMERIC(10,0)

    SELECT@PNo = MIN(PNo)

    FROMPT

    WHEREPDone = 0

    WHILE @PNo IS NOT NULL

    BEGIN

    ...

    UPDATEPT

    SETPDone = 1

    WHEREPNo = @PNo

    SELECT@PNo = MIN(PT)

    FROMPT

    WHEREPDone = 0

    END


    N 56°04'39.16"
    E 12°55'05.25"

  • Sorry I forgot the Identity(1,1) on the create table... 😉

    ----------------------------------------------
    Try to learn something about everything and everything about something. - Thomas Henry Huxley

    :w00t:
    Posting Best Practices[/url]
    Numbers / Tally Tables[/url]

    SQL-4-Life
  • Peso (6/11/2008)


    DECLARE @PNo NUMERIC(10,0)

    SELECT@PNo = MIN(PNo)

    FROMPT

    WHEREPDone = 0

    WHILE @PNo IS NOT NULL

    BEGIN

    ...

    UPDATEPT

    SETPDone = 1

    WHEREPNo = @PNo

    SELECT@PNo = MIN(PT)

    FROMPT

    WHEREPDone = 0

    END

    You forgot

    SET @PNo = NULL

    before assigning new MIN(PT) inside of the loop.

    If there are no records with PDone = 0 @PNo remains unchanged.

    It's gonna be endless loop.

    _____________
    Code for TallyGenerator

  • I believe you are wrong Sergiy, again.

    SET NOCOUNT ON

    DECLARE@Sample TABLE (i INT, Done TINYINT)

    INSERT@Sample

    SELECT1, 0 UNION ALL

    SELECT2, 1 UNION ALL

    SELECT3, 0

    DECLARE @i INT

    SELECT@i = MIN(i)

    FROM@Sample

    WHEREDone = 0

    WHILE @i IS NOT NULL

    BEGIN

    PRINT'Now processing record' + STR(@i, 2)

    UPDATE@Sample

    SETDone = 1

    WHEREi = @i

    SELECT@i = MIN(i)

    FROM@Sample

    WHEREDone = 0

    END

    Above code is tested on both SQL Server 2000 sp4 and SQL Server 2005 sp2.


    N 56°04'39.16"
    E 12°55'05.25"

  • I can't get the alternatives to run faster than the Cursor so thats annoying, but I'll keep plugging on. Just tried a ROWCOUNT method but that took double the time.

  • It's SQL 2K but 65 mode so no Table Variables for now 🙁

  • I use table variable just for convenience.

    You can use temp tables as well.

    SET NOCOUNT ON

    CREATE TABLE#Sample

    (

    i INT,

    Done TINYINT

    )

    INSERT#Sample

    SELECT1, 0 UNION ALL

    SELECT2, 1 UNION ALL

    SELECT3, 0

    DECLARE @i INT

    SELECT@i = MIN(i)

    FROM#Sample

    WHEREDone = 0

    WHILE @i IS NOT NULL

    BEGIN

    PRINT'Now processing record' + STR(@i, 2)

    UPDATE#Sample

    SETDone = 1

    WHEREi = @i

    SELECT@i = MIN(i)

    FROM#Sample

    WHEREDone = 0

    END

    DROP TABLE#Sample

    Also tested on both SQL Server 2000 sp4 and SQL Server 2005 sp2.


    N 56°04'39.16"
    E 12°55'05.25"

  • It looks to me like the only reason you need some sort of cursor/loop is because your stored procedure is designed to work on one row at a time. Why not re-write your SP to run its operation against the entire SET of data? This should eliminate the need for a cursor/loop altogether and in most instances, prove to be faster and more efficient that the cursor/loop.

    John Rowan

    ======================================================
    ======================================================
    Forum Etiquette: How to post data/code on a forum to get the best help[/url] - by Jeff Moden

  • John Rowan (6/11/2008)


    It looks to me like the only reason you need some sort of cursor/loop is because your stored procedure is designed to work on one row at a time. Why not re-write your SP to run its operation against the entire SET of data? This should eliminate the need for a cursor/loop altogether and in most instances, prove to be faster and more efficient that the cursor/loop.

    Second that.

    From your code, the main part is:

    *****************

    SELECT @PNo = min(PNo) FROM #PT

    WHERE PDone = 0

    UPDATE #PT SET PDone = 1 WHERE PNo = @PNo

    UPDATE PT SET PDone = 1 WHERE PNo = @PNo

    ********************

    So, why not use:

    UPDATE PT SET PDone = 1 WHERE PDone = 0 ???

Viewing 15 posts - 1 through 15 (of 23 total)

You must be logged in to reply to this topic. Login to reply