Hi,
I have a fairly simple pl/sql procedure invoked by a java program that itself is invoked when a user clicks a link on a website. The procedure takes in 2-3 parameters, and its main purpose is to insert 1 record (based on the params) into a table. (If needed, a version of the proc is in the LiveSQL link. Table and column names changed to protect the innocent.)
Prior to the insert, the procedure tests whether the row it's about to insert already exists. This is necessary because we're only avoiding dupes for the specific values passed in by this program, whereas dupes involving other values are OK, so there is no unique constraint.
If the row already exists, then the proc inserts this "failed" attempt into a logging table (this part is done by error handling, which I'm somewhat regretting and thinking of rewriting in a simple ELSE part of an IF, but it's unlikely this is relevant to my question).
If the row does
not yet exist, then it gets inserted, but I should note that there's a quick SELECT INTO between the test and the insert.
Vast majority of the time this works fine, but a handful of duplicates have gotten into the table! The timestamps on the date_inserted field indicate that the dupes were inserted anywhere from 0 to a whole 18 seconds apart from one another.
I have some ideas on why this may be happening (multiple clicks & network latency causing multiple sessions & procedure calls to fire simultaneously...perhaps first session committing between the time second session tests and inserts), but outside a unique constraint, is there anything I can do within the procedure to stop these dupes from sneaking in?
Would it stop the dupes entirely if I test for the row's existence within the insert? Something like:
insert into user_demo
with new_rec as (
select
1 as internal_id,
'ABC' as demo_code,
'blah' as user_demo_comment,
sysdate as date_inserted
from dual
)
select *
from new_rec
where not exists
(select 'x'
from user_demo ud2
where ud2.internal_id = new_rec.internal_id
and ud2.demo_code = demo_code);
Thank you!
Phil
PS. If you look at the proc in LiveSQL, please excuse the gross varchar2 type on what should probably be a date parameter. I'm aware this is not a best practice, but letting you know it's there in the interest of full disclosure.
Thank you in advance,
Phil
You have a concurrency problem here.
Oracle Database has statement level consistency by default. This means DML can only see changes committed at the time the statement starts.
So if two people call this procedure at the same time, duplicate values are inevitable. The flow is could go like this:
T(ime)1 = user 1 runs the select, finds no rows
T2 = user 1 inserts a row
T3 = user 2 runs the select
At this point user 1
has not committed their changes. So user 2 can't see the row user 1 inserted. Thus the query at T3 returns no rows, and they insert the same values.
An easy way to see this is to stick a wait into the procedure just before the commit. This is to give you enough time to run the process in a second session while waiting for the first to complete:
insert into users
values ( 1, 1 );
insert into demos
values ( 'DEM', 'DEMO' );
commit;
create or replace procedure p_demo(
p_id varchar2,
p_demo_code varchar2,
clicked_on varchar2 default to_char(sysdate,'mm/dd/yyyy hh24:mi:ss')
) as
l_internal_id number;
l_user_demo_comment varchar2(100);
cursor cur_demo is
select 'x'
from user_demo
where internal_id = l_internal_id
and demo_code = demo_code;
r_demo cur_demo%rowtype;
e_user_already_demoed exception;
begin
select internal_id
into l_internal_id
from users
where user_facing_id = p_id;
open cur_demo;
fetch cur_demo into r_demo;
if cur_demo%found then
close cur_demo;
raise e_user_already_demoed;
else
select demo_desc||' '||clicked_on
into l_user_demo_comment
from demos
where demo_code = p_demo_code;
insert into user_demo (
internal_id, demo_code, user_demo_comment, date_inserted
) values (
l_internal_id, p_demo_code, l_user_demo_comment, sysdate
);
dbms_session.sleep ( 10 );
commit;
end if;
close cur_demo;
exception
when e_user_already_demoed then
dbms_output.put_line ( 'ALREADY DEMOED' );
when no_data_found then
dbms_output.put_line ( 'NDF' );
END p_demo;
/
PRO session 1
exec p_demo ( 1, 'DEM' );
PRO in session 2
exec p_demo ( 1, 'DEM' );
PRO session 1 or 2
select *
from user_demo;
INTERNAL_ID DEMO_CODE USER_DEMO_COMMENT DATE_INSERTED
1 DEM DEMO 09/29/2020 15:41:31 29-SEP-2020 15:41:31
1 DEM DEMO 09/29/2020 15:41:32 29-SEP-2020 15:41:32
Would it stop the dupes entirely if I test for the row's existence within the insert?The problem is still possible here. A second user could start the insert before the first commits. So the query won't see the new row and insert a duplicate.
The easiest way to solve this is with a unique constraint. As you only want to prevent duplicates for a subset of values, you'll want a function-based unique index. This maps duplicate allowed values to null and duplicates disallowed to a column.
For example, for the code UNQ, this only allows you to insert one row with that value:
create unique index ui
on user_demo ( case when demo_code = 'UNQ' then demo_code end );
insert into demos
values ( 'UNQ', 'Unique code' );
commit;
You can then rewrite the whole procedure to a plain insert:
create or replace procedure p_demo(
p_id varchar2,
p_demo_code varchar2,
clicked_on varchar2 default to_char(sysdate,'mm/dd/yyyy hh24:mi:ss')
) as
begin
insert into user_demo (
internal_id, demo_code, user_demo_comment, date_inserted
) values (
( select internal_id
from users
where user_facing_id = p_id
),
p_demo_code, (
select demo_desc||' '||clicked_on
from demos
where demo_code = p_demo_code
), sysdate
);
end p_demo;
/
At this point you can also remove the wait to see the effect. If the first session hasn't committed, any other sessions trying to insert the code UNQ are blocked until it completes:
PRO session 1
exec p_demo ( 1, 'UNQ' );
PRO session 2 - this will be blocked until session 1 commits/rollsback
exec p_demo ( 2, 'UNQ' );
PRO session 1
commit;
PRO session 2
ORA-00001: unique constraint (CHRIS.UI) violated
select * from user_demo;
INTERNAL_ID DEMO_CODE USER_DEMO_COMMENT DATE_INSERTED
1 DEM DEMO 09/29/2020 15:41:31 29-SEP-2020 15:41:31
1 DEM DEMO 09/29/2020 15:41:32 29-SEP-2020 15:41:32
1 UNQ Unique code 09/29/2020 15:46:40 29-SEP-2020 15:46:40