You Asked
Hi Tom,
Good Day.
In my current company, a colleague of mine have written a stored procedure (which is provided below). He asked me to tune. In my current company we do not have access to any of the dynamic performance views or any other tools like TKPROF to profile the stored procedure to find out where it takes time.
At the outset, the logic in the stored procedure is correct and seems to be perfect, but takes very long time to execute.
The logic is provided below:
a) There are 3 tables MS_FO_INVESTMENTVEHICLE, MS_FO_AREANAME, MS_FO_FEESCHEDULE.
b) Exact replica of these 3 tables are MS_FO_INVESTVEH_UPDATES, MS_FO_ANAME_UPDATES, MS_FO_FEESCH_UPDATES. These tables are exposed to other applications accessing this database.
c) Data comes in two modes one is Refresh (R) and other one is Delta (here it is specified as U)
d) Whenever data comes into the system, they are initially written into the tables specified in a) and then the replicated data are updated or inserted in tables provided in b)
e) In case of 'R', the records in the tables provided in b) must be deleted and then fresh data to be inserted. In case of 'U', the records in MS_FO_INVESTVEH_UPDATES must be updated (if already exists) or inserted (new records) and the records must be deleted and inserted in other 2 tables specified in b)
f) Once this is done, there is a column called Processed, which must be updated to Y. Default is N
All the necessary indexes are created.
Please pardon me for the format of the stored procedure provided below
--------------Procedure Starts Here--------------
/* Created for Feed Handle Process */
create or replace procedure db_FeedUpdMorningStarProc(inibdpartyid in number) as
cursor rucur is select distinct refreshupdate from ms_fo_investmentvehicle where ibdpartyid=inibdpartyid and processed='N' order by refreshupdate;
tempid number;
begin
for ru in rucur
loop
/* R implies Full Refresh. In this case, the records for the particular IBDPARTYID and LEGALTYPE would be deleted and then inserted with the new data*/
if ru.refreshupdate ='R' then
dbms_output.put_line('Calling Refresh Procedure');
db_RefreshDataProc(inibdpartyid);
dbms_output.put_line('Calling Refresh Procedure Completed');
/* U implies Delta. In this case, the records for the particular IBDPARTYID and LEGALTYPE would be deleted from two child tables and then inserted with the new data*/
elsif ru.refreshupdate ='U' then
dbms_output.put_line('Calling Update Procedure');
db_UpdateDataProc(inibdpartyid);
dbms_output.put_line('Calling Update Procedure Completed');
end if;
end loop;
dbms_output.put_line('Calling Update N to Y Procedure');
db_UpdateNToYProc(inibdpartyid);
dbms_output.put_line('Update N to Y Procedure Completed');
exception
When others then
dbms_output.put_line('Error is ' || SQLERRM || ' ----------> ' || tempid);
end;
/* The following procedure takes care of Refresh Data */
create or replace procedure db_RefreshDataProc(inibdpartyid in varchar2) as
cursor refcur is select distinct legaltype from ms_fo_investmentvehicle where ibdpartyid=inibdpartyid and processed='N';
begin
for rcur in refcur
loop
delete from MS_FO_INVESTVEH_UPDATES where ibdpartyid=inibdpartyid and legaltype=rcur.legaltype;
delete from ms_fo_feeschupdates where ibdpartyid=inibdpartyid and exists (select id__ from ms_fo_investmentvehicle where legaltype=rcur.legaltype and processed='N');
delete from ms_fo_aname_updates where ibdpartyid=inibdpartyid and exists (select id__ from ms_fo_investmentvehicle where legaltype=rcur.legaltype and processed='N');
dbms_output.put_line('Delete Refresh Done');
insert into MS_FO_INVESTVEH_UPDATES select * from ms_fo_investmentvehicle where processed='N' and ibdpartyid=inibdpartyid and legaltype=rcur.legaltype;
insert into ms_fo_feeschupdates select * from ms_fo_feeschedule where exists (select id__ from ms_fo_investmentvehicle where ibdpartyid=inibdpartyid and processed='N' and legaltype=rcur.legaltype);
insert into ms_fo_aname_updates select * from ms_fo_areaname where exists (select id__ from ms_fo_investmentvehicle where ibdpartyid=inibdpartyid and processed='N' and legaltype=rcur.legaltype);
dbms_output.put_line('Insert Refresh Done');
end loop;
commit;
dbms_output.put_line('Refresh Done');
exception
When others then
dbms_output.put_line('Error In Refresh Procedure and Error is ' || SQLERRM);
end;
/* The following procedure takes care of Update Processed value from N to Y Data */
create or replace procedure db_UpdateNToYProc(inibdpartyid in varchar2) as
begin
update ms_fo_investmentvehicle set processed='Y' where ibdpartyid=inibdpartyid and processed='N';
dbms_output.put_line('N On Investmentvehicle Done');
update MS_FO_INVESTVEH_UPDATES set processed='Y' where ibdpartyid=inibdpartyid and processed='N';
dbms_output.put_line('N On Investmentvehicle Updates Done');
commit;
exception
When others then
dbms_output.put_line('Error In Update N To Y Procedure and Error is ' || SQLERRM);
end;
/* The following procedure takes care of Update Data */
create or replace procedure db_UpdateDataProc(inibdpartyid in varchar2) as
begin
merge into MS_FO_INVESTVEH_UPDATES a
using (select ID__, INVESTMENTVEHICLENAME, LEGALTYPE, LEGALNAME, SHARECLASSTYPE_ID, CUSIP, ISIN, TRADINGSYMBOL, FUNDFAMILYNAME, FUNDFAMILYNAME_ID, RATINGY3, RISKRATINGY3, RATINGY5, RISKRATINGY5, RATINGY10, RISKRATINGY10, RATINGOVERALL, RISKRATINGOVERALL, FEEDPATHID, IBDPARTYID, CREATEDDATE, PROCESSED, NAME, ID, FRONTLOAD, DEFERLOAD, REFRESHUPDATE from ms_fo_investmentvehicle where ibdpartyid=inibdpartyid and processed='N' and refreshupdate='U') b
on (a.id__=b.id__)
when MATCHED then update set INVESTMENTVEHICLENAME=b.INVESTMENTVEHICLENAME, LEGALNAME=b.LEGALNAME, SHARECLASSTYPE_ID=b.SHARECLASSTYPE_ID, ISIN=b.ISIN, TRADINGSYMBOL=b.TRADINGSYMBOL, FUNDFAMILYNAME=b.FUNDFAMILYNAME, FUNDFAMILYNAME_ID=b.FUNDFAMILYNAME_ID, RATINGY3=b.RATINGY3, RISKRATINGY3=b.RISKRATINGY3, RATINGY5=b.RATINGY5, RISKRATINGY5=b.RISKRATINGY5, RATINGY10=b.RATINGY10, RISKRATINGY10=b.RISKRATINGY10, RATINGOVERALL=b.RATINGOVERALL, RISKRATINGOVERALL=b.RISKRATINGOVERALL, FEEDPATHID=b.FEEDPATHID, CREATEDDATE=b.CREATEDDATE, NAME=b.NAME, ID=b.ID, FRONTLOAD=b.FRONTLOAD, DEFERLOAD=b.DEFERLOAD where a.cusip=b.cusip and a.processed=b.processed and a.refreshupdate=b.refreshupdate and a.ibdpartyid=b.ibdpartyid and a.legaltype=b.legaltype
when not MATCHED then insert(ID__, INVESTMENTVEHICLENAME, LEGALTYPE, LEGALNAME, SHARECLASSTYPE_ID, CUSIP, ISIN, TRADINGSYMBOL, FUNDFAMILYNAME, FUNDFAMILYNAME_ID, RATINGY3, RISKRATINGY3, RATINGY5, RISKRATINGY5, RATINGY10, RISKRATINGY10, RATINGOVERALL, RISKRATINGOVERALL, FEEDPATHID, IBDPARTYID, CREATEDDATE, PROCESSED, NAME, ID, FRONTLOAD, DEFERLOAD, REFRESHUPDATE) values (b.ID__, b.INVESTMENTVEHICLENAME, b.LEGALTYPE, b.LEGALNAME, b.SHARECLASSTYPE_ID, b.CUSIP, b.ISIN, b.TRADINGSYMBOL, b.FUNDFAMILYNAME, b.FUNDFAMILYNAME_ID, b.RATINGY3, b.RISKRATINGY3, b.RATINGY5, b.RISKRATINGY5, b.RATINGY10, b.RISKRATINGY10, b.RATINGOVERALL, b.RISKRATINGOVERALL, b.FEEDPATHID, b.IBDPARTYID, b.CREATEDDATE, b.PROCESSED, b.NAME, b.ID, b.FRONTLOAD, b.DEFERLOAD, b.REFRESHUPDATE);
dbms_output.put_line('Merge Done');
delete from ms_fo_feeschupdates where ibdpartyid=inibdpartyid and exists (select id__ from ms_fo_investmentvehicle where processed='N');
dbms_output.put_line('Delete 1 Done');
delete from ms_fo_aname_updates where ibdpartyid=inibdpartyid and exists (select id__ from ms_fo_investmentvehicle where processed='N');
dbms_output.put_line('Delete 2 Done');
insert into ms_fo_feeschupdates select * from ms_fo_feeschedule where exists (select id__ from ms_fo_investmentvehicle where ibdpartyid=inibdpartyid and processed='N');
insert into ms_fo_aname_updates select * from ms_fo_areaname where exists (select id__ from ms_fo_investmentvehicle where ibdpartyid=inibdpartyid and processed='N');
commit;
dbms_output.put_line('Update Done');
exception
When others then
dbms_output.put_line('Error In Update Procedure and Error is ' || SQLERRM);
end;
--------------Procedure Ends Here--------------
a) I would like to know a way I can profile the stored procedures
b) Is there a way I can use Bulk Insert here? If yes, can you please site some examples or links where it is demonstrated
Thanks a lot
Hari
and Tom said...
... In my current company we do not have access to any of the dynamic performance views or any other tools like TKPROF to profile the stored procedure to find out where it takes time. ...
why not? get it, without it - you are somewhat DOOMED.
exception
When others then
dbms_output.put_line('Error is ' || SQLERRM || ' ----------> ' || tempid);
end; I hate your code. All of it.
http://asktom.oracle.com/pls/asktom/asktom.search?p_string=%22i+hate+your+code%22 I cannot bear to read it with that 'construct'.
Also, without some formatting - it is unreadable.
I don't know your schema
I don't know your data volumes
I don't know anything about your stuff.
and I cannot read the existing code
and it has that horrible "when others then null" bug in it.
... All the necessary indexes are created. ...
either that statement is wrong or it is right. We have no way to judge that from where we sit
for a batch process - probably ZERO indexes would be useful. I see slow by slow processing in there (for loops).
Suggestion: get rid of slow by slow code, turn it into a single set operation.
I see some really "questionable" code there.
Look at this delete:
delete from ms_fo_feeschupdates where ibdpartyid=inibdpartyid and exists (select id__ from ms_fo_investmentvehicle where legaltype=rcur.legaltype and processed='N');
Now, rcur is defined as:
select distinct legaltype from ms_fo_investmentvehicle where ibdpartyid=inibdpartyid and processed='N';
since you get distinct legaltype from ms_fo_investmentvehicle where ibdpartyid=inibdpartyid and processed='N';
would that delete not just simply be (outside of a loop - no looping required/desired for this!)
delete from ms_fo_feeschupdates where ibdpartyid=inibdpartyid
and exists (select null from ms_fo_investmentvehicle where
ibdpartyid=inibdpartyid and processed='N');
you KNOW that
and exists (select id__ from ms_fo_investmentvehicle where legaltype=rcur.legaltype and processed='N');
must be satisfied since you are looping over
select distinct legaltype from ms_fo_investmentvehicle where ibdpartyid=inibdpartyid and processed='N';
that legaltype for processed = 'N' must exist, else you would not have retrieved it on the distinct
Further, that code in the loop - why is there a loop at all, why are they not just all SET BASED operations - what point, what purpose, what goal is the loop? (only to slow things down....)
As far as I can tell, the one procedure should be nothing more than:
create or replace procedure db_RefreshDataProc(inibdpartyid in varchar2)
as
begin
delete from MS_FO_INVESTVEH_UPDATES
where ibdpartyid=inibdpartyid
and legaltype in ( select legaltype from ms_fo_investmentvehicle where ibdpartyid=inibdpartyid and processed='N' );
delete from ms_fo_feeschupdates where ibdpartyid=inibdpartyid
and exists (select null from ms_fo_investmentvehicle where
ibdpartyid=inibdpartyid and processed='N');
delete from ms_fo_aname_updates
where ibdpartyid=inibdpartyid
and exists (select null from ms_fo_investmentvehicle where
ibdpartyid=inibdpartyid and processed='N');
dbms_output.put_line('Delete Refresh Done');
insert into MS_FO_INVESTVEH_UPDATES
select *
from ms_fo_investmentvehicle
where processed='N' and ibdpartyid=inibdpartyid;
insert into ms_fo_feeschupdates
select *
from ms_fo_feeschedule
where exists (select null from ms_fo_investmentvehicle where ibdpartyid=inibdpartyid and processed='N';
insert into ms_fo_aname_updates
select *
from ms_fo_areaname
where exists (select id__ from ms_fo_investmentvehicle where ibdpartyid=inibdpartyid and processed='N');
end;
remember that rcur is defined as:
select distinct legaltype from ms_fo_investmentvehicle where ibdpartyid=inibdpartyid and processed='N';
in the loop therefore:
delete from MS_FO_INVESTVEH_UPDATES where ibdpartyid=inibdpartyid and legaltype=rcur.legaltype;
would just be
delete from MS_FO_INVESTVEH_UPDATES where ibdpartyid=inibdpartyid and legaltype in (that rcur set);
and
delete from ms_fo_feeschupdates where ibdpartyid=inibdpartyid and exists (select id__ from ms_fo_investmentvehicle where legaltype=rcur.legaltype and processed='N');
is just
delete from ms_fo_feeschupdates where ibdpartyid=inibdpartyid
and exists (select null from ms_fo_investmentvehicle where ibdpartyid=inibdpartyid and processed='N');
and so on.
And then I see that RefreshDataProc is called itself in a loop - meaning, we need to FURTHER decompose all of this and build an even larger, more comprehensive delete statement (the goal being to remove EVERY LOOP in your code and replace with SINGLE SQL STATEMENTS that do all of the work in bulk - avoiding every index possible!!!)
This code needs heavy duty review and refactoring. It needs to literally be ripped apart - and turned into a specification and rewritten from the ground up.
and please, never ever EVER code:
exception
When others then
dbms_output.put_line('Error In Update Procedure and Error is ' || SQLERRM);
end again, it makes you look like a high school programmer at best. It is the single most worst practice I know of.
Rating
(3 ratings)
Is this answer out of date? If it is, please let us know via a Comment