Multiple records UPDATE overwrite TRIGGERS modification in the same table. [CORE6005]

[firebird-automations]

Submitted by: Javier Fernández Castillejo (jfcastillejo)

CREATE TABLE TEST_UPDATE
(
ID INTEGER NOT NULL,
FIELD1 INTEGER,
FIELD2 INTEGER,
CONSTRAINT PK_TEST_UPDATE PRIMARY KEY (ID)
);

SET TERM ^^ ;
CREATE TRIGGER TEST_UPDATE_AU FOR TEST_UPDATE ACTIVE AFTER UPDATE POSITION 0 AS
begin
Update TEST_UPDATE
SET FIELD2=NEW.FIELD1
Where ID=NEW.ID+1;
end ^^
SET TERM ; ^^

Insert into TEST_UPDATE (ID,FIELD1,FIELD2) VALUES (1,0,0);
Insert into TEST_UPDATE (ID,FIELD1,FIELD2) VALUES (2,0,0);
Insert into TEST_UPDATE (ID,FIELD1,FIELD2) VALUES (3,0,0);
Insert into TEST_UPDATE (ID,FIELD1,FIELD2) VALUES (4,0,0);
Insert into TEST_UPDATE (ID,FIELD1,FIELD2) VALUES (5,0,0);

Update TEST_UPDATE
Set Field1=10
Where ID in (2,3);

=== In Firebird 3.0.4 ===
Select *
From TEST_UPDATE;

ID,FIELD1,FIELD2
1,0,0
2,10,0
3,10,0 <== This value must be 10
4,0,10
5,0,0

In Firebird 2.5 is Correct!!!!
ID,FIELD1,FIELD2
1,0,0
2,10,0
3,10,10
4,0,10
5,0,0

[firebird-automations]

Commented by: @asfernandes

There is no ORDER in the UPDATE, so Firebird is free to first update id=3 then id=2.

You're relying on undefined behavior.

[firebird-automations]

Commented by: Javier Fernández Castillejo (jfcastillejo)

The code is just an example. It should not assume that has to know whether TRIGGERS modified later or earlier records, so the UPDATE must use the latest version of records when will do the upgrade.

I must trust that the actions of the TRIGGERS must remain and I do not believe that the behavior should depend on the order in which the records are updated, because in that case, I can not trust that the TRIGGERS are running correctly in all cases .

Sorry for my English.

[firebird-automations]

Commented by: @asfernandes

Ah ok, the UPDATE now run against a stable cursor, i.e., it don't see values updated in triggers.

[firebird-automations]

Commented by: Sean Leyne (seanleyne)

Adriano,

Please clarify which "UPDATE" would have the stable cursor.

I agree with Javier, the 3.x results are not what I would expect.

[firebird-automations]

Modified by: Javier Fernández Castillejo (jfcastillejo)

summary: Multiple records UPDATE overwrite TIGGERS modification in the same table. => Multiple records UPDATE overwrite TRIGGERS modification in the same table.

[firebird-automations]

Commented by: @mrotteveel

It has always been discouraged to manipulate a table in a trigger for that same table.

The problem seems to be that the update for id = 3 will cancel out the effects of the update for id = 2, and it behaves as if the actual executed statement is update set field1 = 10, field2 = field2 where id in (2, 3) where the value of field2 is the value as seen at the start of the statement (field2 = 0). The change done by the trigger is not visible in the update and becomes undone by the update.

Cursor stability is mentioned in the FB 3 release notes: https://www.firebirdsql.org/file/documentation/release_notes/html/en/3_0/rnfb30-compat-sql.html#rnfb30-compat-cursorstability

[firebird-automations]

Commented by: Sean Leyne (seanleyne)

Mark,

If it was the case that the in trigger UPDATE where to set the value of Field1, then I would completely agree with the current functionality.

But the trigger is making a change to a field which is not part of the UPDATE SQL.

Based on this (without knowing the SQL definition of "cursor") the scope of "cursor" stability is wrong, it should not extend to fields not part of the SQL update.

[firebird-automations]

Commented by: @mrotteveel

I'm only trying to explain the observed behavior. This is caused by the fact that an update writes a new record version, and that record version is based on the record as seen by the update statement and due to the cursor stability that is the state of the record as it was at the start of the statement. That means effectively that the update done by the trigger becomes undone because the state wasn't seen by the update statement.

The fact the field isn't referenced in the update doesn't seem to matter because updates in Firebird affect an entire record because of the MVC properties (Firebird doesn't rewrite individual fields).

[firebird-automations]

Commented by: @dyemanov

By the SQL spec, the record set to be modified/removed by the searched UPDATE/DELETE statement is determined and "fixed" before UPDATE/DELETE is executed. This is what we call a "stable cursor". This means that changes performed by UPDATE are not visible to the UPDATE's WHERE clause. This also implicitly means that changes performed by the underlying triggers are also not visible to the UPDATE's WHERE clause.

[firebird-automations]

Commented by: @mrotteveel

Dmitry, given you explicitly mention "not visible to the UPDATE's WHERE clause" and nothing about effects on the update itself, that sounds like you say that the observed behavior is indeed a bug. Is that correct and is this indeed a bug?

[firebird-automations]

Commented by: Javier Fernández Castillejo (jfcastillejo)

Regarding the "Cursor Stability" Effects there is a Note that says ... "The SQL standard stipulates that the MERGE statement must raise an error if multiple matches are found. Firebird is not strict in this regard, but its behavior should be considered undefined in these cases. "

Undefined? In a software? I will try to explain it to the end users.

MySQL / MariaDB directly throws an exception if you try to modify over the same table.

SQLServer executes the example exactly as Firebird 2.5 does.

From my point of view, it is correct for the cursor to scroll through the selected records at the beginning of the sentence regardless of the changes that occur, but the update of the record should always be made starting from the last version of the record of that transaction.

It can also be more chilling if we add to the previous example ...

CREATE TABLE TEST_SUM
(
ID INTEGER NOT NULL,
TOTAL_FIELD2 INTEGER,
CONSTRAINT PK_TEST_SUM PRIMARY KEY (ID)
);

INSERT INTO TEST_SUM (ID, TOTAL_FIELD2) VALUES (0, 0);

SET TERM ^^ ;
ALTER TRIGGER TEST_UPDATE_AU AS
begin
Update TEST_UPDATE
SET FIELD2=NEW.FIELD1
Where ID=NEW.ID+1;
Update TEST_SUM
Set TOTAL_FIELD2=TOTAL_FIELD2+NEW.FIELD2
Where ID=0;
end ^^
SET TERM ; ^^

Update TEST_UPDATE
Set Field1=10
Where ID in (2,3);

=== RESULT - TEST_UPDATE ===
ID FIELD1 FIELD2
-----------------------------------
1 0 0
2 10 0
3 10 0 <=== Undefined??? Ahhh!! understood!!!
4 0 10
5 0 0

FIELD2 SUM=10

=== RESULT - TEST_SUM ===
ID TOTAL_FIELD2
------------------------
0 20 <=== 20? Houston, We have another problem!!!

Sorry for my English.

[firebird-automations]

Commented by: @livius2

Hi all.

I am not core dev but if i can say something about.
I see that fix (if required) is not simple because SQL standard require that

1. update expression is stable and update like
update tab set A=A+5, B=A+4 where A=7;
should work as
update tab set NEW.A=OLD.A+5, NEW.B=OLD.A+4 where OLD.A=7;

2. than update in the trigger should be considered as

update tab set A=A+5, B=A+4 where A=9;
should work as

update tab set NEW.A=OLD.A+5, NEW.B=OLD.A+4 where OLD.A=9;

but here OLD values should be from UPDATE 2 start point of view.
And should see already changed records by update and by trigger fired in point 1 which is not finished yet.

update tab set NEW.A=OLD2.A+5, NEW.B=OLD2.A+4 where OLD2.A=9;

after finishing update from point 2 update from point 1 should see changes form update 2
but only in the triggers? because in where and in update expressions it should see stable view.

if we extend this in point 3,4,5 - as update from trigger 1 run update which run trigger which can run update ....

But as from begining, we should ask what is the trigger? Is this update expression which should be stable?
I suppose that only OLD values should be stable NEW should see any changes performed by any update method.
And this should be applied to the final record version

[firebird-automations]

Commented by: @abzalov

>>but only in the triggers?

There are also selective procedures that perform the update. These procedures can be called from the update statement that updates the same table.

update tab
set field_1 = (select result from proc_with_update_same_tab(1))
where id = 1;

[firebird-automations]

Commented by: @hvlad

The problem is not with stable cursor. The problem is in order in which triggers fired.

See below:

CREATE OR ALTER TRIGGER TEST_UPDATE_AU
FOR TEST_UPDATE ACTIVE AFTER UPDATE
AS
BEGIN
UPDATE TEST_UPDATE
SET FIELD2 = FIELD2 + NEW.FIELD1 ----- changed line
WHERE ID = http://NEW.ID + 1;

UPDATE TEST_SUM
SET TOTAL_FIELD2 = TOTAL_FIELD2 + NEW.FIELD2
WHERE ID = 0;
END
^

INSERT INTO TEST_UPDATE (ID,FIELD1,FIELD2) VALUES (1,0,0);
INSERT INTO TEST_UPDATE (ID,FIELD1,FIELD2) VALUES (2,0,0);
INSERT INTO TEST_UPDATE (ID,FIELD1,FIELD2) VALUES (3,0,0);
INSERT INTO TEST_UPDATE (ID,FIELD1,FIELD2) VALUES (4,0,0);
INSERT INTO TEST_UPDATE (ID,FIELD1,FIELD2) VALUES (5,0,0);

UPDATE TEST_UPDATE SET FIELD1=10
WHERE ID IN (2,3);

SELECT * FROM TEST_UPDATE ORDER BY ID

ID FIELD1 FIELD2
1 0 0
2 10 0
3 10 10
4 0 10
5 0 0

SELECT * FROM TEST_SUM

ID TOTAL_FIELD2
0 30

Now insert records in opposite order:

ROLLBACK

INSERT INTO TEST_UPDATE (ID,FIELD1,FIELD2) VALUES (5,0,0);
INSERT INTO TEST_UPDATE (ID,FIELD1,FIELD2) VALUES (4,0,0);
INSERT INTO TEST_UPDATE (ID,FIELD1,FIELD2) VALUES (3,0,0);
INSERT INTO TEST_UPDATE (ID,FIELD1,FIELD2) VALUES (2,0,0);
INSERT INTO TEST_UPDATE (ID,FIELD1,FIELD2) VALUES (1,0,0);

UPDATE TEST_UPDATE SET FIELD1=10
WHERE ID IN (2,3);

SELECT * FROM TEST_UPDATE ORDER BY ID

ID FIELD1 FIELD2
1 0 0
2 10 0
3 10 10
4 0 20 ----- like it ?
5 0 0

SELECT * FROM TEST_SUM

ID TOTAL_FIELD2
0 40

It was on FB 2.5

So, yes, Houston - you have a problem - don't write such triggers that creates dependecy on the same table and on the physical order of rows.

PS With MS SQL you can't reproduce this as it have no ROW's triggers.
Oracle will not allow it - it raise "table mutated" error, or something like this, IIRC

[firebird-automations]

Commented by: @asfernandes

Depending on the update order, it will not be a "stable cursor problem", but depending on the other it may be.

But I agree this is not a bug but an undefined behavior the user should avoid.