Saturday, January 13, 2007

Short loop

It's good to see a GOTO every now and then. However, the sender of this one (thanks!) was most impressed by the loop that goes from 1 to v_totalcntr, and indeed the fact that there is a loop and a v_totalctr variable at all, with the variable carefully set from the cursor's %ROWCOUNT, when it can only ever have one value:

DECLARE
  CURSOR cur_pricing IS
      SELECT col1, col2
      FROM   sometable;

  var_pricing cur_pricing%ROWTYPE;

BEGIN
  OPEN cur_pricing;
  FETCH cur_pricing INTO var_pricing;

  IF cur_pricing%NOTFOUND THEN
      GOTO continue;
  END IF;

  v_totalcntr := cur_pricing%ROWCOUNT;

  FOR r IN 1..v_totalcntr
  LOOP
      -- loads of stuff here
      -- but no fetch from cur_pricing
      -- not even for the one time this loop will execute :-)
  END LOOP;

  <<continue>>
  NULL;
END;

Of course you could just fetch the value you want and process it, but where would be the fun in that?

PS The person who sent this in emailed me with a point I must admit hadn't occurred to me:

Saw you posted this one - thanks. But did you pick up on what they probably thought they were doing? I think whoever wrote it thought that %rowcount would have the TOTAL number of rows that the cursor would return - so they thought they would be looping around ALL the records in the cursor. The fact that they also forgot to fetch again in the loop just adds to the problem of course.

2 comments:

Anonymous said...

Is the missing CLOSE cur_pricing; line an editorial liberty or does this code crash on every call after the first? If the latter, does the application silently discard (or log) the error, or do we have reason to believe this code is never used by the application?

Either way, WTF?

William Robertson said...

No, the original code does not bother close the cursor as it should, but it gets away with it because of the default behaviour of PL/SQL in closing cursors when the function completes.