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

Cursors should ignore changes made by the same statement [CORE3362] #3728

Closed
firebird-automations opened this issue Feb 25, 2011 · 35 comments
Closed

Comments

@firebird-automations
Copy link
Collaborator

Submitted by: @hvlad

Is related to CORE92
Is related to CORE634
Is related to CORE2799
Is related to CORE3317
Is duplicated by CORE4697
Is related to CORE5305

Votes: 4

Commits: 2dc879a 6ca0cbb 2a67db1 513ce5e 60dc5dd 69df3d7 4cb5794 07e245a FirebirdSQL/fbt-repository@a4252bc FirebirdSQL/fbt-repository@07678ef FirebirdSQL/fbt-repository@2b6e11e

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

Test ("core_3362_basic.fbt") verifies BASIC issues that were accumulated in miscenaleous tickets.
More complex cases (which involve not only SQL but also PSQL features) will
follow in separate .fbt in order to keep size of each test in reasonable limits.

See also test for CORE5794

@firebird-automations
Copy link
Collaborator Author

Modified by: @hvlad

Link: This issue is related to CORE92 [ CORE92 ]

@firebird-automations
Copy link
Collaborator Author

Modified by: @hvlad

Link: This issue depends on CORE634 [ CORE634 ]

@firebird-automations
Copy link
Collaborator Author

Modified by: @hvlad

Link: This issue depends on CORE634 [ CORE634 ] =>

@firebird-automations
Copy link
Collaborator Author

Modified by: @hvlad

Link: This issue is related to CORE634 [ CORE634 ]

@firebird-automations
Copy link
Collaborator Author

Modified by: @hvlad

Link: This issue is related to CORE2799 [ CORE2799 ]

@firebird-automations
Copy link
Collaborator Author

Modified by: @hvlad

Link: This issue is related to CORE3317 [ CORE3317 ]

@firebird-automations
Copy link
Collaborator Author

Commented by: @hvlad

Firebird used implicit cursors for INSERT FROM SELECT, UPDATE and DELETE statements.

For example INSERT FROM SELECT executed internally as

FOR SELECT ...
DO INSERT ...

UPDATE\DELETE executed as

FOR SELECT ...
AS CURSOR c
DO UPDATE\DELETE ... WHERE CURRENT OF c

where FOR SELECT forms an implicit cursor based on UPDATE\DELETE statement.

SQL Standard (introduced after initial Interbase implementation of DML statements) requires that set of records which will be affected by UPDATE\DELETE statements should be evaluated first and real action (UPDATE or DELETE) should use this stable recordset. I.e. action implementation should not affect updating recordset in any way.

Firebird implementation not satisfied this requirement because no intermediate record set is created (no bookmarks is created, etc).
There is many examples of such non-standard behavior, some of them already reported in related tracker tickets.

The goal of this ticket is to formally describe issue, collect related bug-reports and to fix issue finally.

@firebird-automations
Copy link
Collaborator Author

Modified by: @hvlad

assignee: Vlad Khorsun [ hvlad ]

@firebird-automations
Copy link
Collaborator Author

Commented by: @asfernandes

After patches applied til today, there is still related bugs with FOR SELECT. The code below causes an infinity loop:

-------------
create table t (
n integer
)!

insert into t values (1)!
insert into t values (2)!
insert into t values (3)!
insert into t values (4)!

create or alter procedure sp1 returns (n integer)
as
begin
for select n from t into n
do
begin
insert into t values (:n);
suspend;
end
end!

select * from sp1!
-------------

@firebird-automations
Copy link
Collaborator Author

Commented by: @dyemanov

AFAIK, the SQL specification declares stability for IUD[M] statements only. I'm not sure compound statements should count here. In the ideal world it would surely be desirable, but IIRC it's quite hard to implement it now. Vlad will hopefully correct me if I'm wrong.

@firebird-automations
Copy link
Collaborator Author

Commented by: @asfernandes

May be, but after initial bugs Vlad said:

"Savepoint mechanism was choosen after long and careful thinking on issue
and still looks most cheap way to have stable implicit cursors (and FOR SELECT
ones) to me."

This test case was sent to him, I'm just putting it here in the tracker to not get lost forever.

@firebird-automations
Copy link
Collaborator Author

Commented by: @hvlad

I still not closed this ticket exactly by this reason.

We discussed with Alex (privately) possible solution for stability of PSQL-cursors and i found it overcomplicated.

We should decide if we need stable PSQL-cursors at all, if its corresponds to Firebird's PSQL nature, and if related
costs (implementation, run-time performance penalty, increased code complexity) is acceptible.

@firebird-automations
Copy link
Collaborator Author

Commented by: @pavel-zotov

(may be it is duplicate, but...)
I could not find any issues about wrong (and also unpredictable) result of UPDATE in such cases:

CASE_1.

recreate table t(id int primary key, f01 int);
commit;
insert into t values(1, 100);
insert into t values(2, 200);
insert into t values(3, 300);
commit;

SQL> update t set f01=(select sum(f01) from t);
SQL> select * from t;

      ID          F01

============ ============
1 600
2 1100
3 2000

CASE_2.

recreate table t(id int primary key, f01 int);
commit;
-- "reverse" order of values in f01 for same IDs: from big to small
insert into t values(1, 300);
insert into t values(2, 200);
insert into t values(3, 100);
commit;

SQL> recreate table t(id int primary key, f01 int);
SQL> commit;
SQL> insert into t values(1, 300);
SQL> insert into t values(2, 200);
SQL> insert into t values(3, 100);
SQL> commit;
SQL> update t set f01=(select sum(f01) from t);
SQL> select * from t;

      ID          F01

============ ============
1 600
2 900
3 1600

Right result in all cases shoud be the same as in the following EB:
set term ^;
execute block returns(id int,f01 int) as
declare s01 int;
begin
for select sum(f01) from t into s01
do update t set f01=:s01;
for select id,f01 from t into id,f01 do suspend;
end^
set term ;^

@firebird-automations
Copy link
Collaborator Author

Commented by: @hvlad

Pavel,

your case works correctly in FB3, i.e. after update all rows have value 600 in field F01.

@firebird-automations
Copy link
Collaborator Author

Commented by: @asfernandes

Vlad,

This code is incorrectly returning 2, 1. The correct is 2, 2.

recreate table z1 (n integer)!
insert into z1 values (1)!

execute block returns (x1 integer, x2 integer)
as
begin
insert into z1 values (2);
select max(n) from z1 into x1;
x2 = (select max(n) from z1);
suspend;
end!

@firebird-automations
Copy link
Collaborator Author

Commented by: @hvlad

Adriano,

good example.

Looks like we have internal savepoint for statement

select max(n) from z1 into x1;

and have no such savepoint for statement

x2 = (select max(n) from z1);

The simplest way to fix this issue is to add savepoint around (select max(n) from z1). It will be empty and will cost almost no cpu\memory.
What do you think ?

@firebird-automations
Copy link
Collaborator Author

Commented by: @asfernandes

Vlad, I do not know much about savepoints internals.

If you say it has no side effect, then go for it.

@firebird-automations
Copy link
Collaborator Author

Commented by: @hvlad

New patch is committed

@firebird-automations
Copy link
Collaborator Author

Commented by: @pavel-zotov

It seems that we have a trouble in FB-3 when update table with FK constraint that references to itself and have been created with update cascade clause.

DDL:

SQL> create database 'tmp.fdb';
SQL> show version;
ISQL Version: LI-T3.0.0.30813 Firebird 3.0 Alpha 2
Server version:
Firebird/Linux/AMD/Intel/x64 (access method), version "LI-T3.0.0.30813 Firebird 3.0 Alpha 2"
on disk structure version 12.0
SQL> create table t(
CON> x int, y int,
CON> constraint t_pk primary key(x),
CON> constraint t_fk foreign key(y) references t(x) on update cascade -- SELF-REFERENCING
CON> );

SQL> insert into t(x, y) values(1, null);
SQL> insert into t(x, y) values(2, 1 );
SQL> insert into t(x, y) values(3, 2 );
SQL> insert into t(x, y) values(4, 3 );
SQL> insert into t(x, y) values(5, 4 );
SQL> update t set y=5 where x=1; -- "closure" of chain
SQL> select * from t;

       X            Y

============ ============
1 5
2 1
3 2
4 3
5 4

SQL> commit;

SQL> show table t;
X INTEGER Not Null
Y INTEGER Nullable
CONSTRAINT T_FK:
Foreign key (Y) References T (X) On Update Cascade
CONSTRAINT T_PK:
Primary key (X)

TEST:

SQL> update t set x=y+1;
SQL> select * from t;

       X            Y

============ ============
6 5
2 1 -- <<< ??? child key WITHOUT parent ??? <<<
3 2
4 3
5 4

-- aux. testing query to insure that we have "orphan" child key:
SQL> select d.x child_x, d.y child_y, m.x parent_x, m.y parent_y from t d left join t m on d.y=m.x where m.x is null;

 CHILD\_X      CHILD\_Y     PARENT\_X     PARENT\_Y

============ ============ ============ ============
2 1 <null> <null>

Compare with 2.5.3.26726 (13-dec-2013):

SQL> update t set x=y+1;
SQL> select * from t;

       X            Y

============ ============
6 10
7 6
8 7
9 8
10 9

(no "orphan" childs as we can see)

PS.
Attempt to backup and restore database with "orphan" child record in FB-3 leads, of course, to abend:
. . .
gbak:creating indexes
gbak: activating and creating deferred index T_PK
gbak: activating and creating deferred index T_FK
gbak:cannot commit index T_FK
gbak: ERROR:violation of FOREIGN KEY constraint "T_FK" on table "T"
gbak: ERROR: Foreign key reference target does not exist
gbak:committing metadata
gbak:fixing system generators
gbak:finishing, closing, and going home
gbak:Database is not online due to failure to activate one or more indices.
gbak:Run gfix -online to bring database online without active indices.

@firebird-automations
Copy link
Collaborator Author

Commented by: @pavel-zotov

Some days ago I've found a couple more samples which make me think that FB still have something unsolved in cursor stability.

SAMPLE #⁠1.
#⁠#⁠#⁠#⁠#⁠#⁠#⁠#⁠#⁠#⁠
DDL:

recreate table t(x int, y int);
commit;
insert into t values(1, 1);
insert into t values(2, 2);
insert into t values(3, 3);
insert into t values(4, 4);
insert into t values(5, 5);
commit;

DML:

update t set x=null, y=(select c from (select count(x)over() c from t) rows 1);
select * from t;

Result:

  X            Y

======= ============
<null> 5
<null> 4
<null> 3
<null> 2
<null> 1

PS-1. Same test in Oracle 11.2g:

set null '<null>'
update t set x=null, y=( select c from (select count(x)over() c from t) where rownum=1 );
select * from t;

     X          Y

---------- ----------
<null> 5
<null> 5
<null> 5
<null> 5
<null> 5

PS-2. Same test in MS SQL 2005 Dev Edition:

create table t(x int, y int)
go
insert into t values(1,1)
insert into t values(2,2)
insert into t values(3,3)
insert into t values(4,4)
insert into t values(5,5)
go

update t set x=null, y=( select top 1 c from (select count(x)over() c from t) t );
select * from t

x y
NULL 5
NULL 5
NULL 5
NULL 5
NULL 5

SAMPLE #⁠2
#⁠#⁠#⁠#⁠#⁠#⁠#⁠#⁠#⁠#⁠

recreate table t(x int, y int);
commit;
insert into t values(1, 1);
insert into t values(2, 2);
insert into t values(3, 3);
insert into t values(4, 4);
insert into t values(5, 5);
commit;

Firebird:

update t
set y=(select (select sum(x) from t tx where tx.x<=tt.x)
from t tt
order by y desc rows 1
);
select * from t;
X Y
=== ============
1 15
2 1
3 1
4 1
5 1

Same test in Oracle 11.2g:

update t
set y=(select s from (
select (select sum(x) from t tx where tx.x<=tt.x) s
from t tt order by y desc
)
where rownum<2
);
select * from t;

     X          Y

---------- ----------
1 15
2 15
3 15
4 15
5 15

@firebird-automations
Copy link
Collaborator Author

Commented by: @hvlad

Cursor stability should be fixed now.

As for self-referencing FK with UPDATE CASCADE - it was never works correctly and stable cursor just slightly changed already wrong behavior.
The core of the issue is order in which cascading triggers are fired. They should fire after whole base UPDATE is finished while currently
cascading triggers are fired while records are updated. With non self-referencing FK there is no problem with current implementation.

@firebird-automations
Copy link
Collaborator Author

Commented by: @pavel-zotov

Seems that at least one issue still exists.

Test case:

SQL> show version;
ISQL Version: LI-T3.0.0.30889 Firebird 3.0 Alpha 2
Server version:
Firebird/Linux/AMD/Intel/x64 (access method), version "LI-T3.0.0.30889 Firebird 3.0 Alpha 2"
Firebird/Linux/AMD/Intel/x64 (remote server), version "LI-T3.0.0.30889 Firebird 3.0 Alpha 2/tcp (oel64)/P13"
Firebird/Linux/AMD/Intel/x64 (remote interface), version "LI-T3.0.0.30889 Firebird 3.0 Alpha 2/tcp (oel64)/P13"
on disk structure version 12.0
SQL> recreate table t(id int primary key, x int);
SQL> commit;
SQL> insert into t(id, x) select row_number()over(),row_number()over() from rdb$types rows 3;
SQL> commit;
SQL> update t set x=null where x not in(select x from t where x is null);
SQL> select * from t;

      ID            X

============ ============
1 <null>
2 2
3 3

Compare with Oracle:

test@ora112g>set null '<null>'
test@ora112g>set feed off;
test@ora112g>set timing off;
test@ora112g>create table t(id number primary key, x number);
test@ora112g>insert into t values(1, 1);
test@ora112g>insert into t values(2, 2);
test@ora112g>insert into t values(3, 3);
test@ora112g>commit;
test@ora112g>update t set x=null where x not in(select x from t where x is null);
test@ora112g>select * from t;

    ID          X

---------- ----------
1 <null>
2 <null>
3 <null>

(same result in MS SQL: all THREE records are updated with NULL).

@firebird-automations
Copy link
Collaborator Author

Commented by: @pavel-zotov

one more sample (maybe related to previous - I don`t know)

DDL:

create table t(x int, y int);
insert into t values(1, 100);
insert into t values(2, 200);
insert into t values(3, 300);
commit;
create view v as select y,count(*) cnt from t group by y;

var-1:

update t set y=null where 2 not in(select count(*) cnt from t group by y);
select * from t;

  X            Y

======= ============
2 <null>
3 <null>
1 <null>

(that's OK)

var-2:

update t set y=null where 2 not in( select cnt from v );

X Y
= ============
2 <null>
3 <null>
1 100

Ora 11.2g and MS SQL results for same DDL+DML as in "var-2" are:
X Y
-- ----------
1 ***null***
2 ***null***
3 ***null***

@firebird-automations
Copy link
Collaborator Author

Commented by: @pavel-zotov

The following script gives unpredictable result with each run:

commit;
recreate table t(x int, y int);
recreate table t2(x int, y int);
commit;

insert into t select r1, r2
from (select 6-row_number()over() r1, row_number()over() r2 from rdb$types rows 5)
order by rand();
commit;
/*
Table `t` now contains such data:
SQL> select x, y, x+y s from t;

       X            Y                     S

       1            5                     6
       2            4                     6
       3            3                     6
       4            2                     6
       5            1                     6

*/

insert into t2 select * from t order by rand();
commit;
--select * from t2;
merge into t using(select x,y from t2) s on t.x=s.y
when matched
then update set t.y=(select count(*) from t where x<>y);

select * from t;
rollback;

If you run this script several times you'll see that table `t` contains new values int `y` column for every new run.

@firebird-automations
Copy link
Collaborator Author

Commented by: @hvlad

Cases with NOT IN (from March 1 and March 12) should be fixed currently - patch is committed.

@firebird-automations
Copy link
Collaborator Author

Modified by: @hvlad

Link: This issue is duplicated by CORE4697 [ CORE4697 ]

@firebird-automations
Copy link
Collaborator Author

Commented by: @pavel-zotov

[hvlad, 09/Jan/14 10:33 AM]:
> As for self-referencing FK with UPDATE CASCADE - it was never works correctly

It seems that "ON UPDATE SET NULL" (rather than `CASCADE`) also doesn`t work correctly.
And this looks sorrowful because following code works OK on WI-V2.0.7.13318, WI-V2.1.7.18553 and WI-V2.5.5.26870:

Test:

recreate table test(
id int constraint test_pk_id primary key using index test_pk_id,
pid int constraint test_fk_pid2id references test(id)
on update SET NULL
);
commit;
insert into test values( 5, null );
insert into test values( 4, 5 );
insert into test values( 3, 4 );
insert into test values( 2, 3 );
insert into test values( 1, 2 );
commit;
update test set pid=1 where id=5;
commit;

update test set id = id + 1 order by id desc;
select * from test;
rollback;

Result on 2.0 -2.5:

ID PID
== ============
6 <null>
5 <null>
4 <null>
3 <null>
2 <null>

Result on 3.0 (checked on WI-T3.0.0.31828):

ID PID
=== ============
6 <null>
5 5
4 4
3 3
2 2

One need to add 2nd `UPDATE` statement in 3.0 to make values in PID become null, i.e.:

update test set id = id + 1 order by id desc;
update test set id = id + 1 order by id desc;
select * from test;

@firebird-automations
Copy link
Collaborator Author

Commented by: @pavel-zotov

One more sample, now with MERGE.

Test script:

recreate table t1(id int);
recreate table t2(id int, x int);
commit;
insert into t1 values(1);
insert into t1 values(2);
insert into t1 values(3);
commit;

set echo on;

insert into t2 (id, x) select id, (select sum(id) from t2) from t1;
set echo off; select * from t2; rollback; set echo on;

insert into t2 (id, x) select id, (select min(id) from t2) from t1;
set echo off; select * from t2; rollback; set echo on;

insert into t2 (id, x) select id, (select max(id) from t2) from t1;
set echo off; select * from t2; rollback; set echo on;

insert into t2 (id, x) select id, (select count(id) from t2) from t1;
set echo off; select * from t2; rollback; set echo on;

merge into t2 using t1 on http://t1.id=t2.id when not matched then insert (id, x) values(http://t1.id, (select sum(id) from t2) );
set echo off; select * from t2; rollback; set echo on;

merge into t2 using t1 on http://t1.id=t2.id when not matched then insert (id, x) values(http://t1.id, (select min(id) from t2) );
set echo off; select * from t2; rollback; set echo on;

merge into t2 using t1 on http://t1.id=t2.id when not matched then insert (id, x) values(http://t1.id, (select max(id) from t2) );
set echo off; select * from t2; rollback; set echo on;

merge into t2 using t1 on http://t1.id=t2.id when not matched then insert (id, x) values(http://t1.id, (select count(*) from t2) );
set echo off; select * from t2; rollback; set echo on;

Output:

insert into t2 (id, x) select id, (select sum(id) from t2) from t1;
set echo off; select * from t2; rollback; set echo on;

      ID            X 

============ ============
1 <null>
2 <null>
3 <null>
-- (i.e. OK: statement "(select sum(id) from t2)" should not be affected by new rows that appear in t2 while it is filled up)

insert into t2 (id, x) select id, (select min(id) from t2) from t1;
set echo off; select * from t2; rollback; set echo on;

      ID            X 

============ ============
1 <null>
2 <null>
3 <null>
-- OK

insert into t2 (id, x) select id, (select max(id) from t2) from t1;
set echo off; select * from t2; rollback; set echo on;

      ID            X 

============ ============
1 <null>
2 <null>
3 <null>
-- OK

insert into t2 (id, x) select id, (select count(id) from t2) from t1;
set echo off; select * from t2; rollback; set echo on;

      ID            X 

============ ============
1 0
2 0
3 0
-- OK

merge into t2 using t1 on http://t1.id=t2.id when not matched then insert (id, x) values(http://t1.id, (select sum(id) from t2) );
set echo off; select * from t2; rollback; set echo on;

      ID            X 

============ ============
1 <null>
2 1
3 3
-- WRONG ?

merge into t2 using t1 on http://t1.id=t2.id when not matched then insert (id, x) values(http://t1.id, (select min(id) from t2) );
set echo off; select * from t2; rollback; set echo on;

      ID            X 

============ ============
1 <null>
2 1
3 1
-- Seems also wrong...

merge into t2 using t1 on http://t1.id=t2.id when not matched then insert (id, x) values(http://t1.id, (select max(id) from t2) );
set echo off; select * from t2; rollback; set echo on;

      ID            X 

============ ============
1 <null>
2 1
3 2
-- And this also seems wrong...

merge into t2 using t1 on http://t1.id=t2.id when not matched then insert (id, x) values(http://t1.id, (select count(*) from t2) );
set echo off; select * from t2; rollback; set echo on;

      ID            X 

============ ============
1 0
2 1
3 2
-- And this too.

@firebird-automations
Copy link
Collaborator Author

Commented by: @hvlad

MERGE should be fixed now.

It seems there is no more known issues with stable cursor (except of issues with self-referencing FK, which is another story, i think)

@firebird-automations
Copy link
Collaborator Author

Commented by: @pavel-zotov

On 3.0.0.32362 all samples with MERGE that were mentioned here work OK.

@firebird-automations
Copy link
Collaborator Author

Modified by: @dyemanov

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

resolution: Fixed [ 1 ]

Fix Version: 3.0 RC2 [ 10048 ]

@firebird-automations
Copy link
Collaborator Author

Commented by: @hvlad

Cascade updates with self-referencing FK should be fixed now, see CORE5305

@firebird-automations
Copy link
Collaborator Author

Modified by: @hvlad

Link: This issue is related to CORE5305 [ CORE5305 ]

@firebird-automations
Copy link
Collaborator Author

Modified by: @pavel-zotov

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

QA Status: Done with caveats

Test Details: Test ("core_3362_basic.fbt") verifies BASIC issues that were accumulated in miscenaleous tickets.
More complex cases (which involve not only SQL but also PSQL features) will
follow in separate .fbt(s) in order to keep size of each test in reasonable limits.

@firebird-automations
Copy link
Collaborator Author

Modified by: @pavel-zotov

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

Test Details: Test ("core_3362_basic.fbt") verifies BASIC issues that were accumulated in miscenaleous tickets.
More complex cases (which involve not only SQL but also PSQL features) will
follow in separate .fbt(s) in order to keep size of each test in reasonable limits.

=>

Test ("core_3362_basic.fbt") verifies BASIC issues that were accumulated in miscenaleous tickets.
More complex cases (which involve not only SQL but also PSQL features) will
follow in separate .fbt in order to keep size of each test in reasonable limits.

See also test for CORE5794

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