When should I commit when using FOR UPDATE in a procedure?

Question

If I use a FOR UPDATE clause in a stored procedure when should I "commit"? After closing the opened cursor or before closing the opened cursor? Below is the procedure im using, am i doing it in correct way?

CREATE OR REPLACE PROCEDURE Proc_UpdateCSClientCount(inMerid     IN  VARCHAR2,
                                                     outCliCount  OUT NUMBER,
                                                     outretvalue  OUT NUMBER)
AS
   CURSOR c1 IS
      SELECT CLIENT_COUNT
        FROM OP_TMER_CONF_PARENT
       WHERE MER_ID = inMerid
      FOR UPDATE OF CLIENT_COUNT;
BEGIN
   OPEN c1;
   IF SQL%ROWCOUNT = 1 THEN
      FETCH c1 INTO outCliCount;
      outCliCount := outCliCount + 1;
      UPDATE OP_TMER_CONF_PARENT
         SET CLIENT_COUNT = outCliCount
       WHERE CURRENT OF c1;
   END IF;
   outretvalue := 0;
   CLOSE c1;
   COMMIT;
EXCEPTION
   WHEN no_data_found THEN
      outretvalue := -1;
END;

Solution 1

You should commit at the end of the transaction. I doubt you can find a reasonable case where the end of the transaction is in the middle of a FOR UPDATE loop.

Perhaps you have heard that committing frequently is a good thing. This is a false myth, this is totally wrong. The opposite is true in Oracle: committing involves extra work and therefore you should only commit when all the work is done, never before.

Furthermore, from a logical point of view, it is unimaginably easier to recover from an error if you can start from scratch instead of having half the work done.

IMO, committing in a procedure should be excessively rare. The calling application should be the one that makes the necessary checks and finally decides if the data should be committed or not.

In conclusion, you can't commit accross a FOR UPDATE loop (it will produce an ORA-01002: fetch out of sequence) and that is a good thing. Whenever you find yourself committing accross a normal loop, you should ask yourself if the commit is really necessary -- most likely it isn't.

If you really need to commit and only fetch once, it doesn't matter if you commit before or after closing the cursor.

---

Update following your code excerpt: there are many things that need correcting in your code (I suppose it is not directly production code but still):

• The exception will never be raised: only implicit SELECT INTO can produce NO_DATA_FOUND.
• SQL%ROWCOUNT is NULL if the preceding statement is a SELECT.
• You could use c1%ROWCOUNT, but this will only return the number of rows fetched: 0 after the initial open.
• I mainly use FOR UPDATE NOWAIT so that two sessions never block each other. If you only use FOR UPDATE, you might as well use a single UPDATE and not use SELECT beforehand.
• This is a matter of preference, but return codes are prone to errors and exceptions are generally preferred. Let the error propagate. Why would anyone call this function on an id that doesn't exist? This is probably a bug in the calling app/procedure so you should not catch it.

So you could rewrite your procedure like this:

CREATE OR REPLACE PROCEDURE Proc_UpdateCSClientCount(inMerid     IN  VARCHAR2, 
                                                     outCliCount OUT NUMBER) AS
BEGIN
   -- lock the row, an exception will be raised if this row is locked
   SELECT CLIENT_COUNT + 1
     INTO outCliCount
     FROM OP_TMER_CONF_PARENT
    WHERE MER_ID = inMerid
   FOR UPDATE OF CLIENT_COUNT NOWAIT;
   -- update the row
   UPDATE OP_TMER_CONF_PARENT
      SET CLIENT_COUNT = CLIENT_COUNT + 1
    WHERE MER_ID = inMerid;
END;

Solution 2

From Oracle Documentation:

All rows are locked when you open the cursor, not as they are fetched. The rows are unlocked when you commit or roll back the transaction. Since the rows are no longer locked, you cannot fetch from a FOR UPDATE cursor after a commit.

That's important. If you've done the task(finished fetch) is not important if you commit before or after closing the cursor.

But if commit between fetchs is needed, as a workaround, use update with rowid, not where current of. Example from doc:

DECLARE
   CURSOR c1 IS SELECT last_name, job_id, rowid FROM employees;
   my_lastname   employees.last_name%TYPE;
   my_jobid      employees.job_id%TYPE;
   my_rowid      UROWID;
BEGIN
   OPEN c1;
   LOOP
      FETCH c1 INTO my_lastname, my_jobid, my_rowid;
      EXIT WHEN c1%NOTFOUND;
      UPDATE employees SET salary = salary * 1.02 WHERE rowid = my_rowid;
      -- this mimics WHERE CURRENT OF c1
      COMMIT;
   END LOOP;
   CLOSE c1;
END;
/

UPDATE(after edit of question): You can do that in a single sql, without cursor.

UPDATE OP_TMER_CONF_PARENT 
set CLIENT_COUNT = CLIENT_COUNT +1 
where MER_ID = inMerid;

UPDATE2. The code should be updated as following in order to work:

...
open C1;
FETCH C1 into OUTCLICOUNT;
--dbms_output.put_line(' count:'||c1%rowcount);
IF c1%rowcount = 1 THEN
      outCliCount := outCliCount + 1;
...

That is: fetch should be done before counting the rows affected, and rows affected is c1%rowcount, not sql%rowcount. If you want to know if a row is updated or not, you should put an else to if and assign a special value to the outretvalue parameter.

Comments

[Gaurav Soni]

:Fetching across commit can potentially cause dreaded ORA-1555 error

[Florin Ghita]

yes, but it's not the case here. In for update cursors you cannot fetch between commits.

[Gaurav Soni]

Yes,that is clear to me ,but the solution you have provided can cause ORA-1555 problem ,may be there is some other work around to achieve this say GTT table .

[Florin Ghita]

Oh, I see, I just gave a solution from docs. But the user even didn't ask for commits between fetch. He wants only to know wich is first close or commit :). So I think we should not go deeper into fetch problem :)

[Gaurav Soni]

:Ok no problem , i was just thinking of its cons :)

[Florin Ghita]

@GauravSoni Please, why you deleted the previous comment? It was very intersting.

[user1]

Hi all i updated my question, please check and tell me where can i commit and also am i handling the procedure in correct and efficient way??? Im sick of posting the code with alignment, i dont knw how to post it in stackoverflow

[Florin Ghita]

I think is ok. You've commited after closing the cursor. All changes, if no error happend, will be commited successfully.

[Gaurav Soni]

@FlorinGhita:The time you start a query oracle records a before image. So the result of your query is not altered by DML that takes place in the mean time (your big transaction). The before image uses the rollback segments to get the values of data that is changed after the before image is taken. By committing in your big transaction you tell oracle the rollback data of that transaction can be overwritten. If your query need data from the rollback segments that is overwritten you get this error. The less you commit the less chance you have that the rollback data you need is overwritten

[user1]

Ok here im checking 'if' condition if it fails it will go to EXCEPTION as of my procedure, is it efficient to use "ELSE" OR "WHEN no_data_found THEN" if the 'if' condition fails

[Florin Ghita]

sorry, I don't understand the question. There is no efficiency problem here, but only of correctness.

[user1]

Im using WHEN no_data_found in my procedure is it good to use "exception" or "else" followed by outretvalue := -1;

[user1]

Oh thankyou but im going for cursors coz i want lock "CLIENT_COUNT" column, only one session should have permission to update and others should oly fetch not update, w/o cursors it wont happen right.

[user1]

Hi vincent, 1.If i use NO WAIT other sessions cant fetch the same row it will throw the error, but i need other sessions to fetch(SELECT) but not update the same row at same time. 2.Is it possible to use FOR UPDATE clause w/o using cursors? 3.And while fetching itself u r incrementing the client count, if the Mer_id is not there, what will happen?

[user1]

4.If i use c1%ROWCOUNT after FETCH c1 INTO will it return the no. of rows affected?

[Vincent Malgrat]

1) In Oracle, locks never block readers. 2) You have to close explicit cursors, not implicit ones. 3) It will raise NO_DATA_FOUND, that is good, the calling app can handle it if it is not an error. 4) Don't use fetch with implicit cursors. If you use an explicit cursor that you fetch, use c1%ROWCOUNT after having fetched it.