SQLite

View Ticket
Login
Ticket Hash: c1e19e12046d23fe4e23f13fd40eb975be082922
Title: Trigger causes query to compute incorrect result
Status: Fixed Type: Code_Defect
Severity: Severe Priority: Immediate
Subsystem: Unknown Resolution: Fixed
Last Modified: 2019-10-26 16:39:11
Version Found In: 3.30.0
User Comments:
mrigger added on 2019-10-25 08:28:42:

Consider the following test case:

PRAGMA temp.recursive_triggers = true;
CREATE TABLE t0(c0, c1 UNIQUE);
CREATE TRIGGER c DELETE ON t0
	BEGIN INSERT INTO t0(c1) VALUES(1);
END;
INSERT INTO t0(c1) VALUES(0);
REPLACE INTO t0(c1) VALUES (0);
SELECT t0.c1 BETWEEN 0 AND (CASE WHEN 1 THEN 1 ELSE t0.c0 END NOT NULL) FROM t0; -- expected: 1 and 1, actual: 1

Although two rows are present, the expression is only evaluated on one row. That two rows are contained and evaluate to TRUE can be verified with the following query, which fetches two records:

SELECT * FROM t0 WHERE t0.c1 BETWEEN 0 AND (CASE WHEN 1 THEN 1 ELSE t0.c0 END NOT NULL); -- |0 and |1


drh added on 2019-10-25 13:15:15:

The following revised script does a better job of illustrating the problem:

.echo on
.mode quote
PRAGMA temp.recursive_triggers = true;
CREATE TABLE t0(aa, bb);
CREATE UNIQUE INDEX t0bb ON t0(bb);
.imposter t0bb x1
CREATE TRIGGER r1 BEFORE DELETE ON t0
  BEGIN INSERT INTO t0(aa,bb) VALUES(99,1);
END;
INSERT INTO t0(aa,bb) VALUES(10,20);
SELECT rowid, * FROM t0;
SELECT * FROM x1;
.eqp trace
REPLACE INTO t0(aa,bb) VALUES(30,20);
.eqp off
SELECT rowid, * FROM t0;
SELECT * FROM x1;

Here is what is happening: When the REPLACE statement runs, the algorithm is approximately the following:

  1. Select a ROWID for the new row. The ROWID algorithm is to choose a a value that is one more than the largest current ROWID in the table. In this case, the table has a single entry with a ROWID of 1, so the new ROWID is 2.
  2. Construct a record for the t0bb entry for the new record.
  3. Use the record from step 2 to to see if the value of t0.bb (20) conflicts with an existing row in the table. If it does, delete the existing row, and in the process fire DELETE triggers on that row.
  4. Construct the record for the t0 table.
  5. Insert the index record from step 2.
  6. Insert the table record from step 4.

In the test case of the script, the DELETE trigger does indeed fire, and attempts to insert a new record in table t0. The actions of the DELETE trigger the same as for the main line:

  1. SELECT a ROWID for the new entry, using the same algorithm. Note that since the t0 has not been modified since step 1, the same ROWID value of 2 is computed. This is the root to the problem.
  2. Construct an entry for the t0bb index.
  3. Check for conflicts on t0bb and delete any conflicts.
  4. Construct an entry for the table.
  5. Insert the new t0bb entry.
  6. Insert the new table entry.

Note that steps 11 through 16 run as part of step 3, when the DELETE trigger fires. So there are two inserts on the inserts into the index t0bb and two inserts into the table t0.

Now, the index entries inserted on steps 15 and 5 have different keys, and so they both persist. But the table entries inserted by steps 16 and 6 have the same key (the same ROWID) and so the entry inserted at 6 overwrites the one inserted at 16. So at the end, we are left with just one entry in the table, but two entries in the index.

What to do about this?

It is not possible to defer the ROWID generation of step 1 until after step 3, because the ROWID is required for step 2.

We could enhance the OP_NewRowid opcode so that it takes a "lease" on each new ROWID it generates and prevents the same ROWID from being created for the same table more than once within a single statement. But it is difficult to know where to store those leases and when the lease should expire.

We could add code that verifies that the ROWID is still unused after the DELETE trigger fires, and then abort the statement if a conflicting row has popped into existence due to the DELETE trigger.

Another approach might be to take a lock on the table being REPLACE-ed into while running DELETE triggers associated with the REPLACE, to prevent the DELETE triggers from messing with the table state.


drh added on 2019-10-25 23:56:35:

This is really a continuation of ticket [a8a4847a2d96f5de]. The previous fix for that ticket was incomplete. Substantial enhancements to the logic that performance uniqueness constraint checks prior to an INSERT or UPDATE are required. In particular, if:

  1. The REPLACE conflict resolution algorithm is used, either on the statement itself or as the default resolution algorithm of any unique index or uniqueness constraint, and
  1. Recursive triggers are enabled and there are DELETE triggers on the table, or if foreign keys are enabled and there is are foreign key constraints with ON DELETE SET NULL.

Then after all uniqueness constraints are run, if any triggers where fired to accomplish REPLACE conflict resolution, then all uniqueness constraints must be checked a second time. Any failures on the second check will use the ABORT algorithm.

Here is another script that demonstrates this problem in a different way:

PRAGMA recursive_triggers=ON;
CREATE TABLE t1(a, b UNIQUE, c UNIQUE);
INSERT INTO t1(a,b,c) VALUES(1,1,1),(2,2,2),(3,3,3),(4,4,4);
CREATE TRIGGER r1 AFTER DELETE ON t1 WHEN OLD.c<>3 BEGIN
  INSERT INTO t1(rowid,a,b,c) VALUES(100,100,100,3);
END;
REPLACE INTO t1(rowid,a,b,c) VALUES(200,1,2,3);
PRAGMA integrity_check;