CODEWITHSUNDEEP
oracle-databasestored-proceduresplsqloracle10goracle11g
user1
user1

Reputation: 727

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

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;

Upvotes: 2

Views: 16193

Answers (3)

Vincent Malgrat
Vincent Malgrat

Reputation: 67762

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;

Upvotes: 6

Florin Ghita
Florin Ghita

Reputation: 17643

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.

Upvotes: 2

Petr Pribyl
Petr Pribyl

Reputation: 3575

If you commit before closing cursor and after that you try to fetch again, you get an INVALID_CURSOR exception. I suggest to commit after closing cursor.

Upvotes: 0

Related Questions