21

I am writing a PL/SQL Procedure that performs a select based on input variables and then inserts a row for each result in the select. I am having trouble debugging what is wrong with my query due my newness to PL/SQL. I know this must be easy, but I am stuck here for some reason. Thanks for your help!

CREATE OR REPLACE PROCEDURE setup_name_map(ranking_id IN NUMBER, class_string IN VARCHAR2) 
IS
BEGIN

    FOR rec IN (SELECT NAME_ID FROM PRODUCT_NAMES WHERE NAME = class_string)
    LOOP
        EXECUTE IMMEDIATE 'INSERT INTO NAME_RANKING (NAME_ID, RANKING_ID) VALUES (' || rec.NAME_ID || ', ' || ranking_id || ')';
    END LOOP;
END;

According to the Oracle Developer Compiler... 'NAME_ID' is an invalid identifier. I've tried putting it in quotes but no dice. It also complains that loop index variables 'REC' use is invalid. Any help is much appreciated.

Craig
  • 1,295
  • 3
  • 16
  • 29

1 Answers1

54

There is no need for dynamic SQL here:

BEGIN

    FOR rec IN (SELECT NAME_ID FROM PRODUCT_NAMES
                WHERE NAME = class_string)
    LOOP
        INSERT INTO NAME_RANKING (NAME_ID, RANKING_ID)
        VALUES (rec.NAME_ID, ranking_id);
    END LOOP;
END;

Better still you can avoid a slow row-by-row cursor approach like this:

BEGIN
    INSERT INTO NAME_RANKING (NAME_ID, RANKING_ID)
    SELECT NAME_ID, ranking_id FROM PRODUCT_NAMES
    WHERE NAME = class_string;
END;

If you really did need the dynamic SQL you should not be concatenating values into it, but using bind variables:

BEGIN

    FOR rec IN (SELECT NAME_ID FROM PRODUCT_NAMES
                WHERE NAME = class_string)
    LOOP
        EXECUTE IMMEDIATE 'INSERT INTO NAME_RANKING 
                           (NAME_ID, RANKING_ID) VALUES (:b1, :b2)
        USING rec.NAME_ID, ranking_id;
    END LOOP;
END;
jswolf19
  • 2,303
  • 15
  • 16
Tony Andrews
  • 129,880
  • 21
  • 220
  • 259