Skip to Main Content

Breadcrumb

Question and Answer

Tom Kyte

Thanks for the question, mandakini.

Asked: February 23, 2009 - 5:10 am UTC

Last updated: February 25, 2009 - 6:00 pm UTC

Version: 10g

Viewed 1000+ times

You Asked

Please see the below code do i need to put commit after all DML...OR one Commit will work ..


if err_sacdm_cus.count>0 then
begin
forall i in err_sacdm_cus.first .. err_sacdm_cus.last save EXCEPTIONS
update customer
set dcmo_err_msg = lv_err_msg(i),
dcmo_processed = lv_processed(i),
dcmo_passed = lv_passed(i),
dcmo_process_date = sysdate
where ss_id = lv_ss_id (i)
and ss_name = lv_ss_name (i);
commit;



delete from sacdm_owner.gs_bfg_migration_cus
where (cus_dcmr_id, cus_sourcesystem) in
( select ss_id, ss_name from sacdm_owner.customer
where dcmo_processed + dcmo_passed = 1
);

commit;

exception
when others then
PKG_SACDM_DCMO_ERROR_LOG.PR_CREATE_ERROR_LOG
(
ENTITY_NAME => 'SACDM_OWNER.CUSTOMER',
RUN_IDENTIFIER => 1,
SS_ID => Null,
SS_NAME => null,
ERR_CODE => Null,
ERR_DESCRIPTION => SQL%BULK_EXCEPTIONS.COUNT || ' records failed duding bulk update'|| substr(sqlerrm,1,700),
ERR_SOURCE => 'PKG_SACDM_DCMO_CUSTOMER.PR_UPDATE_SACDM_CUS - forall update',
DML_STATEMENT => null
);
end;
end if;
commit;

and Tom said...

where to begin, where to begin.


Ok, I'll begin with "when others, not followed by raise or raise_application_error is almost certainly a bug in your developed code". In your case, it is ABSOLUTELY a bug in your code. In short, because of that:

I hate your code
http://asktom.oracle.com/pls/ask/search?p_string=%22i+hate+your+code%22


never use when others without re-raising the exception.


Lets look at your "logic" here:
if err_sacdm_cus.count>0 then
    begin
      forall i in err_sacdm_cus.first .. err_sacdm_cus.last 
              save          EXCEPTIONS                           
      update customer
....
      commit;
     
      delete from sacdm_owner.gs_bfg_migration_cus
...           
        commit; 
             
    exception
      when others then       
         null basically, do nothing
    end;
  end if;
  commit; 


You really need to get a grasp on "transaction processing" and what it is all about. This code is frankly, scary.

You basically say "if I update the first set of stuff, I'll commit it, but if the delete fails - that is OK, we'll just log it and go on our merry way"


If I had my way - the keywords commit and rollback would be stricken from the plsql language entirely. You probably should NOT have commit anywhere in your code (only the client that invokes you is smart enough to know when the transaction is done).


If you have access to Expert Oracle Database Architecture - I recommend reading the chapters on transaction processing and concurrency. If not, buy any book on transaction processing and read it. Get the concepts guide (free, otn.oracle.com) - read it.

And stop committing until your transaction is DONE, that is the only time you commit, period.

And 99.9% of your code - isn't smart enough to know when that is (when it is ok to commit), only the CLIENT code does.

and use RAISE after when others, it is the only logical thing to do.

Rating

  (2 ratings)

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

Comments

ALTER SESSION DISABLE COMMIT IN PROCEDURE

Sokrates, February 23, 2009 - 4:28 pm UTC

should be the default again !
(like it was in 7.3)

Except ....

David Aldridge, February 25, 2009 - 10:45 am UTC

>> If I had my way - the keywords commit and rollback would be stricken from the plsql language entirely.

Before someone takes this quote and uses it to bash me for scattering commits in PL/SQL like rice at a wedding, let me just say that "following a direct path insert we cannot reference the target table again until we have commited, which is probably why the commits that you're complaining about are in there, and in any case all of those DDL statements are implicit commits anyway".

In other words, the usual over-defensive "doesn't apply to data warehousing" comment.
Tom Kyte
February 25, 2009 - 6:00 pm UTC

it is more "when used by someone that understands what they are doing, knows what commit means, understands that if they commit before they are done - they must make the process restartable (as you would in a warehouse situation)...." then commit and rollback are safe.