CODEWITHSUNDEEP
stored-proceduresplsqloracle10g
sam
sam

Reputation: 405

trouble with cursor inside procedure

CREATE TABLE TEST_CASE
  (
    ID NUMBER(19,2),
    CURRENCY_TYPE VARCHAR2(30),
    PAIDAMT NUMBER(19,2),
    RECVDAMT NUMBER(19,2),
    AMTDUE NUMBER,
    TRANSACTION_DATE VARCHAR2(30)
  );

I've created a procedure to fetch the fields which have AMT in their name. But the procedure shows error while executing, I can not figure out why is this error generating.

create or replace procedure chk_amt
(
    vtbl varchar2
)
as
tblcursor sys_refcursor;
tblsqlstr varchar2(1000);
importedrows VARCHAR2(1000); 
BEGIN
    tblsqlstr := 'Select COLUMN_NAME from user_tab_columns where table_name= '|| vtbl ||' and COLUMN_NAME like upper(''%AMT%'')' ;   
    OPEN tblcursor for tblsqlstr;
loop
fetch tblcursor into importedrows;
DBMS_OUTPUT.PUT_LINE(importedrows); 
EXIT WHEN tblcursor%NOTFOUND;
end loop;
CLOSE tblcursor;
end;
/

error is

ORA-00904: "TEST_CASE": invalid identifier
ORA-06512: at "***.CHK_AMT", line 11
ORA-06512: at line 2

How can I solve this error??

Upvotes: 0

Views: 121

Answers (2)

steve godfrey
steve godfrey

Reputation: 1234

Add some single quotes around vtbl to your dynamic statement , so it becomes :- 

tblsqlstr := 'Select COLUMN_NAME from user_tab_columns where table_name= '''|| vtbl ||''' and COLUMN_NAME like upper(''%AMT%'')' ;

This means that when your code is run, if vtbl has a value table_a, the statement that is actually run will be 'table_a' rather than table_a.

N.B. Take care not to make this procedure public, as it is vulnerable to sql injection. Ideally you should use a bind variable instead. Example below :-

CREATE OR REPLACE PROCEDURE chk_amt(vtbl VARCHAR2) AS
tblcursor    SYS_REFCURSOR;
tblsqlstr    VARCHAR2(1000);
importedrows VARCHAR2(1000);
BEGIN
tblsqlstr := 'Select COLUMN_NAME from user_tab_columns where table_name= :val_bnd and COLUMN_NAME like upper(''%AMT%'')';
OPEN tblcursor FOR tblsqlstr USING vtbl;
LOOP
    FETCH tblcursor
        INTO importedrows;
    dbms_output.put_line(importedrows);
    EXIT WHEN tblcursor%NOTFOUND;
END LOOP;
CLOSE tblcursor;
END;

Upvotes: 2

Justin Cave
Justin Cave

Reputation: 231651

Unless you have a particular reason for using dynamic SQL, it's much easier to use static SQL. It's also generally easier to use implicit cursors unless you have some reason that explicit cursors are advantageous. Since your code snippet doesn't show any need to use dynamic SQL or explicit cursors, it can be simplified to

create or replace procedure chk_amt
(
    vtbl varchar2
)
as
BEGIN
  FOR columns IN (SELECT column_name
                    FROM user_tab_columns
                   WHERE table_name = vtbl
                     AND column_name LIKE '%AMT%')
  LOOP
    DBMS_OUTPUT.PUT_LINE(columns.column_name);
  END LOOP; 
end;

The specific error you're getting is the result of concatenating the vtbl variable into your dynamic SQL statement. If you're going to concatenate the string together like that, you would need to put a single quote before and after the variable in the string and you would have to escape any single quotes in the table name (of course, it's likely there are any single quotes in a table name). If you must use dynamic SQL, you would be much better served using bind variables instead

tblsqlstr := 'Select COLUMN_NAME 
                from user_tab_columns 
                where table_name= :1 
                  and COLUMN_NAME like upper(''%AMT%'')' ;   
OPEN tblcursor for tblsqlstr using vtbl;

In addition to being more efficient and avoiding the potential for SQL injection attacks, that avoids the need to escape the data in your local variables and it avoids the need to add extra quotes to the string.

Upvotes: 2

Related Questions