Better looping - replace for cursor

  • Recently we replaced cursors with while loop and table variables.

    The block is something similar to

    [Code]

    Declare @tablevar.

    insert into @tablevar select stmt

    select top 1

    while ( @@rowcount <> 0 )

    begin

    processing..

    delete the row

    select top 1

    end

    [/code]

    is there any better way, now the sp is performing very slow.

    Thanks for your time

  • krishnaroopa (1/13/2011)


    Recently we replaced cursors with while loop

    Ahh , you fell for that one , it is a myth that replacing cursors with a while loop is a good idea.

    You need to replace the whole loop with set based logic.

    Post the whole code and i'll take a look.



    Clear Sky SQL
    My Blog[/url]

  • Dave Ballantyne (1/13/2011)


    krishnaroopa (1/13/2011)


    Recently we replaced cursors with while loop

    Ahh , you fell for that one , it is a myth that replacing cursors with a while loop is a good idea.

    You need to replace the whole loop with set based logic.

    Post the whole code and i'll take a look.

    He's right Loops are slow

    It's better if you can do it one query

    not a loop

    The database structure should allow it

  • [Code]

    declare @tablevar table ( tvid int, clause varchar(100), clauserank tinyint, val int )

    insert into @tablevar select * from vallookup

    select top 1 @tivid=tvid, @clause=clause, @clauserank=clauserank, @val=val from @tablevar

    while ( @@rowcount <> 0 )

    begin

    IF @clause IS NOT NULL AND LTRIM(@clause) <> ''

    BEGIN

    SET @clauseSID = NULL;

    SET @clauseSQL = N'SELECT @@clauseSID=id from tb2 where ' + @clause

    EXECUTE dbo.sp_executesql @clauseSQL, N'@clauseSID bigint OUTPUT', @clauseSID OUTPUT

    IF @clauseSID = @inputid

    BREAK

    END

    ELSE BREAK

    delete from @tablevar where tvid = @tvid

    select top 1 @tivid=tvid, @clause=clause, @clauserank=clauserank, @val=val from @tablevar

    end

    [/code]

    The thing is, for each row, get the clause and check whether it satisfies. If yes, break out of the loop else continue. 🙁

  • Yuck, that is a horrible idea.

    You should be able to join the conditions logically or'ing them by this technique

    http://www.sqlservercentral.com/articles/comma+separated+list/71700/

    Then execute the whole statement as a whole with 'top 1 '



    Clear Sky SQL
    My Blog[/url]

  • But here the clauses are ordered by clause rank. If the highest rank does not satisfy only, we will move to the next clause.

    Also, I should know the row which clause is satisfied.

    If this is resolved.. it will really solve a lot of issues for me.

  • krishnaroopa (1/13/2011)


    Recently we replaced cursors with while loop and table variables.

    You didn't really. Looping over a tableset is a cursor in disguise.

  • The issue seems to me to be not the performance of the looping per-se , but the execution speed of the dynamic sql that is being built.

    Would that be a fair assumption ?

    In that case you need to ensure that all possible condition are supported by indexes etc...

    You could do the ranking by using the condition in a case statement.

    This is all sounding like a very very bad idea though...



    Clear Sky SQL
    My Blog[/url]

  • Dave Ballantyne (1/13/2011)


    krishnaroopa (1/13/2011)


    Recently we replaced cursors with while loop

    Ahh , you fell for that one , it is a myth that replacing cursors with a while loop is a good idea.

    You need to replace the whole loop with set based logic.

    Post the whole code and i'll take a look.

    Basically, if the code utilizes the keyword "WHILE", then it is a form of a cursor that should be updated. They are both loop-based, Row-By-Agonizing-Row (RBAR) processing that should be avoided.

    Wayne
    Microsoft Certified Master: SQL Server 2008
    Author - SQL Server T-SQL Recipes


    If you can't explain to another person how the code that you're copying from the internet works, then DON'T USE IT on a production system! After all, you will be the one supporting it!
    Links:
    For better assistance in answering your questions
    Performance Problems
    Common date/time routines
    Understanding and Using APPLY Part 1 & Part 2

  • Dave Ballantyne (1/13/2011)


    Yuck, that is a horrible idea.

    You should be able to join the conditions logically or'ing them by this technique

    http://www.sqlservercentral.com/articles/comma+separated+list/71700/

    Then execute the whole statement as a whole with 'top 1 '

    Thanks for the plug Dave! 😀

    Wayne
    Microsoft Certified Master: SQL Server 2008
    Author - SQL Server T-SQL Recipes


    If you can't explain to another person how the code that you're copying from the internet works, then DON'T USE IT on a production system! After all, you will be the one supporting it!
    Links:
    For better assistance in answering your questions
    Performance Problems
    Common date/time routines
    Understanding and Using APPLY Part 1 & Part 2

  • i feel your pain, i also once worked on a place that had stuff like this.

    apart from changing how the system works to get away from that loop, the best you can do is optimize each clause with good indexing

    most times changing the system to get away from that situation is the best solution but not the chosen one

    --
    Thiago Dantas
    @DantHimself

  • As my name suggests i dont totally agree. It depends where you work at and who or what team will be supporting it. On occasion these guys tend to provide solutions that do indeed work and are stellar as far as performance however if the guy or team who is supporting it doesnt understand it because it is too complex then it does not good. There is a balance there. Performance and readability and it's relative to the people u work w/ skill set. If their converted front end developers then i find it best to use loop functionality because they can more readily understand it. If your writting the code for banking software or a site who does 20 billion in online sales yearly then you need to trash the loop and go w the most efficient way and be damned w the readability. Dont be that guy who over complexifies a simle problem. It's a balance that needs to be met.

    I'm just sayin....

  • sorry. i do agree w the indexing just not the loop haters out there

  • @BaldingLoopMan: Loop code has its place, but when the OP specifically states things are taking too long, it's time for that method to move on. In general: if they're switching systems, they're switching languages. It's part of the reason that coders make bad DBAs and DBAs make bad coders. It's a completely different way of thinking to do either well. Alright, I'll get off my soapbox.

    krishnaroopa (1/13/2011)


    But here the clauses are ordered by clause rank. If the highest rank does not satisfy only, we will move to the next clause.

    Also, I should know the row which clause is satisfied.

    If this is resolved.. it will really solve a lot of issues for me.

    Krishna: Your cursor/loop code above does not have an order by in it, thus, there is no ordering by clause rank. You're grabbing them in whatever order the system presents them to you. I'll shoot one item in the foot before it's stated: The clustered index does not guarantee the order. You need an ORDER BY clause if you want them in a specific ordering.

    I assume @inputID is passed from the CREATE PROC parameters? I didn't see it defined anywhere in your code, though you're checking for it. You're also not grabbing a single row, if you are checking for a single ID occurence against your rules from valLookup.

    You might change your SQL to SET @clauseSID = CASE WHEN EXISTS( SELECT id FROM tb2 WHERE ' + @clause + ' AND id = ' + CAST( @inputID AS VARCHAR(10)) + ' THEN 1 ELSE 0 END'

    That will restrict it to just the ID in question being tested and should get you some further speed.

    Without seeing the rules hiding under @clause and the proc in its entirety (I assume tb2 is NOT the actual table name there...), we won't be able to assist you with a rebuild.


    - Craig Farrell

    Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.

    For better assistance in answering your questions[/url] | Forum Netiquette
    For index/tuning help, follow these directions.[/url] |Tally Tables[/url]

    Twitter: @AnyWayDBA

  • BaldingLoopMan (1/14/2011)


    sorry. i do agree w the indexing just not the loop haters out there

    Not to worry... I only hate slow loops... 😉

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

    Change is inevitable... Change for the better is not.


    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)

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

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