Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed data not visible in WHEN-section if exception occured inside SP that has been called from this code [CORE4483] #4803

Closed
firebird-issue-importer opened this issue Jul 4, 2014 · 22 comments

Comments

@firebird-issue-importer
Copy link

@firebird-issue-importer firebird-issue-importer commented Jul 4, 2014

Submitted by: @pavel-zotov

Assigned to: @aafemt

Votes: 1

1. DDL:

create or alter procedure dlock(a_id int) as begin end;
recreate table tlog(id int);
recreate table dlist(id int primary key);
commit;
insert into dlist values(1);
commit;
set term ^;
create or alter procedure dlock(a_id int) as
declare v_id int;
begin
update dlist set id = id where id = :a_id;
--select id from dlist where id = :a_id for update with lock into v_id;
end
^set term ;^
commit;

2. Create THREE databases on:
1) LI-V2.5.3.26744 (yes, FB 2.5)
2) LI-T3.0.0.31202
3) LI-T3.0.0.31208
- and apply this DDL on them.

3. Create the following test (named e.g. 'catch.sql'):

show version;
rollback;
set transaction no wait;
set term ^;
execute block as
begin
delete from tlog;
insert into tlog(id) values(1);
rdb$set_context('USER_TRANSACTION', 'CNT_IN_MAIN_CODE',(select count(*) from tlog));

\-\-update dlist set id = id order by id rows 1;
execute procedure dlock\(1\);

when any do
begin
rdb$set_context('USER_TRANSACTION','CNT_IN_WHEN_SECTION',(select count(*) from tlog));
exception;
end
end
^ set term ;^
set list on;
select
rdb$get_context('USER_TRANSACTION','CNT_IN_MAIN_CODE') as CNT_IN_MAIN_CODE
,rdb$get_context('USER_TRANSACTION','CNT_IN_WHEN_SECTION') as CNT_IN_WHEN_SECTION
from rdb$database;
rollback;

The column CNT_IN_WHEN_SECTION will contain the number of rows in the table TLOG which should be seen inside WHEN-section.
Obviously, it should be equal to 1.

4. Open three times a couple of ISQL sessions (for each of server versions listed above) and do:

4.1) session #⁠1:
SQL> select id from dlist; -- only to ensure that at least one row exists;
SQL> update dlist set id = id;

4.2) session #⁠2:
SQL> in catch.sql;

I have the following results in session #⁠2 (error "update conflicts" message was skipped):

1.LI-V2.5.3.26744:

CNT_IN_MAIN_CODE 1
CNT_IN_WHEN_SECTION 0

2. LI-T3.0.0.31202:

CNT_IN_MAIN_CODE 1
CNT_IN_WHEN_SECTION 1

3. LI-T3.0.0.31208:

CNT_IN_MAIN_CODE 1
CNT_IN_WHEN_SECTION 0

The result in LI-T3.0.0.31202 seems to me solely correct.
Something was broken after this build #⁠31202 ?

PS.

No problem found if we change in DML script ("catch.sql"):

execute procedure dlock\(1\);

to:

update dlist set id = id order by id rows 1;

(i.e. this trouble is somehow related to call of SP)

Commits: 27395a0 FirebirdSQL/fbt-repository@0ae7bde

====== Test Details ======

Test contains two examples, both taken from discuss with dimitr, see letter 02-feb-2015 23:47.
Checked on WI-T4.0.0.127 - works OK.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 4, 2014

Commented by: @pavel-zotov

PPS. Changed priority to critical: can`t continue developing of OLTP-emulation test because of this :(

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 4, 2014

Modified by: @pavel-zotov

priority: Major [ 3 ] => Critical [ 2 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 4, 2014

Commented by: @aafemt

This bug is fixed in my workcopy, patch was already submitted to firebird-devel.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 4, 2014

Commented by: @pavel-zotov

When (approx.) this patch will be applied to trunk ?

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 4, 2014

Commented by: @aafemt

I betted with Paul B. on "never". So far I'm winning.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 4, 2014

Commented by: @pavel-zotov

Sorry, I have to say that my build #⁠31202 has been changed by special patch of Dimitry Sibiryakov related to savepoints. And SD just told me that he intentionally fixed this bug on this patch.
Build #⁠31155 (~ beginning of june 2014) also has this problem - so, this seems to be common for all builds.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 4, 2014

Commented by: @pavel-zotov

> I betted with Paul B. on "never". So far I'm winning.

It's terrible answer :-)
How to save ("fix") in LOG table results which were added in *this* table at the beginning of transaction in the start of proc P_01 but some exception occured in any procedure which has been called from P_01 ?..

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 5, 2014

Commented by: @pavel-zotov

One more sample, can be done in single session.

DDL:

create or alter procedure p_03(a_id int) as begin end;
create or alter procedure p_02(a_id int) as begin end;
create or alter procedure p_01(a_id int) as begin end;

recreate table tlog(unit varchar(31));

recreate table dlist( id int primary key, s varchar(3) );
commit;

insert into dlist values(1, 'a');
commit;

set term ^;
create or alter procedure p_03(a_id int) as
begin

insert into tlog\(unit\) values\('p\_03'\);
rdb$set\_context\('USER\_TRANSACTION', 'CNT\_IN\_P03\_CODE',\(select count\(\*\) from tlog\)\); \-\- must be 4

update dlist set s = s \|\| '3' where id = :a\_id;

when any do
begin
rdb$set_context('USER_TRANSACTION', 'CNT_IN_P03_WHEN',(select count(*) from tlog)); -- must be 4
exception;
end
end
^
create or alter procedure p_02(a_id int) as
begin

insert into tlog\(unit\) values\('p\_02'\);
rdb$set\_context\('USER\_TRANSACTION', 'CNT\_IN\_P02\_CODE',\(select count\(\*\) from tlog\)\); \-\- must be 3

update dlist set s = s \|\| '2' where id = :a\_id;
execute procedure p\_03\(a\_id\);

when any do
begin
rdb$set_context('USER_TRANSACTION', 'CNT_IN_P02_WHEN',(select count(*) from tlog)); -- must be 3
exception;
end
end
^
create or alter procedure p_01(a_id int) as
begin

insert into tlog\(unit\) values\('p\_01'\);
rdb$set\_context\('USER\_TRANSACTION', 'CNT\_IN\_P01\_CODE',\(select count\(\*\) from tlog\)\); \-\- must be 2

update dlist set s = s \|\| '1' where id = :a\_id;
execute procedure p\_02\(a\_id\);

when any do
begin
rdb$set_context('USER_TRANSACTION', 'CNT_IN_P01_WHEN',(select count(*) from tlog)); -- must be 2
exception;
end
end
^
set term ;^
commit;

Test (will raise exception 'string overflow' but it is suppressed in main code):

rollback;
set transaction no wait;
set term ^;
execute block as
begin
delete from tlog;
insert into tlog(unit) values('main');
rdb$set_context('USER_TRANSACTION', 'CNT_IN_MAIN_CODE',(select count(*) from tlog));

execute procedure p\_01\(1\);

when any do
begin
rdb$set_context('USER_TRANSACTION','CNT_IN_MAIN_WHEN',(select count(*) from tlog));
--exception;
end
end
^ set term ;^
set list on;
select
rdb$get_context('USER_TRANSACTION', 'CNT_IN_MAIN_CODE') as cnt_in_main
,rdb$get_context('USER_TRANSACTION', 'CNT_IN_P01_CODE') as cnt_p01_CODE
,rdb$get_context('USER_TRANSACTION', 'CNT_IN_P01_WHEN') as cnt_p01_WHEN

,rdb$get_context('USER_TRANSACTION', 'CNT_IN_P02_CODE') as cnt_p02_CODE
,rdb$get_context('USER_TRANSACTION', 'CNT_IN_P02_WHEN') as cnt_p02_WHEN

,rdb$get_context('USER_TRANSACTION', 'CNT_IN_P03_CODE') as cnt_p03_CODE
,rdb$get_context('USER_TRANSACTION', 'CNT_IN_P03_WHEN') as cnt_p03_WHEN

,rdb$get_context('USER_TRANSACTION', 'CNT_IN_MAIN_WHEN') as cnt_main_WHEN
from rdb$database;
rollback;

Result:

CNT_IN_MAIN 1

CNT_P01_CODE 2
CNT_P01_WHEN 1 -- wrong

CNT_P02_CODE 3
CNT_P02_WHEN 2 -- wrong

CNT_P03_CODE 4
CNT_P03_WHEN 4 --Ok!

CNT_MAIN_WHEN 0 -- wrong

So, we can see all 4 rows of table TLOG in WHEN-section only in 'deepest' level, P_03.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 5, 2014

Commented by: @dyemanov

I'm not sure this is a bug. You wrongly assume that in P_01 and P_02 only "execute procedure" will be rolled back before entering the WHEN block, while in fact the entire block is rolled back.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 5, 2014

Commented by: @pavel-zotov

> while in fact the entire block is rolled back.

If so, *every* procedure in the last example (P_01...P_03) should rollback changes in its WHEN-section, regardless of this SP 'deepness'. Including P_03: it should *not* see record which has been inserted by this SP.
But P_03 *sees* this record, it show CNT_P03_CODE = CNT_P03_WHEN = 4.
Why ?

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 5, 2014

Commented by: @dyemanov

If error is raised during DML statement, this statement *only* is rolled back and then WHEN block is executed. This is what happens in P_03. If error is raised elsewhere (including "execute procedure"), then the entire block is rolled back and then WHEN block is executed. This is what happens in P_01 and P_02.

We can surely improve / fix the "execute procedure" case. But the issue is actually wider, compare:

(1)

begin
insert into tlog(unit) values('1');
update tlog set unit = null where 0/0 = 0; -- throws division by zero
when any do
-- only update is rolled back, tlog contains one row
end

(2)

begin
insert into tlog(unit) values('1');
some_var = 0/0; -- throws division by zero
when any do
-- insert is also rolled back, tlog contains no rows
end

The problem is that we cannot wrap every statement with a savepoint frame, it would kill the performance. So only insert/update/delete are wrapped.

So generally, you cannot control what block changes can be seen inside a WHEN block. Relying on that is not robust, you should not use such application logic.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 5, 2014

Commented by: @aafemt

Dmitry, there is no need to wrap evey statement to a savepoint, it is enough to control savepoint's number to rollback to. (Which, actually, kills both CORE4483 and CORE4424 in one shoot.) Try your testcases against build with my patch and you'll see how it works.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 5, 2014

Commented by: @dyemanov

I know, but I'm not sure it works in all cases.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 5, 2014

Commented by: @aafemt

Quoting Vlad: "you have a testcase, of course, don't you?.."
I, personally, failed to imagine a case when it won't work. When execution flow is entering WHEN handler, all successfully ended nested savepoints are already merged into the block's one. All nested savepoints that left belong to failed statement and must be rollbacked. After that database state will be exactly expected.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 5, 2015

Modified by: @dyemanov

Version: 3.0 Beta 2 [ 10586 ]

Version: 2.5.4 [ 10585 ]

Version: 2.5.3 Update 1 [ 10650 ]

Version: 2.1.7 [ 10651 ]

Version: 3.0 Beta 1 [ 10332 ]

Version: 2.5.3 [ 10461 ]

Version: 2.1.6 [ 10460 ]

Version: 3.0 Alpha 1 [ 10331 ]

Version: 2.5.2 Update 1 [ 10521 ]

Version: 2.1.5 Update 1 [ 10522 ]

Version: 2.5.2 [ 10450 ]

Version: 2.5.1 [ 10333 ]

Version: 2.5.0 [ 10221 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 2, 2016

Commented by: @aafemt

Fixed in HEAD.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 2, 2016

Modified by: @aafemt

Fix Version: 4.0 Initial [ 10621 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 2, 2016

Modified by: @dyemanov

Fix Version: 4.0 Alpha 1 [ 10731 ]

Fix Version: 4.0 Initial [ 10621 ] =>

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 31, 2016

Modified by: @dyemanov

assignee: Dimitry Sibiryakov [ aafemt ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 1, 2016

Modified by: @aafemt

status: Open [ 1 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 10, 2016

Modified by: @pavel-zotov

status: Resolved [ 5 ] => Resolved [ 5 ]

QA Status: Done successfully

Test Details: Test contains two examples, both taken from discuss with dimitr, see letter 02-feb-2015 23:47.
Checked on WI-T4.0.0.127 - works OK.

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 10, 2016

Modified by: @pavel-zotov

status: Resolved [ 5 ] => Closed [ 6 ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment