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
Comments
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 ... UPDATE\DELETE executed as FOR SELECT ... 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). The goal of this ticket is to formally describe issue, collect related bug-reports and to fix issue finally. |
Modified by: @hvladassignee: Vlad Khorsun [ hvlad ] |
Commented by: @asfernandes After patches applied til today, there is still related bugs with FOR SELECT. The code below causes an infinity loop: ------------- insert into t values (1)! create or alter procedure sp1 returns (n integer) select * from sp1! |
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. |
Commented by: @asfernandes May be, but after initial bugs Vlad said: "Savepoint mechanism was choosen after long and careful thinking on issue This test case was sent to him, I'm just putting it here in the tracker to not get lost forever. |
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 |
Commented by: @pavel-zotov (may be it is duplicate, but...) CASE_1.recreate table t(id int primary key, f01 int); SQL> update t set f01=(select sum(f01) from t);
============ ============ CASE_2.recreate table t(id int primary key, f01 int); SQL> recreate table t(id int primary key, f01 int);
============ ============ Right result in all cases shoud be the same as in the following EB: |
Commented by: @hvlad Pavel, your case works correctly in FB3, i.e. after update all rows have value 600 in field F01. |
Commented by: @asfernandes Vlad, This code is incorrectly returning 2, 1. The correct is 2, 2. recreate table z1 (n integer)! execute block returns (x1 integer, x2 integer) |
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. |
Commented by: @asfernandes Vlad, I do not know much about savepoints internals. If you say it has no side effect, then go for it. |
Commented by: @hvlad New patch is committed |
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> insert into t(x, y) values(1, null);
============ ============ SQL> commit; SQL> show table t; TEST:SQL> update t set x=y+1;
============ ============ -- aux. testing query to insure that we have "orphan" child key:
============ ============ ============ ============ Compare with 2.5.3.26726 (13-dec-2013):SQL> update t set x=y+1;
============ ============ (no "orphan" childs as we can see) PS. |
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.
|
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. |
Commented by: @pavel-zotov Seems that at least one issue still exists. Test case:SQL> show version;
============ ============ Compare with Oracle:test@ora112g>set null '<null>'
---------- ---------- (same result in MS SQL: all THREE records are updated with NULL). |
Commented by: @pavel-zotov one more sample (maybe related to previous - I don`t know) DDL:create table t(x int, y int); var-1:update t set y=null where 2 not in(select count(*) cnt from t group by y);
======= ============ (that's OK) var-2:update t set y=null where 2 not in( select cnt from v ); X Y Ora 11.2g and MS SQL results for same DDL+DML as in "var-2" are: |
Commented by: @pavel-zotov The following script gives unpredictable result with each run: commit; insert into t select r1, r2
*/ insert into t2 select * from t order by rand(); select * from t; If you run this script several times you'll see that table `t` contains new values int `y` column for every new run. |
Commented by: @hvlad Cases with NOT IN (from March 1 and March 12) should be fixed currently - patch is committed. |
Commented by: @pavel-zotov [hvlad, 09/Jan/14 10:33 AM]: It seems that "ON UPDATE SET NULL" (rather than `CASCADE`) also doesn`t work correctly. Test:recreate table test( update test set id = id + 1 order by id desc; Result on 2.0 -2.5: ID PID Result on 3.0 (checked on WI-T3.0.0.31828): ID PID 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; |
Commented by: @pavel-zotov One more sample, now with MERGE. Test script:recreate table t1(id int); set echo on; insert into t2 (id, x) select id, (select sum(id) from t2) from t1; insert into t2 (id, x) select id, (select min(id) from t2) from t1; insert into t2 (id, x) select id, (select max(id) from t2) from t1; insert into t2 (id, x) select id, (select count(id) from t2) from t1; 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) ); 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) ); 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) ); 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) ); Output:insert into t2 (id, x) select id, (select sum(id) from t2) from t1;
============ ============ insert into t2 (id, x) select id, (select min(id) from t2) from t1;
============ ============ insert into t2 (id, x) select id, (select max(id) from t2) from t1;
============ ============ insert into t2 (id, x) select id, (select count(id) from t2) from t1;
============ ============ 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) );
============ ============ 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) );
============ ============ 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) );
============ ============ 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) );
============ ============ |
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) |
Commented by: @pavel-zotov On 3.0.0.32362 all samples with MERGE that were mentioned here work OK. |
Modified by: @dyemanovstatus: Open [ 1 ] => Resolved [ 5 ] resolution: Fixed [ 1 ] Fix Version: 3.0 RC2 [ 10048 ] |
Modified by: @pavel-zotovstatus: 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. |
Modified by: @pavel-zotovstatus: Resolved [ 5 ] => Resolved [ 5 ] Test Details: Test ("core_3362_basic.fbt") verifies BASIC issues that were accumulated in miscenaleous tickets. => Test ("core_3362_basic.fbt") verifies BASIC issues that were accumulated in miscenaleous tickets. See also test for CORE5794 |
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
The text was updated successfully, but these errors were encountered: