If replace triggers are run during uniqueness checking, then rerun all

uniqueness checks a second time using the ABORT algorithm.
Fix for ticket [c1e19e12046d23fe]

FossilOrigin-Name: fbac0c65d8464b126d385262d176864add55452ec9e3d5eb76ffee06e820cb9c

manifest

@@ -1,5 +1,5 @@
-C Performance\soptimization\sin\ssqlite3BtreeCursor().
+C If\sreplace\striggers\sare\srun\sduring\suniqueness\schecking,\sthen\srerun\sall\nuniqueness\schecks\sa\ssecond\stime\susing\sthe\sABORT\salgorithm.\nFix\sfor\sticket\s[c1e19e12046d23fe]
-D 2019-10-25T14:46:05.015
+D 2019-10-26T00:04:21.466
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -487,7 +487,7 @@ F src/hash.c 8d7dda241d0ebdafb6ffdeda3149a412d7df75102cecfc1021c98d6219823b19
 F src/hash.h 9d56a9079d523b648774c1784b74b89bd93fac7b365210157482e4319a468f38
 F src/hwtime.h 747c1bbe9df21a92e9c50f3bbec1de841dc5e5da
 F src/in-operator.md 10cd8f4bcd225a32518407c2fb2484089112fd71
-F src/insert.c 2b930afc0fb4c245916d19f638e4b332f4a6d8a3a7c73c288ec18e25caf46900
+F src/insert.c 83ccd3f1f45f048bca2dbc0f9c34e8f3a57fdcbc43ddec04a7938090ffb85a9e
 F src/legacy.c d7874bc885906868cd51e6c2156698f2754f02d9eee1bae2d687323c3ca8e5aa
 F src/loadext.c 4ddc65ae13c0d93db0ceedc8b14a28c8c260513448b0eb8c5a2ac375e3b6a85d
 F src/main.c 3e01f6a1c96643381b5f9d79e4ff7f2520bc5712197746fb0852283e78cccf66
@@ -1061,7 +1061,7 @@ F test/indexexpr1.test c26c8b352311c1deb30642cd0379e5cb94e416c7e9e0885e92d9e0155
 F test/indexexpr2.test b580f378423bca443ffab47ada677203cfcf8a60f48a8aa20065f27c8f7739b5
 F test/indexfault.test 98d78a8ff1f5335628b62f886a1cb7c7dac1ef6d48fa39c51ec871c87dce9811
 F test/init.test 15c823093fdabbf7b531fe22cf037134d09587a7
-F test/insert.test c0e1b23f6359e06316b3a49f7747c5d65c0b6473619011e4fb86f6801edba6df
+F test/insert.test d15bf96ec2120ecd27cb07be2b44b141a98b4b7f58762a40178f8205f9b9a96a
 F test/insert2.test 4d14b8f1b810a41995f6286b64a6943215d52208
 F test/insert3.test 1b7db95a03ad9c5013fdf7d6722b6cd66ee55e30
 F test/insert4.test 7802ada6ba8738661b9f6c0e26858d3375b40cc7180289fd350644cd7a08fec9
@@ -1848,7 +1848,7 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P 8d964e1c21d4cea699023e02b0616a75c5859dd083c9365cdcbc0676ebbdaae4
+P ea068b099c96b8b9526114732d2a6be186cf381b7329d102778ad25b95510c9e
-R 8536c2fabd9f7098038221269ecc3472
+R cace6824aa8651d52f54d321985ad4c0
 U drh
-Z ac1675b2b1cf8ed8d7717d3ccad9b463
+Z 1fcf2849ed7e21c156487e77513e1cff

manifest.uuid

@@ -1 +1 @@
-ea068b099c96b8b9526114732d2a6be186cf381b7329d102778ad25b95510c9e
+fbac0c65d8464b126d385262d176864add55452ec9e3d5eb76ffee06e820cb9c

src/insert.c

@@ -1319,6 +1319,13 @@ void sqlite3GenerateConstraintChecks(
   int upsertJump = 0;    /* Address of Goto that jumps into upsert subroutine */
   int ipkTop = 0;        /* Top of the IPK uniqueness check */
   int ipkBottom = 0;     /* OP_Goto at the end of the IPK uniqueness check */
+  /* Variables associated with retesting uniqueness constraints after
+  ** replace triggers fire have run */
+  int regTrigCnt;       /* Register used to count replace trigger invocations */
+  int addrRecheck = 0;  /* Jump here to recheck all uniqueness constraints */
+  int lblRecheckOk = 0; /* Each recheck jumps to this label if it passes */
+  Trigger *pTrigger;    /* List of DELETE triggers on the table pTab */
+  int nReplaceTrig = 0; /* Number of replace triggers coded */
 
   isUpdate = regOldData!=0;
   db = pParse->db;
@@ -1483,6 +1490,50 @@ void sqlite3GenerateConstraintChecks(
     }
   }
 
+  /* Determine if it is possible that triggers (either explicitly coded
+  ** triggers or FK resolution actions) might run as a result of deletes
+  ** that happen when OE_Replace conflict resolution occurs. (Call these
+  ** "replace triggers".)  If any replace triggers run, we will need to
+  ** recheck all of the uniqueness constraints after they have all run.
+  ** But on the recheck, the resolution is OE_Abort instead of OE_Replace.
+  **
+  ** If replace triggers are a possibility, then
+  **
+  **   (1) Allocate register regTrigCnt and initialize it to zero.
+  **       That register will count the number of replace triggers that
+  **       fire.  Constraint recheck only occurs if the number if positive.
+  **   (2) Initialize pTrigger to the set of all DELETE triggers.
+  **   (3) Initialize addrRecheck and lblRecheckOk
+  **
+  ** The uniqueness rechecking code will create a series of tests to run
+  ** in a second pass.  The addrRecheck and lblRecheckOk variables are
+  ** used to link together these tests which are separated from each other
+  ** in the generate bytecode.
+  */
+  if( (db->flags & (SQLITE_RecTriggers|SQLITE_ForeignKeys))==0 ){
+    /* There are not DELETE triggers nor FK constraints.  No constraint
+    ** rechecks are needed. */
+    pTrigger = 0;
+    regTrigCnt = 0;
+  }else{
+    if( db->flags&SQLITE_RecTriggers ){
+      pTrigger = sqlite3TriggersExist(pParse, pTab, TK_DELETE, 0, 0);
+      regTrigCnt = pTrigger!=0 || sqlite3FkRequired(pParse, pTab, 0, 0);
+    }else{
+      pTrigger = 0;
+      regTrigCnt = sqlite3FkRequired(pParse, pTab, 0, 0);
+    }
+    if( regTrigCnt ){
+      /* Replace triggers might exist.  Allocate the counter and
+      ** initialize it to zero. */
+      regTrigCnt = ++pParse->nMem;
+      sqlite3VdbeAddOp2(v, OP_Integer, 0, regTrigCnt);
+      VdbeComment((v, "trigger count"));
+      lblRecheckOk = sqlite3VdbeMakeLabel(pParse);
+      addrRecheck = lblRecheckOk;
+    }
+  }
+
   /* If rowid is changing, make sure the new rowid does not previously
   ** exist in the table.
   */
@@ -1572,17 +1623,12 @@ void sqlite3GenerateConstraintChecks(
         ** to run without a statement journal if there are no indexes on the
         ** table.
         */
-        Trigger *pTrigger = 0;
+        if( regTrigCnt ){
-        if( db->flags&SQLITE_RecTriggers ){
-          pTrigger = sqlite3TriggersExist(pParse, pTab, TK_DELETE, 0, 0);
-        }
-        if( pTrigger || sqlite3FkRequired(pParse, pTab, 0, 0) ){
           sqlite3MultiWrite(pParse);
           sqlite3GenerateRowDelete(pParse, pTab, pTrigger, iDataCur, iIdxCur,
                                    regNewData, 1, 0, OE_Replace, 1, -1);
-          sqlite3VdbeAddOp3(v, OP_NotExists, iDataCur, addrRowidOk, regNewData);
+          sqlite3VdbeAddOp2(v, OP_AddImm, regTrigCnt, 1); /* incr trigger cnt */
-          VdbeCoverage(v);
+          nReplaceTrig++;
-          sqlite3RowidConstraint(pParse, OE_Abort, pTab);
         }else{
 #ifdef SQLITE_ENABLE_PREUPDATE_HOOK
           assert( HasRowid(pTab) );
@@ -1632,6 +1678,7 @@ void sqlite3GenerateConstraintChecks(
     int regR;            /* Range of registers holding conflicting PK */
     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 */
 
     if( aRegIdx[ix]==0 ) continue;  /* Skip indices that do not change */
     if( pUpIdx==pIdx ){
@@ -1745,6 +1792,7 @@ void sqlite3GenerateConstraintChecks(
 
     /* Check to see if the new index entry will be unique */
     sqlite3VdbeVerifyAbortable(v, onError);
+    addrConflictCk = 
       sqlite3VdbeAddOp4Int(v, OP_NoConflict, iThisCur, addrUniqueOk,
                            regIdx, pIdx->nKeyCol); VdbeCoverage(v);
 
@@ -1829,23 +1877,59 @@ void sqlite3GenerateConstraintChecks(
         break;
       }
       default: {
-        Trigger *pTrigger = 0;
+        int nConflictCk;   /* Number of opcodes in conflict check logic */
-        int bRetryConstraintCheck = 0;
+
         assert( onError==OE_Replace );
-        if( db->flags&SQLITE_RecTriggers ){
+        nConflictCk = sqlite3VdbeCurrentAddr(v) - addrConflictCk;
-          pTrigger = sqlite3TriggersExist(pParse, pTab, TK_DELETE, 0, 0);
+        if( regTrigCnt ){
-        }
-        if( pTrigger || sqlite3FkRequired(pParse, pTab, 0, 0) ){
           sqlite3MultiWrite(pParse);
-          bRetryConstraintCheck = 1;
+          nReplaceTrig++;
         }
         sqlite3GenerateRowDelete(pParse, pTab, pTrigger, iDataCur, iIdxCur,
             regR, nPkField, 0, OE_Replace,
             (pIdx==pPk ? ONEPASS_SINGLE : ONEPASS_OFF), iThisCur);
-        if( bRetryConstraintCheck ){
+        if( regTrigCnt ){
-          sqlite3VdbeAddOp4Int(v, OP_NoConflict, iThisCur, addrUniqueOk,
+          VdbeOp *pOp;     /* Conflict check opcode to copy */
-                               regIdx, pIdx->nKeyCol); VdbeCoverage(v);
+          int p2;          /* New P2 value for copied conflict check opcode */
+          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, 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 */
+          pOp = sqlite3VdbeGetOp(v, addrConflictCk);
+          while( nConflictCk>0 && !db->mallocFailed ){
+            if( sqlite3OpcodeProperty[pOp->opcode]&OPFLG_JUMP ){
+              p2 = lblRecheckOk;
+            }else{
+              p2 = pOp->p2;
+            }
+            if( pOp->opcode!=OP_IdxRowid ){
+              sqlite3VdbeAddOp4(v, pOp->opcode, pOp->p1, p2, pOp->p3,
+                                pOp->p4.z, pOp->p4type);
+              sqlite3VdbeChangeP5(v, pOp->p5);
+            }
+            nConflictCk--;
+            pOp++;
+          }
+          /* If the retest fails, issue an abort */
           sqlite3UniqueConstraint(pParse, OE_Abort, pIdx);
+
+          sqlite3VdbeJumpHere(v, addrBypass); /* Terminate the recheck bypass */
         }
         seenReplace = 1;
         break;
@@ -1867,6 +1951,25 @@ void sqlite3GenerateConstraintChecks(
     sqlite3VdbeJumpHere(v, ipkBottom);
   }
 
+  /* Recheck all uniqueness constraints after replace triggers have run */
+  testcase( regTrigCnt!=0 && nReplaceTrig==0 );
+  if( nReplaceTrig ){
+    sqlite3VdbeAddOp2(v, OP_IfNot, regTrigCnt, lblRecheckOk);VdbeCoverage(v);
+    if( !pPk ){
+      if( isUpdate ){
+        sqlite3VdbeAddOp3(v, OP_Eq, regNewData, addrRecheck, regOldData);
+        sqlite3VdbeChangeP5(v, SQLITE_NOTNULL);
+        VdbeCoverage(v);
+      }
+      sqlite3VdbeAddOp3(v, OP_NotExists, iDataCur, addrRecheck, regNewData);
+      VdbeCoverage(v);
+      sqlite3RowidConstraint(pParse, OE_Abort, pTab);
+    }else{
+      sqlite3VdbeGoto(v, addrRecheck);
+    }
+    sqlite3VdbeResolveLabel(v, lblRecheckOk);
+  }
+
   /* Generate the table record */
   if( HasRowid(pTab) ){
     int regRec = aRegIdx[ix];

test/insert.test

@@ -519,8 +519,29 @@ do_catchsql_test insert-16.6 {
 } {1 {UNIQUE constraint failed: p1.b}}
 integrity_check insert-16.7
 
-
+# 2019-10-25 ticket c1e19e12046d23fe
-
+do_catchsql_test insert-17.1 {
+  PRAGMA temp.recursive_triggers = true;
+  DROP TABLE IF EXISTS t0;
+  CREATE TABLE t0(aa, bb);
+  CREATE UNIQUE INDEX t0bb ON t0(bb);
+  CREATE TRIGGER "r17.1" BEFORE DELETE ON t0
+    BEGIN INSERT INTO t0(aa,bb) VALUES(99,1);
+  END;
+  INSERT INTO t0(aa,bb) VALUES(10,20);
+  REPLACE INTO t0(aa,bb) VALUES(30,20);
+} {1 {UNIQUE constraint failed: t0.rowid}}
+integrity_check insert-17.2
+do_catchsql_test insert-17.3 {
+  DROP TABLE IF EXISTS t1;
+  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 "r17.3" 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);
+} {1 {UNIQUE constraint failed: t1.c}}
+integrity_check insert-17.4
 
 
 finish_test