I made a cursor in order to merge/update records in the database
I was wondering if it is correct or if anyone has any suggestions in order to improve the query.
DECLARE
CURSOR c_itemloc
IS
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.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;
l_item item_loc.item%TYPE;
l_loc item_loc.loc%TYPE;
loc_type item_loc.loc_type%TYPE;
l_source_method item_loc.source_method%TYPE;
l_primary_supp item_loc.primary_supp%TYPE;
l_source_wh item_loc.source_wh%TYPE;
i NUMBER;
l_commit VARCHAR2(1) := 'Y';
BEGIN
i :=0;
FOR r_itemloc IN c_itemloc
LOOP
i := i+1;
UPDATE item_loc il
SET
il.source_method = r_itemloc.source_method , -- 'S'
il.loc_type = r_itemloc.loc_type , -- 'S'
il.primary_supp = r_itemloc.primary_supp ,
il.source_wh = r_itemloc.source_wh ,
il.last_update_datetime = SYSDATE
WHERE item = r_itemloc.item
AND loc = r_itemloc.loc;
IF l_commit = 'Y' AND mod(i, 1000) = 0 THEN
COMMIT ;
END IF;
END LOOP;
EXCEPTION
WHEN OTHERS THEN
dbms_output.put_line('SOMETHING WENT WRONG');
END;
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 il
USING (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) itemloc
ON (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;