ORA-00900: invalid SQL statement for Oracle Procedure

Question

I am trying to execute my below procedure but kept getting error (ORA-00900: invalid SQL statement)

CREATE OR REPLACE PROCEDURE RESETUSERSESSION (run IN VARCHAR2)
IS    
    cursor usersessiondetail_cur IS    
          SELECT usd.CLIENTID,usd.OPERID,usd.REGISTER,usd.MACHINE_ID,usd.SESSIONNUMBER
          FROM cashiering_dev.CSH_USER usr, cashiering_dev.CSH_USERSESSIONDETAIL usd
          WHERE usr.clientid = usd.clientid 
          AND usr.operid = usd.operid
          AND usr.register = usd.register
          AND usr.machine_id = usd.machine_id 
          AND usr.sessionnumber = usd.sessionnumber
          AND usr.Machine_ID = 'basrytest'
          AND usd.LOGOFFDATETIME IS NULL;


 BEGIN
      OPEN usersessiondetail_cur;      

        FOR vItems in usersessiondetail_cur
        LOOP
           EXECUTE IMMEDIATE 'UPDATE csh_UserSessionDetail 
                         SET ClientID =vItems.CLIENTID 
                        WHERE ClientID =vItems.CLIENTID 
                         AND OperID =vItems.OPERID 
                         AND Register =vItems.REGISTER 
                         AND Machine_ID =vItems.MACHINE_ID 
                         AND SessionNumber =vItems.SESSIONNUMBER';                     
       END LOOP;

      CLOSE usersessiondetail_cur;



END;

When you are using cursor for loop do not open/close cursor explicitly (OPEN usersessiondetail_cur). Plus in this case there is absolutely no need to use dynamic SQL('UPDATE' statement). — Nick Krasnov
Are you getting the error trying to compile the procedure, or really when trying to execute it? Either way, how are you doing it - which client and command, etc? What is the full error stack you get? (And why are you using a loop at all, instead of just doing a single update? What is your run parameter for?) — Alex Poole

APC APC · Accepted Answer · 2016-12-28T05:07:05

Your SQL is invalid because the cursor projection names are not in scope when the dynamic SQL string is executed. You need to use placeholders like this:

   FOR vItems in usersessiondetail_cur
    LOOP
       EXECUTE IMMEDIATE 'UPDATE csh_UserSessionDetail 
                     SET ClientID = :p1
                    WHERE ClientID = :p2 
                     AND OperID = :p3
                     AND Register = :p4 
                     AND Machine_ID = :p5 
                     AND SessionNumber = :p6' 
           using vItems.CLIENTID 
                 , vItems.CLIENTID 
                , vItems.OPERID 
                , Items.REGISTER 
                , vItems.MACHINE_ID 
                , vItems.SESSIONNUMBER;                     
   END LOOP;

Your dynamic code is not an anonymous PL/SQL block or a CALL statement so parameters are passed by position not name, which means you must pass vItems.CLIENTID twice. Find out more.

Other observations

First and foremost, there is absolutely no need to implement dynamic execution for this SQL.
The OPEN and CLOSE cursor statements are not used with a FOR cursor loop.
You do not need an explicit cursor declaration for this query.
The row-by-agonising row UPDATE with a cursor loop is bad practice and needlessly inefficient compared to set-based UPDATE statement.
Your procedure doesn't use the run parameter ...
... but the cursor does have a hardcoded string for MACHINE_ID.
Lastly, the UPDATE statement doesn't actually change the state of the table, because it sets CLIENT_ID = CLIENT_ID, so the whole procedure is pointless.

Apart from that, everything is fine.

I assume you're writing this as a test for understanding how to use dynamic SQL rather than as an implementation of business logic. But even if it is a test it is better to write a proper piece of code which does something. Especially when you're sharing the code with others on StackOverflow. Posting code with so many issues is distracting because potential respondents don't know which to tackle.

ORA-00900: invalid SQL statement for Oracle Procedure

2 Answers