CODEWITHSUNDEEP
oracle-databasestored-proceduresplsqlcursordynamic-sql
AmateurOracleUser
AmateurOracleUser

Reputation: 21

ORA-00900: invalid SQL statement for Oracle Procedure

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;

Upvotes: 1

Views: 4698

Answers (2)

APC
APC

Reputation: 146209

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.

Upvotes: 2

Avrajit Roy
Avrajit Roy

Reputation: 3303

A much simpler approach with just FOR loop. We dont require to Open Close cursor in this case as this is internally handled by Oracle. Also I do not understand the need of updating Client ID again. If we are selecting the Client ID in where Clause there is no significance of Updating. Anyhow Enjoy :)

CREATE OR REPLACE
PROCEDURE RESETUSERSESSION(
    run IN VARCHAR2)
AS
BEGIN
  FOR vItems IN
  (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
  )
  LOOP
    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;
END;
/

Upvotes: 0

Related Questions