CODEWITHSUNDEEP
sqloracle-databasestored-proceduresdynamic-sql
shmail
shmail

Reputation: 31

Oracle stored procedure: FOR LOOP over a column from a variable

I want to save a list of column values into the variable input_cols and then loop over the values

CREATE OR REPLACE PROCEDURE map_values (mapping_table VARCHAR2(64)) AS
TYPE col_names IS TABLE OF VARCHAR2(64);
input_cols col_names;
BEGIN
    EXECUTE IMMEDIATE
        'SELECT COLUMN_NAME FROM SYS.DBA_TAB_COLUMNS 
        WHERE TABLE_NAME = '' ' || mapping_table || ' '' '
    BULK COLLECT INTO (input_cols);
    FOR in_col IN input_cols
    LOOP 
        dbms_output.put_line ('test');
    END LOOP;
END;

I am getting the error

PLS-00103: Encountered the symbol "LOOP" when expecting one of the following: * & - + / at mod remainder rem .. <an exponent (**)> ||

Upvotes: 0

Views: 1138

Answers (3)

William Robertson
William Robertson

Reputation: 15991

Although you can construct dynamic queries by concatenating values, it's generally better to use bind variables where possible, for example:

execute immediate
    'select column_name from user_tab_columns where table_name = :b1'
    bulk collect into input_cols
    using p_table;

I recommend getting into the habit of anchoring types in your code to the corresponding database object, when there is one. For example, this:

mapping_table dba_tab_columns.table_name%type

instructs the compiler to look up the type of dba_tab_columns.table_name and use that. However, I would generally avoid the dba_ views in procedures like this and stick to user_ views, e.g. user_tab_columns, to limit them to objects you own. If you must use dba_ views, you should also include the table owner, as there may be more than one table with the same name.

I also prefer to name my parameters in a way that separates them from column names etc. There are various conventions (camelCase, prefixing with i_ for in or p_ for parameter, prefixing with the procedure name e.g. map_values.mapping_table), so pick one you like.

Putting that together, you get something like this:

create or replace procedure map_values
    ( p_table user_tab_columns.table_name%type )
as
    type col_names is table of user_tab_columns.column_name%type;
    input_cols col_names;
begin
    execute immediate
        'select column_name from user_tab_columns where table_name = :b1 order by column_id'
        bulk collect into input_cols
        using p_table;

    for i in 1..input_cols.count loop 
        dbms_output.put_line(input_cols(i));
    end loop;
end map_values;

Or, if you don't specifically need a collection and just want to loop through a result set:

create or replace procedure map_values
    ( p_table user_tab_columns.column_name%type )
as
    columns_cur sys_refcursor;
    colname user_tab_columns.column_name%type;
begin
    open columns_cur for
        'select column_name from user_tab_columns where table_name = :b1 order by column_id'
        using p_table;

    loop 
        fetch columns_cur into colname;
        exit when columns_cur%notfound;
        dbms_output.put_line(colname);
    end loop;
    
    close columns_cur;
end;

As Koen pointed out in the comments, though, there is no need for dynamic SQL in this example, so a much simpler version could be just:

create or replace procedure map_values
    ( p_table user_tab_columns.column_name%type )
as
begin
    for r in (
        select column_name from user_tab_columns
        where  table_name = p_table
        order by column_id
    )
    loop
        dbms_output.put_line(r.column_name);
    end loop;
end map_values;

Upvotes: 2

Petr
Petr

Reputation: 540

Please do not use dynamic SQL for this use case, static SQL is sufficient and will be more efficient.

For looping, you have to use indexes and get the element from collection by index. There is no functionality that will enable you to directly assign collection element to a variable (like you are trying).

create or replace procedure map_values(mapping_table sys.dba_tab_columns%table_name)
as
  type col_names is table of sys.dba_tab_columns.column_name%type;
  input_cols col_names;
begin
  select column_name
  bulk collect into input_cols
  from sys.dba_tab_columns
  where table_name = mapping_table;
  
  for i in 1 .. input_cols.count
  loop
    dbms_output.put_line(input_cols(i));
  end loop;
end;

Upvotes: 0

Koen Lostrie
Koen Lostrie

Reputation: 18630

Word of advice: use a tool like SQL Developer to create your procedures. They show the compilation errors in a much clearer way. If you're new to PL/SQL, start with the very basics (empty procedure), compile, fix error if any and add code. There are 3 blocking issues in your code - debugging that is pretty hard.

I added a comment for each of the errors

create or replace PROCEDURE map_values 
(mapping_table VARCHAR /* just define the datatype, not the precision */
)
AS
TYPE col_names IS TABLE OF VARCHAR2(64) INDEX BY BINARY_INTEGER;
input_cols col_names;
BEGIN
    EXECUTE IMMEDIATE
        'SELECT COLUMN_NAME FROM SYS.DBA_TAB_COLUMNS 
        WHERE TABLE_NAME = ''' ||mapping_table|| ''' '
    BULK COLLECT INTO input_cols; /* no brackets needed */
        dbms_output.put_line ('test:');

    FOR in_col IN 1 .. input_cols.COUNT /* this is not a implicit cursor but a collection - you need to iterate over it.*/
    LOOP 
        dbms_output.put_line ('test:'||input_cols(in_col));
    END LOOP;
END;
/

Upvotes: 1

Related Questions