what''s wrong with my cursor!!!!

  • Hello

    I have empt table where job_id is field in it.

    I want to update empt table if job_id = '1' or '3' by '999'

    And if job_id not '1' or '3' then update by '456'

    So what's wrong with my cursor

    T.I.A

    Shashank

    -----------------------------------------------------

    CREATE procedure lnt.textt

    @qurstr  varchar(10)

    AS

    Declare @in AS Varchar(10)

    Declare crs_cc cursor for select job_id from empt FOR UPDATE OF job_id

    open crs_cc

    fetch next from crs_cc INTO @in

    while @@fetch_status = 0

    IF @qurstr = '1' OR @qurstr = '3'

     BEGIN

      UPDATE empt set job_id = '999'

      where job_id =  @qurstr

      print 'U Miss By Miles!!!!'

      print @qurstr

     END

     

    IF  @qurstr IN (SELECT job_id from empt where job_id not in ('1','3'))

     BEGIN

      update empt set job_id = '456'

      where job_id =  @qurstr

      print ' Success'

     END

    FETCH NEXT FROM crs_cc

    CLOSE crs_cc

    DEALLOCATE crs_cc

    GO


    Regards,

    Papillon

  • Why are you using a cursor for this??

    UPDATE empt

    SET job_id = CASE WHEN @qurstr IN ('1', '3') THEN '999' ELSE '456' END

    WHERE job_id = @qurstr

  • Hey chris !!!

    I got what i want

    Thank you so much

     

    Shashank

     


    Regards,

    Papillon

  • Great, happy to help. But why where you using a cursor in the first place? In SQL Server cursors are extremely seldomly necessary (if ever).

  • Hey!

    I just wanted to know how cursors are working here nothing more than that

    But I am getting one problem , I  created that procedure like below--------

    CREATE procedure lnt.textt

    @qurstr varchar(10)

    AS

    Begin

    Declare @in AS Varchar(1000)

    Declare crs_cc cursor for select job_id from empt

    open crs_cc

    fetch next from crs_cc INTO @in

    while @@fetch_status = 0

    IF @qurstr = '1' OR @qurstr = '3'

    BEGIN

    UPDATE empt set job_id = '999'

    where job_id = @qurstr

    print 'U Miss By Miles!!!!'

    print @qurstr

    END

    ELSE

    BEGIN

    update empt set job_id = '456'

    where job_id = @qurstr

    print ' Success'

    END

    FETCH NEXT FROM crs_cc

    CLOSE crs_cc

    DEALLOCATE crs_cc

    End

    -------

    In empt table there are hardly 15 records , and when I passed parameter for @qurstr , then this procedure has took more than 120 seconds!!!!(I donot know exactly what time that will , I cancelled execution) But why this much time it took??????

    And if possible can you tell me some articles where I will get information regarding best use of cursors in SQL SERVER 2000

    T.I.A

    Shashank


    Regards,

    Papillon

  • There's a few problems with this, other than the obvious that a cursor is in no way necessary here.

    You're never using the value you get from the cursor. I'm not sure if that's a mistake or intentional. The way it's written you will update the same record in empt repeatedly

    The second fetch statement doesn't specify a variable. (It should read the same as the first FETCH NEXT FROM ... INTO @...) Since you never use the variable it has no effect here but in a scenario when you actually use the value you would notice incorrect behaviour.

    The cursor has not been specified with any settings (static, keyset, etc) and since you are updatng the value that the cursor retrieves, there is a possibility of fetching the same record from the cursor many times. To avoid that, you could specify the cursor as STATIC.

    Always be very careful if inside a cursor you are updating/inserting/deleting a table that the cursor is based upon.

    Read through BoL for a very good overview of how cursors work.

    As for the best use, the best use of a cursor is not to use one at all. In 99% of the times I see people using cursors there was a set-base way that would have worked and worked much faster. In general cursors are slower and more resource intensive than an equivalent set-based solution.

    Gail Shaw
    Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
    SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

    We walk in the dark places no others will enter
    We stand on the bridge and no one may pass
  • also....not only is @in not being used at all - it is also declared as Varchar(1000) - so excessive for something that has no place in the procedure other than being "fetched into"....what did you intend to use it for shashank ?!

    ....and it would definitely help to change datatype of job_id to int..unless there are other values in this column that are string candidates...

    ..others will come along and repeat what chris/gila monster have told you..cursors are evil - if you're using a cursor, just post your code here and it'll be rendered cursorless in less than the time it takes for you to post it....just compare chris's update to the procedure you posted....doesn't the sheer brevity and elegance take your breath away ?!?!

    UPDATE empt
    SET job_id = CASE WHEN @qurstr IN ('1', '3') THEN '999' ELSE '456' END
    WHERE job_id = @qurstr
    







    **ASCII stupid question, get a stupid ANSI !!!**

  • Cursors ARE evil, but as for why it ran so long - you need to use WHILE...BEGIN...FETCH...END. Your while loops on the IF...ELSE blocks and never gets to the FETCH.

  • I can't believe I missed that....

    Gail Shaw
    Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
    SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

    We walk in the dark places no others will enter
    We stand on the bridge and no one may pass
  • Hello friends!!!

    Thanks for your help , the only biggest mistake i was doing in procedure was

    I write my fetch (fetch next from crs_cc INTO @in  ) statement before @@fetch_status = 0 so everytime it goes into loop and my execution didnot stop!!!!!!

    I realised with the help of you only

    Thanks through journey!!!

    Shashank


    Regards,

    Papillon

  • Nononono The location of the fetches is correct. You left out a necessary BEGIN... END construct. That's why the execution didn't stop.

    You must fetch before going into the loop or the variable @@fetch_status will have a meaningless value. Correct layout is as follows

    DECLARE cur .....

    OPEN cur

    FETCH NEXT FROM cur INTO variables

    WHILE @@FETCH_STATUS=0

    BEGIN

     -- Do stuff here

     FETCH NEXT FROM cur INTO variables

    END

    CLOSE cur

    Gail Shaw
    Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
    SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

    We walk in the dark places no others will enter
    We stand on the bridge and no one may pass
  • Hi gilamonster!

    ALTER PROCEDURE lnt.textt

    @qurstr VARCHAR(10)

    AS

    DECLARE @in AS VARCHAR(10)

    DECLARE crs_cc CURSOR FOR SELECT job_id FROM empt

    OPEN crs_cc

    --FETCH NEXT FROM crs_cc INTO @in

    WHILE @@fetch_status = 0

    FETCH NEXT FROM crs_cc INTO @in

    IF @qurstr = '1' OR @qurstr = '3'

     BEGIN

      UPDATE empt SET job_id = '999'

      WHERE job_id = @qurstr

      PRINT 'U Miss By Miles!!!!'

      PRINT @qurstr

     END

    ELSE

     BEGIN

      UPDATE empt SET job_id = '456'

      WHERE job_id = @qurstr

      PRINT ' Success'

      PRINT @qurstr

     END

    --FETCH NEXT FROM crs_cc INTO @in

    CLOSE crs_cc

    DEALLOCATE crs_cc

    See here i write FETCH statement after WHILE statement and comment the next FETCH statement and I got out put as I needed!!

    But now see below

    ALTER PROCEDURE lnt.textt

    @qurstr VARCHAR(10)

    AS

    DECLARE @in AS VARCHAR(10)

    DECLARE crs_cc CURSOR FOR SELECT job_id FROM empt

    OPEN crs_cc

    FETCH NEXT FROM crs_cc INTO @in

    WHILE @@fetch_status = 0

    --FETCH NEXT FROM crs_cc INTO @in

    IF @qurstr = '1' OR @qurstr = '3'

     BEGIN

      UPDATE empt SET job_id = '999'

      WHERE job_id = @qurstr

      PRINT 'U Miss By Miles!!!!'

      PRINT @qurstr

     END

    ELSE

     BEGIN

      UPDATE empt SET job_id = '456'

      WHERE job_id = @qurstr

      PRINT ' Success'

      PRINT @qurstr

     END

    FETCH NEXT FROM crs_cc INTO @in

    CLOSE crs_cc

    DEALLOCATE crs_cc

    Here I write as you told me! and I am getting same output as above!

    But sometimes it took unlimited time for above execution!!!!

    Tell me what is difference between above two executions?

    What they will cause?

    T.I.A

    Shashank


    Regards,

    Papillon

  • You missed my point. If you don't FETCH before going into the WHILE, you don't know the value of @@FETCH_STATUS since it is GLOBAL for ALL cursors on a connection.

    You need a BEGIN...END block around ALL the statements that need to be inside the loop as a WHILE will only loop the single command or command block that immedatly follows it.

    See my changes (in red) below for something that will work as intended, everytime.

    ALTER PROCEDURE lnt.textt

    @qurstr VARCHAR(10)

    AS

    DECLARE @in AS VARCHAR(10)

    DECLARE crs_cc CURSOR FOR SELECT job_id FROM empt

    OPEN crs_cc

    FETCH NEXT FROM crs_cc INTO @in

    WHILE @@fetch_status = 0

    BEGIN

     IF @qurstr = '1' OR @qurstr = '3'

      BEGIN

       UPDATE empt SET job_id = '999'

       WHERE job_id = @qurstr

       PRINT 'U Miss By Miles!!!!'

       PRINT @qurstr

      END

     ELSE

      BEGIN

       UPDATE empt SET job_id = '456'

       WHERE job_id = @qurstr

       PRINT ' Success'

       PRINT @qurstr

      END

    FETCH NEXT FROM crs_cc INTO @in

    END

    CLOSE crs_cc

    DEALLOCATE crs_cc

    The BEGIN...END I added means that both the IF and the FETCH are within the WHILE. The way you changed it, with the FETCH after the WHILE, only the FETCH would loop and your data mod statement wouldn't.

    Please, please read Books Online for details on cursors and execution flow.

    Also, previous comments still apply. You're not using the output from the cursor at all (@in). Why?

    A cursor is not required here at all. I know you said you're learning cursors but why don't you try and write something that requires a cursor.

    Try playing with the following to see how cursors behave.

    DECLARE @tblName VARCHAR(128)

    DECLARE @i INT

    DECLARE curTables CURSOR LOCAL FORWARD_ONLY READ_ONLY FOR

     SELECT name FROM sysobjects WHERE xtype='u' ORDER by crdate

    OPEN curTables

    FETCH NEXT FROM curTables INTO @tblName

    SET @i=1

    WHILE @@FETCH_STATUS=0

     BEGIN

      PRINT 'Table ' + CAST(@i AS VARCHAR(3)) + ': ' + @tblName

      FETCH NEXT FROM curTables INTO @tblName

     SET @i=@i+1

     END

    CLOSE curTables

    DEALLOCATE curTables

    Gail Shaw
    Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
    SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

    We walk in the dark places no others will enter
    We stand on the bridge and no one may pass
  • Hey Gilamonster!!

    I got the things!!!

    That makes me smile

    Thanks

    Regards

    Shashank


    Regards,

    Papillon

Viewing 14 posts - 1 through 13 (of 13 total)

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