Can anyone help me with my pl/sql script to update/merge in the database
Assuming that the code does functionally what you want...
1) Remove the exception handler. There is no benefit to catching an unknown exception that you cannot handle unless you are doing something like logging it and rethrowing it. Catching an exception only to call dbms_output
not only hides the exception details and the stack trace that you'd need to debug the exception, it may hide the exception entirely if the caller happens not to read from the buffer dbms_output
writes to.
2) Committing in a loop is generally a bad idea. Most importantly, what happens if your session dies in the middle? You'd have a partially committed update with no way to resume the operation when you restarted the code later on. You'd have to re-update all the rows that you had previously updated and whose updates you committed which could have downstream impacts. And committing in a loop slows down your code for no real reason.
3) If all you are trying to do is update a table, the most efficient approach would be to write a single UPDATE
statement that updates all the rows at once rather than iterating over a cursor and doing a lot of single-row updates. There may be other reasons to favor a cursor (it may make the code easier for other developers to understand, for example) but from a performance standpoint, if you can do it in SQL, SQL is going to be the most efficient approach.
1) You're replacing a meaningful error with a meaningless one. Plus, if you have haven't set output on
, an error will be missed entirely. The best thing to do would be to simply remove the exception block. If you can't do that, you should dump at least SQLERRM
and probably DBMS_UTILITY.FORMAT_ERROR_BACKTRACE
to DBMS_OUTPUT
. To catch and record errors well, you'd need to pass the exception details to a separate procedure that would use an autonomous transaction to write those details to a table. Though, even in that case, you're best off re-raising the error after you record it.
2) Committing every X records is a poor practice in most cases. If these records are all being updated together, then they should be part of the same transaction.
3) You could do this in a single statement using either UPDATE
or MERGE
. Generally that's preferred, as it avoids additional context switches. Personally, I like MERGE
in this scenario:
MERGE INTO item_loc ilUSING (SELECT item, loc, loc_type, source_method, primary_supp, source_wh FROM (SELECT dc_vert.item, dc_vert.loc, dc_vert.loc_type, dc_vert.source_method, dc_vert.primary_supp, w.primary_vwh source_wh, dc_vert.actie, MAX(dc_vert.actie) OVER (PARTITION BY dc_vert.item, dc_vert.loc) actie_max, COUNT(dc_vert.primary_supp) OVER (PARTITION BY dc_vert.item, dc_vert.loc) primary_count FROM dc_item_loc_pim_lms dc_vert, item_supplier isu, store sto, wh w WHERE dc_vert.primary_supp IS NOT NULL AND isu.item = dc_vert.item AND dc_vert.primary_supp = isu.supplier AND w.wh = dc_vert.source_wh AND sto.store = dc_vert.loc AND isu.supp_discontinue_date >= SYSDATE) WHERE actie = actie_max AND primary_count = 1) itemlocON (il.item = itemloc.item AND il.loc = itemloc.loc)WHEN MATCHED THEN UPDATE SET il.source_method = itemloc.source_method, il.loc_type = itemloc.loc_type, il.primary_supp = itemloc.primary_supp, il.source_wh = itemloc.source_wh, il.last_update_datetime = SYSDATE;
I would suggest you to use the modern join syntax
FROM dc_item_loc_pim_lms dc_vert INNER JOIN item_supplier isu ON dc_vert.item = isu.item AND dc_vert.primary_supp = isu.supplier INNER JOIN store sto ON dc_vert.loc = sto.store INNER JOIN wh w ON dc_vert.source_wh = w.whWHERE dc_vert.primary_supp IS NOT NULL AND isu.supp_discontinue_date >= SYSDATE