I have been told the script I wrote below would be faster written in SQL alone and there
would be issues using the MIN on the date_ended (could both be null for duplicate records). Is this
true and why? And why is my code so wrong?
Requires Free Membership to View
spool &&filename -- set serveroutput on format wrapped execute dbms_output.new_line; execute dbms_output.put_line('In UPDATE_VLE_RESP.sql...'); execute dbms_output.new_line;
DECLARE -- =============================================================== -- Constants -- =============================================================== -- VLE AUTHOR and COAUTHOR responsibilities constants sql_c_usage_type constant varchar2(1) := 'P'; sql_c_vle_auth constant varchar2(10) := 'VLE_AUTHOR'; sql_c_vle_coauth constant varchar2(12) := 'VLE_COAUTHOR'; sql_c_ps_role_object constant varchar2(3) := 'PPI'; sql_c_rv_domain constant varchar2(15) := 'PS_ROLE_OBJECTS';
-- new VLE PROG_ORG responsibilities constants
sql_c_prog_org constant varchar2(8) := 'PROG_ORG';
-- =============================================================== -- Type and Subtype Declarations -- =============================================================== -- =============================================================== -- Variable Declarations -- ===============================================================
sql_commit_level NUMBER := 0; sql_future_vle_resp_cnt NUMBER := 0; sql_vle_upd_total NUMBER := 0; sql_msg_text VARCHAR2(1000); sql_hdr VARCHAR2(1);
sql_vle_rpt_total NUMBER := 0; -- -- ===================================================== -- Cursors -- ===================================================== -- get all current PPI responsibility types of VLE_AUTHOR and VLE_COAUTHOR. -- This will be used to close off these responsibilities and report those -- responsibilities that have been closed off in the future before they are updated. CURSOR c_psr_vle (p_vle_usage_type IN MISREG_PROG_STRUCT_ROLES.usage_type%TYPE, p_vle_auth IN MISREG_PROG_STRUCT_ROLES.opr_type_code%TYPE, p_vle_coauth IN MISREG_PROG_STRUCT_ROLES.opr_type_code%TYPE, p_vle_ps_role_object IN MISREG_PROG_STRUCT_ROLES.rv_ps_role_object%TYPE, p_vle_rv_domain IN MISREG_PROG_STRUCT_ROLES.rv_domain%TYPE) IS
SELECT rowid,
psr.role_code,
psr.hple_person_code,
psr.opr_type_code, -- VLE_AUTHOR/VLE_COAUTHOR
psr.rv_ps_role_object, -- 'PPI'
psr.rv_domain, -- 'PS_ROLE_OBJECTS'
psr.ppi_programme_code,
psr.ppi_consecutive_version_no,
psr.ppi_concurrent_version_no,
psr.ppi_programme_syllabus_period,
psr.ppi_domain_psp,
psr.ppi_location_code,
psr.ppi_calendar_type_code,
psr.ppi_moa_code,
psr.date_started,
psr.date_ended
FROM misreg_prog_struct_roles psr
WHERE psr.usage_type = p_vle_usage_type
AND psr.opr_type_code IN (p_vle_auth, p_vle_coauth)
AND psr.rv_ps_role_object = p_vle_ps_role_object
AND psr.rv_domain = p_vle_rv_domain
AND SYSDATE BETWEEN psr.date_started AND NVL(psr.date_ended,SYSDATE);
--
-- =====================================================
-- Procedures and Functions
-- =====================================================
PROCEDURE show_error(p_err_code number,
p_err_text varchar2) IS
BEGIN
raise_application_error(p_err_code, p_err_text);
END show_error;
------------------------------------------------------------------------------
PROCEDURE show_sys_error(p_err_code number,
p_err_text varchar2) IS
BEGIN
show_error(p_err_code => p_err_code,
p_err_text => p_err_text||' - '||
dbms_utility.format_error_stack);
END show_sys_error;
------------------------------------------------------------------------------
PROCEDURE show_msg(p_text VARCHAR2, p_new_line VARCHAR2) IS
sql_buffer NUMBER := 1;
BEGIN
WHILE sql_buffer <= LENGTH(p_text) LOOP
dbms_output.put_line(substr(p_text,sql_buffer,200));
sql_buffer := sql_buffer + 200;
END LOOP;
IF p_new_line = 'Y' THEN
dbms_output.new_line;
END IF;
END show_msg;
------------------------------------------------------------------------------
-- Insert PPI responsibility type of PROG_ORG based on existing current
-- PPI responsibility type of VLE_AUTHOR/VLE_COAUTHOR. If person has both
-- responsibilities then only 1 PROG_ORG record is created for the one with
-- the earliest end date. NB. if no end date has been recorded then PROG_ORG
-- record will be created with a NULL end date.
PROCEDURE ins_prog_org_resps (p_vle_usage_type IN MISREG_PROG_STRUCT_ROLES.usage_type%TYPE,
p_vle_auth IN MISREG_PROG_STRUCT_ROLES.opr_type_code%TYPE,
p_vle_coauth IN MISREG_PROG_STRUCT_ROLES.opr_type_code%TYPE,
p_prog_org IN MISREG_PROG_STRUCT_ROLES.opr_type_code%TYPE,
p_vle_ps_role_object IN MISREG_PROG_STRUCT_ROLES.rv_ps_role_object%TYPE,
p_vle_rv_domain IN MISREG_PROG_STRUCT_ROLES.rv_domain%TYPE) IS
sql_role_code MISREG_PROG_STRUCT_ROLES.role_code%TYPE;
BEGIN
--
sql_msg_text := 'Copying VLE responsibilities to new PROG_ORG responsibilities...';
show_msg(p_text => sql_msg_text, p_new_line => 'N');
--
INSERT INTO misreg_prog_struct_roles (role_code,
usage_type,
created_by,
created_date,
hple_person_code,
opr_type_code,
rv_ps_role_object,
rv_domain,
ppi_programme_code,
ppi_consecutive_version_no,
ppi_concurrent_version_no,
ppi_programme_syllabus_period,
ppi_domain_psp,
ppi_location_code,
ppi_calendar_type_code,
ppi_moa_code,
date_started,
date_ended)
SELECT misreg_prog_struct_role_seqs.nextval, inner_view.*
FROM (SELECT DISTINCT p_vle_usage_type in_usage_type, -- 'P'
USER,
SYSDATE in_cr_date,
psr.hple_person_code,
p_prog_org in_prog_org, -- 'PROG_ORG'
p_vle_ps_role_object in_ps_role_object, -- 'PPI'
p_vle_rv_domain in_vle_rv_domain, -- 'PS_ROLE_OBJECTS'
psr.ppi_programme_code,
psr.ppi_consecutive_version_no,
psr.ppi_concurrent_version_no,
psr.ppi_programme_syllabus_period,
psr.ppi_domain_psp,
psr.ppi_location_code,
psr.ppi_calendar_type_code,
psr.ppi_moa_code,
SYSDATE in_date_started,
psr.date_ended
FROM misreg_prog_struct_roles psr
WHERE psr.usage_type = p_vle_usage_type -- 'P'
AND psr.opr_type_code IN (p_vle_auth,p_vle_coauth)
AND psr.rv_ps_role_object = p_vle_ps_role_object
AND psr.rv_domain = p_vle_rv_domain
AND SYSDATE BETWEEN psr.date_started AND NVL(psr.date_ended,SYSDATE)
AND NVL(psr.date_ended,SYSDATE) =
(SELECT MIN(NVL(psr2.date_ended,SYSDATE)) earliest_date
FROM misreg_prog_struct_roles psr2
WHERE psr.usage_type = psr2.usage_type
AND psr.hple_person_code = psr2.hple_person_code
AND psr.rv_domain = psr2.rv_domain
AND psr.rv_ps_role_object = psr2.rv_ps_role_object
AND psr.ppi_programme_code = psr2.ppi_programme_code
AND psr.ppi_consecutive_version_no = psr2.ppi_consecutive_version_no
AND psr.ppi_concurrent_version_no = psr2.ppi_concurrent_version_no
AND psr.ppi_programme_syllabus_period = psr2.ppi_programme_syllabus_period
AND psr.ppi_domain_psp = psr2.ppi_domain_psp
AND psr.ppi_location_code = psr2.ppi_location_code
AND psr.ppi_calendar_type_code = psr2.ppi_calendar_type_code
AND psr.ppi_moa_code = psr2.ppi_moa_code
AND SYSDATE BETWEEN psr2.date_started AND NVL(psr2.date_ended,SYSDATE))) inner_view;
--
sql_msg_text := '---- There were '||SQL%rowcount||' VLE responsibilities copied to new PROG_ORG responsibilities...';
show_msg(p_text => sql_msg_text, p_new_line => 'Y');
-- dbms_output.put_line(chr(10));
-- dbms_output.put_line('VLE responsibilities records copied: '||SQL%rowcount);
EXCEPTION
WHEN OTHERS THEN
rollback;
show_sys_error(-20120,
'Error: Inserting Prog Struct Roles for '||
'Usage Type: '|| p_vle_usage_type|| ' and ' ||
'Opr Type Code: '|| p_prog_org || ' and ' ||
'Prog Struct Role object: '|| p_vle_ps_role_object);
END;
------------------------------------------------------------------------------
-- Check if there are any existing future PPI responsibility types of
-- VLE_AUTHOR/VLE_COAUTHOR. If there are then no further processing is done.
FUNCTION future_vle_resp (p_vle_usage_type IN MISREG_PROG_STRUCT_ROLES.usage_type%TYPE,
p_vle_auth IN MISREG_PROG_STRUCT_ROLES.opr_type_code%TYPE,
p_vle_coauth IN MISREG_PROG_STRUCT_ROLES.opr_type_code%TYPE,
p_vle_ps_role_object IN MISREG_PROG_STRUCT_ROLES.rv_ps_role_object%TYPE,
p_vle_rv_domain IN MISREG_PROG_STRUCT_ROLES.rv_domain%TYPE)
RETURN NUMBER
IS
v_return NUMBER :=0;
BEGIN
SELECT COUNT(psr.role_code)
INTO v_return
FROM misreg_prog_struct_roles psr
WHERE psr.usage_type = p_vle_usage_type -- 'P'
AND psr.opr_type_code IN (p_vle_auth,p_vle_coauth)
AND psr.rv_ps_role_object = p_vle_ps_role_object
AND psr.rv_domain = p_vle_rv_domain
AND SYSDATE < psr.date_started;
--
RETURN v_return;
--
EXCEPTION
WHEN OTHERS THEN
rollback;
show_sys_error(-20120,
'Error: Checking future Prog Struct Roles for '||
'Usage Type: '|| p_vle_usage_type|| ' and ' ||
'Opr Type Codes: '|| p_vle_auth || ', ' || p_vle_coauth || ' and ' ||
'Prog Struct Role object: '|| p_vle_ps_role_object);
END;
--
--=====================================================
-- Main Processing
--=====================================================
BEGIN
dbms_output.enable('1000000');
-- =========================================================
-- PROCESS VLE RESPONSIBILITIES UPDATE
-- =========================================================
--
-- Check if there are any future responsibilities
sql_future_vle_resp_cnt := future_vle_resp (sql_c_usage_type,
sql_c_vle_auth,
sql_c_vle_coauth,
sql_c_ps_role_object,
sql_c_rv_domain);
--
IF sql_future_vle_resp_cnt = 0 THEN
-- Copy existing VLE responsibilities
ins_prog_org_resps (sql_c_usage_type,
sql_c_vle_auth,
sql_c_vle_coauth,
sql_c_prog_org,
sql_c_ps_role_object,
sql_c_rv_domain);
--
-- Get existing VLE responsibilities to be closed off
sql_msg_text := 'Closing off existing current VLE responsibilities...';
show_msg(p_text => sql_msg_text, p_new_line => 'Y');
-- dbms_output.put_line(chr(10));
-- dbms_output.put_line('Closing off existing VLE responsibilities...');
--
sql_hdr := 'Y';
FOR c_upd_vle in c_psr_vle(sql_c_usage_type,
sql_c_vle_auth,
sql_c_vle_coauth,
sql_c_ps_role_object,
sql_c_rv_domain) LOOP
--
sql_vle_upd_total := sql_vle_upd_total + 1;
--
IF c_upd_vle.date_ended IS NOT NULL THEN
-- current VLE responsibility has been closed off in the future so report it
IF sql_hdr = 'Y' THEN
sql_msg_text := 'Current VLE Responsibilities closed off in the future';
show_msg(p_text => sql_msg_text, p_new_line => 'N');
sql_msg_text := '-----------------------------------------------------';
show_msg(p_text => sql_msg_text, p_new_line => 'Y');
sql_hdr := 'N';
END IF;
-- keep a running total of reported recs to use in output
sql_vle_rpt_total := sql_vle_rpt_total + 1;
sql_msg_text := 'Rec Num: '|| sql_vle_rpt_total;
show_msg(p_text => sql_msg_text, p_new_line => 'N');
sql_msg_text := '----------------------';
show_msg(p_text => sql_msg_text, p_new_line => 'N');
sql_msg_text := 'Role Code: '||c_upd_vle.role_code||'. '||
'Person Code: '||c_upd_vle.hple_person_code||'. '||
'Opr Type Code: '||c_upd_vle.opr_type_code||'. '||
'PS Role Obj: '||c_upd_vle.rv_ps_role_object||'. '||
'RV Domain: '||c_upd_vle.rv_domain||'. '||
'PPI Prog Code: '||c_upd_vle.ppi_programme_code||'. '||
'PPI Cons: '||c_upd_vle.ppi_consecutive_version_no||'. '||
'PPI Conc: '||c_upd_vle.ppi_concurrent_version_no||'. '||
'PPI Period: '||c_upd_vle.ppi_programme_syllabus_period||'. '||
'PPI Domain PSP: '||c_upd_vle.ppi_domain_psp||'. '||
'PPI Location: '||c_upd_vle.ppi_location_code||'. '||
'PPI Cal Type: '||c_upd_vle.ppi_calendar_type_code||'. '||
'PPI MoA: '||c_upd_vle.ppi_moa_code||'. '||
'Date Started: '||c_upd_vle.date_started||'. '||
'Date Ended: '||c_upd_vle.date_ended||'.';
--
show_msg(p_text => sql_msg_text, p_new_line => 'Y');
-- VLE responsibilities record has been closed off in the future
END IF;
--
-- Update VLE responsibilities and close off
UPDATE misreg_prog_struct_roles
SET date_ended = (SYSDATE - 1)
WHERE rowid = c_upd_vle.rowid;
--
sql_commit_level := sql_commit_level + 1; -- Commit after every X records
IF sql_commit_level > 1000 THEN
COMMIT;
sql_commit_level := 0;
END IF;
--
END LOOP;
--
sql_msg_text := '---- There were '||sql_vle_upd_total||' current VLE Responsibilities closed off (date_ended set to '||(sysdate-1)||')...';
show_msg(p_text => sql_msg_text, p_new_line => 'Y');
--
COMMIT;
--
ELSE
dbms_output.new_line;
sql_msg_text := 'Error: Cannot process VLE responsibilities updates.'||chr(10)||
'There are future Prog Struct Roles for...'||chr(10)||
'Usage Type: '|| sql_c_usage_type|| ' and '||
'Opr Type Codes: '|| sql_c_vle_auth || ', ' || sql_c_vle_coauth || ' and ' ||chr(10)||
'Prog Struct Role object: '|| sql_c_ps_role_object;
show_msg(p_text => sql_msg_text, p_new_line => 'Y');
--
END IF;
--
sql_msg_text := '...UPDATE_VLE_RESP.sql finished.';
show_msg(p_text => sql_msg_text, p_new_line => 'N');
END;
/
The likely reason why you've been told the work would be best accomplished in SQL alone (if possible) is that you are using row by row processing. And, as Tom Kyte likes to say, "row by row is slow by slow". Doing anything one row at a time instead of in sets will, by its nature, be slower.
Think of it this way: If you had a grocery list of 10 items, you could go to the grocery store and pick up the first item, pay for it, take it home, put it away, then look at your list to see what the next item is you need, go back to the store, get that item, and so on until you get all 10 items. That doesn't sound very efficient, does it? You'd want to get the whole "set" of items on the list in one trip, right? Well, processing through a data set one row at a time is like going to the grocery store 10 times to get 10 items. It's just plain slower. It may sound funny, but a real-life scenario like that one is analogous to the difference in processing a SQL statement vs doing the same thing in a row by row PL/SQL loop.
The big clue you have in your code that you could consolidate what you need to do into either one or maybe two distinct SQL statements is evidenced by the fact that the SQL statements you execute as part of your main processing loop are all basically the same. They all have basically the same predicate (WHERE clause). And, you don't really do anything with each row that you UPDATE except set the date_ended field to a different value. So, why do it row by row? Why not just make the UPDATE statement use the needed SELECT in the WHERE clause so that it returns all the rows that need to be updated and then update them directly instead of doing them one at a time.
Basically here's what you're doing:
- Execute a query to see if there are any rows where the date_started is greater than today's date.
- If there are, then you execute an INSERT to put new rows in the table (this logic escapes me a bit, but if you say it works as desired, then OK). Your usage of the MIN clause could likely be done with an analytic function (that would be my preference), but that's a whole other topic that I'll be glad to address if you want to post a follow-up if you find you need/want to delve into that.
- You then read all the rows, which should now be the original rows plus the ones you just inserted, one at a time and UPDATE each row's date_ended.
So, I'd say that at worst case, you should be able to execute two statements: 1) to do an INSERT for the rows you need to add and 2) a single UPDATE to modify the date_ended. Depending on how many rows are affected by this process, the time and resource usage savings could be quite significant by getting rid of the row by row approach.
One other thing that you can research a bit more on your own that will help you understand the additional overhead of the row by row approach is something called context-switching. Whenever you execute SQL from inside a PL/SQL loop, Oracle has to context switch out of the PL/SQL execution engine into the SQL execution engine. It executes the statement (in your case, an UPDATE) and then switches back to the PL/SQL engine to continue processing. Those switches carry an overhead. Even if it's only 1/1000th of a second, if you are dealing with thousands of rows, the context switches alone can eat up a lot of time that isn't necessary. You completely avoid those switches if you use a single SQL statement instead. You can still execute your two statements in a PL/SQL block to be able to use exception handling and so forth, but you really don't need all that extra code that you've got in there to process the updates one at a time. Again, think of our grocery store example, if you had 100,000 rows to update, you could do 100,000 separate UPDATE statements (like you're doing now), or you can do a single UPDATE that updates 100,000 rows.
I'll end by telling you to try it and see. It's easy to compare the timings of each way of doing it. If you're only dealing with a few hundred rows, you may not "feel" the difference quite so much as if you're dealing with thousands or tens of thousands. But, from a big picture performance optimization point of view, you should always avoid row by row processing whenever you can.
This was first published in February 2011
Join the conversationComment
Share
Comments
Results
Contribute to the conversation