]> git.ipfire.org Git - thirdparty/sqlite.git/commitdiff
After a DELETE trigger is fired by REPLACE conflict handling, retest all UNIQUE const...
authordan <Dan Kennedy>
Sat, 18 Jul 2026 19:31:56 +0000 (19:31 +0000)
committerdan <Dan Kennedy>
Sat, 18 Jul 2026 19:31:56 +0000 (19:31 +0000)
FossilOrigin-Name: e2ab286bb713e1358ae9a1d50dc894362d0ea6f1a2e8467c587f42f0e483dd3a

manifest
manifest.uuid
src/insert.c
test/upsert1.test

index 15ec56e3295273a38e3c325662699980aea28f2f..9a17e98c057c8bdf744564aed62cb6f0a8630197 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Enhancements\sto\snew\stest\scases,\sfor\simproved\sreadability.
-D 2026-07-18T09:04:36.385
+C After\sa\sDELETE\strigger\sis\sfired\sby\sREPLACE\sconflict\shandling,\sretest\sall\sUNIQUE\sconstraints\sto\sensure\sthat\sthe\srow\sbeing\sinserted\sor\supdated\sdoes\snot\sconflict\swith\sone\swritten\sby\sthe\sDELETE\strigger.\sAbort\sthe\sstatement\sif\sit\sdoes.\sPossible\sfix\sfor\sforum\spost\s[forum:/info/2026-07-18T03:35:48Z\s|\s2026-07-18T03:35:48Z].
+D 2026-07-18T19:31:56.512
 F .fossil-settings/binary-glob 61195414528fb3ea9693577e1980230d78a1f8b0a54c78cf1b9b24d0a409ed6a x
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
@@ -702,7 +702,7 @@ F src/hash.c 03c8c0f4be9e8bcb6de65aa26d34a61d48a9430747084a69f9469fbb00ea52ca
 F src/hash.h 46b92795a95bfefb210f52f0c316e9d7cdbcdd7e7fcfb0d8be796d3a5767cddf
 F src/hwtime.h 5cb15147c8583d0fc4748e1c12ea6f38c9deaeefa147a4d8d379fd9bc81fee9a
 F src/in-operator.md 10cd8f4bcd225a32518407c2fb2484089112fd71
-F src/insert.c 71776cca5bf05b05b1427fa1dc1b0d391e7f3805abaf9b6f3c24ec7350040838
+F src/insert.c 64c22744bce3b1e9630288043c1967a60b4f02c8fc7707fdb10d13c1ec30d297
 F src/json.c e4af4d799e4e7630c633b3b356d65e92d1781e42343ab77b8ef747c932762012
 F src/legacy.c d7874bc885906868cd51e6c2156698f2754f02d9eee1bae2d687323c3ca8e5aa
 F src/loadext.c 5cd4cd7ec6e1a46416806899086363fbf95eecd3bfb9a388b436c9a77243c902
@@ -1947,7 +1947,7 @@ F test/upfrom2.test 66f3ebf721b3cebd922faee5c386bf244f816d416b57c000753ff51af623
 F test/upfrom3.test 6130f24ebf97f5ea865e5d2a14a2d543fe5428a62e87cc60f62d875e45c1f5f0
 F test/upfrom4.test 78f742a6577c91a7a55c64edb8811004e7c6aa99b8d57b2320f70a918c357807
 F test/upfromfault.test 3a10075a0043f0c4fad6614b2c371f88a8ba5a4acab68b907438413865d6a8d6
-F test/upsert1.test 107290c1f7c4d504fa8190e479841ad29403af529f1cefef5c1fd501a6a584e1
+F test/upsert1.test d29de66c2a034a2ed83a3e26e693e9d02bd3b6e0e67dcde62631c6e15023ca79
 F test/upsert2.test 720e94d09f7362a282bc69b3c6b83d51daeaaf0440eb4920a08b86518b8c7496
 F test/upsert3.test 88d7d590a1948a9cb6eac1b54b0642f67a9f35a1fc0f19b200e97d5d39e3179c
 F test/upsert4.test 25d2a1da92f149331ae0c51ca6e3eee78189577585eab92de149900d62994fa5
@@ -2215,8 +2215,8 @@ F tool/warnings-clang.sh bbf6a1e685e534c92ec2bfba5b1745f34fb6f0bc2a362850723a9ee
 F tool/warnings.sh a554d13f6e5cf3760f041b87939e3d616ec6961859c3245e8ef701d1eafc2ca2
 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f
 F tool/winmain.c 00c8fb88e365c9017db14c73d3c78af62194d9644feaf60e220ab0f411f3604c
-P 49b67b7da6bceae3ee87153480e676f53a472b0f6e0ed8d1866ed819e8f64fa1
-R a1baa1ad016fb074a57c9a0c952a57d3
-U drh
-Z a3259deb28c29afa079e0e7d4cb177b8
+P 053e0d1f011d9ff4d57a6e33adf8de9a7a5550a3c5f6a6d4c511f75b85a09a99
+R 732f1b3492a9559cb0f57b5c2b2574f5
+U dan
+Z d92037b0ac0aaa02a191794baee1be8a
 # Remove this line to create a well-formed Fossil manifest.
index 6233bce6127086a6f96e20578e2b698d04d4741d..527b644fb5196f7cc57eb5be80688e691313f1fc 100644 (file)
@@ -1 +1 @@
-053e0d1f011d9ff4d57a6e33adf8de9a7a5550a3c5f6a6d4c511f75b85a09a99
+e2ab286bb713e1358ae9a1d50dc894362d0ea6f1a2e8467c587f42f0e483dd3a
index 15a9a5fe89f84ac91a62c37df111b08e45090a9e..a981968973fb3cdafaa2546fb1cfef8070dbb126 100644 (file)
@@ -2230,6 +2230,22 @@ void sqlite3GenerateConstraintChecks(
       pTrigger = 0;
       regTrigCnt = sqlite3FkRequired(pParse, pTab, 0, 0);
     }
+    if( regTrigCnt ){
+      /* At this point regTrigCnt is non-zero if there are DELETE triggers
+      ** or FK triggers. But we only care about these things if there is
+      ** a chance that a row will be deleted by an ON CONFLICT REPLACE
+      ** constraint. So zero regTrigCnt if no such constraint can be found. */
+      if( overrideError!=OE_Replace ){
+        if( overrideError!=OE_Default ){
+          regTrigCnt = 0;
+        }else if( pkChng==0 || pPk || pTab->keyConf!=OE_Replace ){
+          for(pIdx=pTab->pIndex; pIdx; pIdx=pIdx->pNext){
+            if( pIdx->onError==OE_Replace ) break;
+          }
+          if( pIdx==0 ) regTrigCnt = 0;
+        }
+      }
+    }
     if( regTrigCnt ){
       /* Replace triggers might exist.  Allocate the counter and
       ** initialize it to zero. */
@@ -2405,6 +2421,7 @@ void sqlite3GenerateConstraintChecks(
     int iThisCur;        /* Cursor for this UNIQUE index */
     int addrUniqueOk;    /* Jump here if the UNIQUE constraint is satisfied */
     int addrConflictCk;  /* First opcode in the conflict check logic */
+    int nConflictCk;     /* Number of opcodes in conflict check logic */
 
     if( aRegIdx[ix]==0 ) continue;  /* Skip indices that do not change */
     if( pUpsert ){
@@ -2581,6 +2598,11 @@ void sqlite3GenerateConstraintChecks(
       }
     }
 
+    nConflictCk = sqlite3VdbeCurrentAddr(v) - addrConflictCk;
+    assert( nConflictCk>0 || db->mallocFailed );
+    testcase( nConflictCk<=0 );
+    testcase( nConflictCk>1 );
+
     /* Generate code that executes if the new index entry is not unique */
     assert( onError==OE_Rollback || onError==OE_Abort || onError==OE_Fail
         || onError==OE_Ignore || onError==OE_Replace || onError==OE_Update );
@@ -2606,13 +2628,7 @@ void sqlite3GenerateConstraintChecks(
         break;
       }
       default: {
-        int nConflictCk;   /* Number of opcodes in conflict check logic */
-
         assert( onError==OE_Replace );
-        nConflictCk = sqlite3VdbeCurrentAddr(v) - addrConflictCk;
-        assert( nConflictCk>0 || db->mallocFailed );
-        testcase( nConflictCk<=0 );
-        testcase( nConflictCk>1 );
         if( regTrigCnt ){
           sqlite3MultiWrite(pParse);
           nReplaceTrig++;
@@ -2626,57 +2642,57 @@ void sqlite3GenerateConstraintChecks(
         if( pTrigger && isUpdate ){
           sqlite3VdbeAddOp1(v, OP_CursorUnlock, iDataCur);
         }
-        if( regTrigCnt ){
-          int addrBypass;  /* Jump destination to bypass recheck logic */
+        seenReplace = 1;
+        break;
+      }
+    }
+    if( regTrigCnt ){
+      int addrBypass;  /* Jump destination to bypass recheck logic */
 
-          sqlite3VdbeAddOp2(v, OP_AddImm, regTrigCnt, 1); /* incr trigger cnt */
-          addrBypass = sqlite3VdbeAddOp0(v, OP_Goto);  /* Bypass recheck */
-          VdbeComment((v, "bypass recheck"));
-
-          /* Here we insert code that will be invoked after all constraint
-          ** checks have run, if and only if one or more replace triggers
-          ** fired. */
-          sqlite3VdbeResolveLabel(v, lblRecheckOk);
-          lblRecheckOk = sqlite3VdbeMakeLabel(pParse);
-          if( pIdx->pPartIdxWhere ){
-            /* Bypass the recheck if this partial index is not defined
-            ** for the current row */
-            sqlite3VdbeAddOp2(v, OP_IsNull, regIdx-1, lblRecheckOk);
-            VdbeCoverage(v);
-          }
-          /* Copy the constraint check code from above, except change
-          ** the constraint-ok jump destination to be the address of
-          ** the next retest block */
-          while( nConflictCk>0 ){
-            VdbeOp x;    /* Conflict check opcode to copy */
-            /* The sqlite3VdbeAddOp4() call might reallocate the opcode array.
-            ** Hence, make a complete copy of the opcode, rather than using
-            ** a pointer to the opcode. */
-            x = *sqlite3VdbeGetOp(v, addrConflictCk);
-            if( x.opcode!=OP_IdxRowid ){
-              int p2;      /* New P2 value for copied conflict check opcode */
-              const char *zP4;
-              if( sqlite3OpcodeProperty[x.opcode]&OPFLG_JUMP ){
-                p2 = lblRecheckOk;
-              }else{
-                p2 = x.p2;
-              }
-              zP4 = x.p4type==P4_INT32 ? SQLITE_INT_TO_PTR(x.p4.i) : x.p4.z;
-              sqlite3VdbeAddOp4(v, x.opcode, x.p1, p2, x.p3, zP4, x.p4type);
-              sqlite3VdbeChangeP5(v, x.p5);
-              VdbeCoverageIf(v, p2!=x.p2);
-            }
-            nConflictCk--;
-            addrConflictCk++;
-          }
-          /* If the retest fails, issue an abort */
-          sqlite3UniqueConstraint(pParse, OE_Abort, pIdx);
+      sqlite3VdbeAddOp2(v, OP_AddImm, regTrigCnt, 1); /* incr trigger cnt */
+      addrBypass = sqlite3VdbeAddOp0(v, OP_Goto);  /* Bypass recheck */
+      VdbeComment((v, "bypass recheck"));
 
-          sqlite3VdbeJumpHere(v, addrBypass); /* Terminate the recheck bypass */
+      /* Here we insert code that will be invoked after all constraint
+      ** checks have run, if and only if one or more replace triggers
+      ** fired. */
+      sqlite3VdbeResolveLabel(v, lblRecheckOk);
+      lblRecheckOk = sqlite3VdbeMakeLabel(pParse);
+      if( pIdx->pPartIdxWhere ){
+        /* Bypass the recheck if this partial index is not defined
+        ** for the current row */
+        sqlite3VdbeAddOp2(v, OP_IsNull, regIdx-1, lblRecheckOk);
+        VdbeCoverage(v);
+      }
+      /* Copy the constraint check code from above, except change
+      ** the constraint-ok jump destination to be the address of
+      ** the next retest block */
+      while( nConflictCk>0 ){
+        VdbeOp x;    /* Conflict check opcode to copy */
+        /* The sqlite3VdbeAddOp4() call might reallocate the opcode array.
+        ** Hence, make a complete copy of the opcode, rather than using
+        ** a pointer to the opcode. */
+        x = *sqlite3VdbeGetOp(v, addrConflictCk);
+        if( x.opcode!=OP_IdxRowid ){
+          int p2;      /* New P2 value for copied conflict check opcode */
+          const char *zP4;
+          if( sqlite3OpcodeProperty[x.opcode]&OPFLG_JUMP ){
+            p2 = lblRecheckOk;
+          }else{
+            p2 = x.p2;
+          }
+          zP4 = x.p4type==P4_INT32 ? SQLITE_INT_TO_PTR(x.p4.i) : x.p4.z;
+          sqlite3VdbeAddOp4(v, x.opcode, x.p1, p2, x.p3, zP4, x.p4type);
+          sqlite3VdbeChangeP5(v, x.p5);
+          VdbeCoverageIf(v, p2!=x.p2);
         }
-        seenReplace = 1;
-        break;
+        nConflictCk--;
+        addrConflictCk++;
       }
+      /* If the retest fails, issue an abort */
+      sqlite3UniqueConstraint(pParse, OE_Abort, pIdx);
+
+      sqlite3VdbeJumpHere(v, addrBypass); /* Terminate the recheck bypass */
     }
     sqlite3VdbeResolveLabel(v, addrUniqueOk);
     if( regR!=regIdx ) sqlite3ReleaseTempRange(pParse, regR, nPkField);
@@ -2715,8 +2731,8 @@ void sqlite3GenerateConstraintChecks(
     }else{
       sqlite3VdbeGoto(v, addrRecheck);
     }
-    sqlite3VdbeResolveLabel(v, lblRecheckOk);
   }
+  if( regTrigCnt ) sqlite3VdbeResolveLabel(v, lblRecheckOk);
 
   /* Generate the table record */
   if( HasRowid(pTab) ){
index 40b9d5d0f8ea3337a7d19c3c3fff81e6190eb382..0b0287e9ae3667835442abb9967c2b18070d338a 100644 (file)
@@ -405,4 +405,32 @@ do_execsql_test upsert1-1420 {
   SELECT * FROM t2 ORDER BY a;
 } {2 20 200 4 40 400 99 30 999}
 
+# This is a slightly simpler version of the test above (1417-1420). Before
+# the bug was fixed, this would:
+#
+#   1) Checks there are no instances of 'HELLO' in the index on t1(b),
+#   2) Deletes the row from t1 as a result of the REPLACE on the PK,
+#   3) Fires the trigger, inserting ('HELLO', 200) into index t1(b).
+#   4) Finishes the insert without rechecking t1(b), inserting 
+#      ('HELLO', 100). Corruption.
+#
+reset_db
+do_execsql_test upsert-1421 {
+  PRAGMA recursive_triggers = ON;
+
+  CREATE TABLE t1(
+      a PRIMARY KEY ON CONFLICT REPLACE,
+      b UNIQUE
+  );
+  INSERT INTO t1 VALUES(100, 'abcde');
+
+  CREATE TRIGGER tr1 AFTER DELETE ON t1 BEGIN
+    INSERT INTO t1 VALUES(200, 'HELLO');
+  END;
+
+  INSERT INTO t1 VALUES(100, 'HELLO');
+
+  SELECT * FROM t1;
+}
+
 finish_test