PL/SQL: “00904. 00000 - ”%s: invalid identifier"

Question

Hi I have the following code:

CREATE OR REPLACE PROCEDURE instead_of_select(
where_in_sname IN VARCHAR2,
where_in_city IN VARCHAR2)
IS
BEGIN
    EXECUTE IMMEDIATE
    'DECLARE rowss  SAL%ROWTYPE;
        BEGIN SELECT * INTO rowss from SAL where SNAME=' || where_in_sname || 
        ' and CITY='|| where_in_city ||';
    END;';
END instead_of_select;

BEGIN 
    instead_of_select('Peel', 'London');
END;    

BUUT I'm not able to figure out what its not liking when I run this. It gives me the following error

PL/SQL: ORA-00904: "LONDON": invalid identifier ORA-06550: line 2, column 11: PL/SQL: SQL Statement ignored ORA-06512: at "SYSTEM.INSTEAD_OF_SELECT", line 7 ORA-06512: at line 2 06550. 00000 - "line %s, column %s:\n%s" *Cause: Usually a PL/SQL compilation error.

Table exists with data in right, what problem could there be?

omg it's make me crazy

Answer 1

The arguments you're passing in are strings, and you are concatenating those into your dynamic query; but without quotes, so the inner query ends up as:

SELECT * INTO rowss from SAL where SNAME=Peel and CITY=London;

You could add the quotes around those values (but they have to be escaped):

EXECUTE IMMEDIATE
'DECLARE rowss  SAL%ROWTYPE;
    BEGIN SELECT * INTO rowss from SAL where SNAME=''' || where_in_sname || 
    ''' and CITY='''|| where_in_city ||''';
END;';

which would instead generate:

SELECT * INTO rowss from SAL where SNAME='Peel' and CITY='London';

But it's much better to use bind variables:

EXECUTE IMMEDIATE
'DECLARE rowss  SAL%ROWTYPE;
    BEGIN SELECT * INTO rowss from SAL where SNAME=:where_in_sname' || 
    ' and CITY=:where_in_city' USING where_in_sname, where_in_city;
END;';

The rowss variable only exists within that dynamic statement though, the procedure and its caller can't see the results.

This doesn't need to by dynamic at all anyway; you can just do:

CREATE OR REPLACE PROCEDURE instead_of_select(
    where_in_sname IN VARCHAR2,
    where_in_city IN VARCHAR2)
IS
    rowss SAL%ROWTYPE;
BEGIN
    SELECT * INTO rowss
    from SAL
    where SNAME=where_in_sname
    and CITY= where_in_city;
END instead_of_select;

But the caller still can't see rowss. It isn't clear what you expect or want to happen to the results - possibly you want to pass them back as an OUT variable, as a record or perhaps as a ref cursor. (Presumably the filter makes the result unique, otherwise you'd get a too-many-rows error; you can still get no-data-found.)

Answer 2

Here's a suggestion to help you debug this kind of thing yourself. Instead of trying to build the big, concatenated, dynamic string as part of your EXEC IMMEDIATE, assign it to a variable, then use dbms_output.put_line to show you exactly what you built, then simply EXEC IMMEDIATE the variable.

CREATE OR REPLACE PROCEDURE instead_of_select(
where_in_sname IN VARCHAR2,
where_in_city IN VARCHAR2)
IS
rowss  SAL%ROWTYPE
v_sql varchar2(4000);
BEGIN
    v_sql := 'SELECT * INTO rowss from SAL where SNAME=' || where_in_sname || 
        ' and CITY='|| where_in_city ';
    dbms_output.put_line(v_sql);
     EXECUTE IMMEDIATE v_sql;
END instead_of_select;

Above, I'm just showing a better way of setting up and running (and debugging) EXEC IMMEDIATE statements. I did not address the need to escape the quotes if you don't use bind variables. Also note that the dbms_output line is for debugging purposes only. You'd comment that out once you've resolved the issue.