Adding whitespace to your code makes it clearer what you're doing:
declare
cursor bowl_avg is
select sid, matches, bowling_avg, wickets
from bowling_stats;
nsid bowling_stats.sid%type;
nmatches bowling_stats.matches%type;
nbowling_avg bowling_stats.bowling_avg%type;
nwickets bowling_stats.wickets%type;
begin
-- 1. Open Cursor
open bowl_avg;
-- 2. Check if Cursor is open
if bowl_avg%isopen then
-- 3. Loop
loop
-- 4. Get record
fetch bowl_avg into nsid, nmatches, nbowling_avg, nwickets;
-- 5. Exit if no records left
exit when bowl_avg%notfound;
-- 6. If there is a record
if bowl_avg%found then
-- 7. Loop
loop
update bowling_stats
set bowling_avg = nwickets / nmatches
where sid = nsid;
-- 8. Exit if there is no record.
exit when bowl_avg%notfound;
end loop;
end if;
end loop;
else
dbms_output.put_line('unable to open cursor');
end if;
close bowl_avg;
end;
/
There are a number of contradictions in there.
- In 1 and 2 you're opening a cursor and then checking if there is an open cursor. A error will be raised if the cursor didn't open so you can ignore this step.
- In 5 and 6 you exit if you can't fetch a new record then check if you have a record. This is a contradiction so stage 6 will (almost) always evaluate to true.
- in 7 and 8 you loop, exiting when you don't have a record. As you've just checked (twice) that you do in fact have a record you'll never exit this loop.
If you insist on doing this with cursors then you can remove most of your code and it should work fine:
declare
cursor bowl_avg is
select sid, matches, bowling_avg, wickets
from bowling_stats;
nsid bowling_stats.sid%type;
nmatches bowling_stats.matches%type;
nbowling_avg bowling_stats.bowling_avg%type;
nwickets bowling_stats.wickets%type;
begin
-- 1. Open Cursor
open bowl_avg;
-- 2. Loop
loop
-- 3. Get record
fetch bowl_avg into nsid, nmatches, nbowling_avg, nwickets;
-- 4. Exit loop if there is no record.
exit when bowl_avg%notfound;
-- 5. Perform UPDATE statement.
update bowling_stats
set bowling_avg = nwickets / nmatches
where sid = nsid;
end loop;
close bowl_avg;
end;
/
As always a single UPDATE statement without using loops, cursors or PL/SQL will be significantly more effective.
