Implicit Cursors are Wonderful

I was sent this script for review. Can you see what's wrong with this picture?

DECLARE
  l_limit CONSTANT PLS_INTEGER := 1000;
  
  TYPE varchar2_tab_typ IS TABLE OF VARCHAR2(100) INDEX BY PLS_INTEGER;
  
  varchar2_tab   varchar2_tab_typ;
  
  CURSOR grants_cur IS
   SELECT 'GRANT SELECT ON '||table_name||' TO some_schema'
   FROM   user_tables;
   
BEGIN
  varchar2_tab.DELETE;
  
  OPEN grants_cur;
  
  LOOP
    FETCH grants_cur BULK COLLECT INTO varchar2_tab LIMIT l_limit;
    
    EXIT WHEN grants_cur%NOTFOUND;
  END LOOP;
  
  CLOSE grants_cur;
  
  FOR i IN 1..varchar2_tab.COUNT LOOP
    EXECUTE IMMEDIATE varchar2_tab(i);
  END LOOP;
END;
/

Did you catch the error? The script is meant to grant select on all of the tables in the current schema to another user. At first glance it looks like it should work, but upon closer inspection you'll realize that it will only run the grant statements for the last set of rows retrieved by the bulk collect. The associative array varchar2_tab is reinitialized (for lack of a better term) for each iteration of the first loop, and contains only the records retrieved by that fetch! If the number of tables was less than or equal to the bulk collect limit, no problem. If it's greater, big problem. Oops!

Using a sample schema with 12 tables, changing the bulk collect limit to 5, and using DBMS_OUPUT instead of an EXECUTE IMMEDIATE, I'll show you what I mean.

SELECT table_name FROM user_tables;

TABLE_NAME
------------------------------
PARTS
NEW_SALARY_INFO
DEPARTMENTS
EMPLOYEES
DEPARTMENT_ERRORS
INVOICES
PAYMENTS
DEPTS_AND_EMPS
T1
T2
BIG_TABLE
BIGGER_TABLE

12 rows selected.

DECLARE
  l_limit CONSTANT PLS_INTEGER := 5;

  TYPE varchar2_tab_typ IS TABLE OF VARCHAR2(100) INDEX BY PLS_INTEGER;

  varchar2_tab   varchar2_tab_typ;

  CURSOR grants_cur IS
   SELECT 'GRANT SELECT ON '||table_name||' TO some_schema'
   FROM   user_tables;

BEGIN
  varchar2_tab.DELETE;

  OPEN grants_cur;

  LOOP
    FETCH grants_cur BULK COLLECT INTO varchar2_tab LIMIT l_limit;

    EXIT WHEN grants_cur%NOTFOUND;
  END LOOP;

  CLOSE grants_cur;

  FOR i IN 1..varchar2_tab.COUNT LOOP
    DBMS_OUTPUT.PUT_LINE(varchar2_tab(i));
    --EXECUTE IMMEDIATE varchar2_tab(i);
  END LOOP;
END;
/
GRANT SELECT ON BIG_TABLE TO some_schema
GRANT SELECT ON BIGGER_TABLE TO some_schema

PL/SQL procedure successfully completed.

As you can see, only two grants would have been run.


In cases like this, I much prefer using implicit cursors especially since Oracle will silently bulk collect them for you. They are easier to write, require much less code, there's no need to explicity open/fetch/close, and have a much higher degree of clarity in my opinion. As a result, I find them to be far less error prone.

DECLARE
BEGIN
  FOR rec IN (
              SELECT 'GRANT SELECT ON '||table_name||' TO some_schema' grant_sql
              FROM   user_tables
             ) LOOP
    DBMS_OUTPUT.PUT_LINE(rec.grant_sql);
  END LOOP;
END;
/
GRANT SELECT ON PARTS TO some_schema
GRANT SELECT ON NEW_SALARY_INFO TO some_schema
GRANT SELECT ON DEPARTMENTS TO some_schema
GRANT SELECT ON EMPLOYEES TO some_schema
GRANT SELECT ON DEPARTMENT_ERRORS TO some_schema
GRANT SELECT ON INVOICES TO some_schema
GRANT SELECT ON PAYMENTS TO some_schema
GRANT SELECT ON DEPTS_AND_EMPS TO some_schema
GRANT SELECT ON T1 TO some_schema
GRANT SELECT ON T2 TO some_schema
GRANT SELECT ON BIG_TABLE TO some_schema
GRANT SELECT ON BIGGER_TABLE TO some_schema

PL/SQL procedure successfully completed.