Skip to Main Content

Breadcrumb

Question and Answer

Chris Saxon

Thanks for the question, Joachim.

Asked: March 17, 2021 - 3:47 pm UTC

Last updated: March 17, 2021 - 4:44 pm UTC

Version: 19.3

Viewed 1000+ times

You Asked

Hi Tom,
my question is regarding changing and commit some data inside a function and if this is a "recommended" way or how to do it in a better way.
I have a function with a cursor and a loop and inside the loop, we change some data with regards to the loop condition.

The DDL is like:
FUNCTION ProcessMessages (ParBusyLastProcessing    OUT INTEGER,
                          ParErrorText             OUT VARCHAR2)
RETURN INTEGER
IS
  ConstFunction CONSTANT VARCHAR2(40) := 'ProcessMessages';
  LocRc                   INTEGER;
  LocInfoText             VARCHAR2(2000);
  LocCountUnprocessedMsg  INTEGER;
BEGIN
  LocRc           := 0;
  LocInfoText     := ' ';
  ParErrorText    := ' ';
  ParBusyLastProcessing := 21;

  FOR CrsSearchNewMsg IN (SELECT NaId
                            FROM NaInfoSapToWlt
                           WHERE Status = 2001 
                             AND NaArt  = MSGTYPE_PROCESS_ORDER
                           ORDER BY NaId)
  LOOP
    -- Check if there are any unprocessed messages,
    -- that have to be processed first 
    LocInfoText := 'SELECT COUNT(*) FROM NaInfoSapToWlt';
    SELECT COUNT(*)
      INTO LocCountUnprocessedMsg
      FROM NaInfoSapToWlt
     WHERE NaArt  <> MSGTYPE_PROCESS_ORDER
       AND Status <> 2004
       AND NaId    < CrsSearchNewMsg.NaId;

    -- If so then exit loop
    IF LocCountUnprocessedMsg > 0
    THEN
      EXIT; -- LOOP
    ELSE
      -- Continue processing
      ParBusyLastProcessing := 21;
    END IF;

    -- Call ERP proessing function
    IF LocRc = 0
    THEN
      LocRc := KP2PASX.Neue_Nachricht ( ParNaId => CrsSearchNewMsg.NaId,
                                        ParText => ParErrorText);
    END IF;
    
    IF LocRc <> 0
    THEN
      -- Rollback processing
      ROLLBACK;
      -- Set status to error
      UPDATE NaInfoSapToWlt
         SET Status     = 2004
           , Fehlertext = SUBSTR(ParErrorText, 1, 256)
       WHERE NaId = CrsSearchNewMsg.NaId;
    END IF;

    -- Reset RC
    LocRc := 0;
    -- Commit status change
    COMMIT;

  END LOOP;

  RETURN LocRc;

EXCEPTION
  WHEN OTHERS THEN
    TRACEHDL.TrcException(ParPackage      => ConstPackageName,
                          ParFunction     => ConstFunction,
                          ParStatement    => LocInfoText,
                          ParMeldungstext => TO_CHAR(SQLCODE) || ' ' || SQLERRM);
    TRACEHDL.TrcEndFunction;

    ParErrorText := GetOraErrorText(ConstPackageName||'.'||ConstFunction||' '||LocInfoText);
    LocRc := -1;
    RETURN LocRC;
END ProcessMessages;


As you can see in line 16, we query the data from the table NaInfoSapToWlt and in line 52 we Update the status for the selected row and commit the change.

Do you expect any problems with doing it in this way or is it a supported and recommended way?

and Chris said...

I expect a couple of things to happen here:

- The process takes a LONG TIME to complete
- You get data consistency issues and possibly ORA-01555 errors

So I would not recommend doing this.

The best way to write fast SQL is to execute one update which changes all the rows. That's what it's designed to do!

Ideally you would take the query driving the for loop and use it in the where clause of your update. As you have various other things going on here, you may need to use bulk processing instead.

https://blogs.oracle.com/oraclemagazine/bulk-processing-with-bulk-collect-and-forall

Is this answer out of date? If it is, please let us know via a Comment

More to Explore

SQL

The Oracle documentation contains a complete SQL reference.