views:

68

answers:

2

I have a cursor that is used to get some preliminary information for some other processing. It is possible that the query backing the cursor may not return any rows, and in these rare cases, we want to raise a special exception (handled and logged elsewhere so processing is not compeltely halted) so that the user knows about what is most likely bad input. Here's what it looks like:

open c_getPrs(in_pnum);
loop

    fetch c_getPrs
        into r_rpmRecord;            

     if c_getPrs%NOTFOUND then
       raise X_INVALID_PNUM;
    end if;

    exit when c_getPrs%rowcount > 1 /*or c_getPrs%NOTFOUND*/;           
end loop;
close c_getPrs;

The problem is that the if-statement ALWAYS executes so the exception is always raised, even when a row is returned. I'm not sure why. If there's a better way to handle this kind of logic, I'm open to that too ;)

+5  A: 

Hi,

your problem lies with your exit condition: on the first pass c_getPrms%rowcount is 1, so you get another pass which raises the exception.

Since you want only one fetch I would suggest the following construct:

OPEN c_getPrms(l_input);

FETCH c_getPrms
   INTO r_prmRecord;

IF c_getPrms%NOTFOUND THEN
   RAISE X_INVALID_PNUM;
END IF;

CLOSE c_getPrms;

I don't like explicit cursor much, so I will also suggest this synthax:

BEGIN
   SELECT ... 
     INTO r_prmRecord 
     FROM ... 
    WHERE ... AND rownum = 1; -- your cursor query
EXCEPTION
   WHEN no_data_found THEN
      RAISE X_INVALID_PNUM;
END;
Vincent Malgrat
+1 Great minds...
Tony Andrews
@Tony: *nods* :)
Vincent Malgrat
Interesting. Any reason you both prefer the implicit cursor over the explicit cursor? I was led to understand that explicit cursors tend to execute faster, but I've only been into PL/SQL for a month or so, so I'm still learning.
FrustratedWithFormsDesigner
@Frustrated: they are identical performance-wise. I find implicit cursors easier to read and therefore easier to maintain. See this SO for example: http://stackoverflow.com/questions/1577734/explicit-opening-and-closing-cursors/1577867#1577867
Vincent Malgrat
Ok, that's a good point. In this case, the query is rather long and nasty, so I think my procedure will be easier to read without the select statement cluttering it up. My IDE allows me to ctrl-click the cursor name and immediately navigate the the cursor definition. I also have a strong feeling this cursor may need to be used in a few more places (at one of them WILL have a real loop ;) ), so I think I'll keep it explicit for now.
FrustratedWithFormsDesigner
+5  A: 

Your code always goes round the loop twice, and so fails if there are less than 2 rows returned by the cursor. You probably don't need the loop at all:

open c_getPrms(in_pnum);

fetch c_getPrms
 into r_prmRecord;

if c_getPrms%NOTFOUND then
  raise X_INVALID_PNUM;
end if;

close c_getPrms;

I would prefer to avoid the cursor altogether, and use "select into" instead:

begin
   select ...
   into   r_prmRecord
   from   ...
   where  ...
exception
   when no_data_found then
      raise X_INVALID_PNUM;
end;

This will raise TOO_MANY_ROWS if the select returns more than 1 row. If you don't want that to happen, i.e. more than 1 row is OK, you could just add "AND ROWNUM = 1" to the query.

Tony Andrews
Heh that's an even better point. At one time the loop was actually necessary (we checked more than the first record) so I guess I didn't notice it was no longer needed.
FrustratedWithFormsDesigner