Logic is in place to handle multiple ON CONFLICT clauses, but it does not work.

Any use of ON CONFLICT will likely lead to memory faults.  This is an
incremental check-in to save my place.

FossilOrigin-Name: 155142314feb007d526f8f67723636fd50dc52d1cd4d3a67dd93b105c9d5c2be

manifest

@@ -1,5 +1,5 @@
-C Use\san\siterator\sfor\sthe\sindex\sloop\sin\ssqlite3GenerateConstraintChecks().\nThe\sidea\sis\sthat\sthis\siterator\scan\sbe\senhanced\sto\straverse\sthe\sindexes\sin\nany\sorder,\sas\srequired\sby\smulti-index\sUPSERT.
+C Logic\sis\sin\splace\sto\shandle\smultiple\sON\sCONFLICT\sclauses,\sbut\sit\sdoes\snot\swork.\nAny\suse\sof\sON\sCONFLICT\swill\slikely\slead\sto\smemory\sfaults.\s\sThis\sis\san\nincremental\scheck-in\sto\ssave\smy\splace.
-D 2020-12-10T20:31:25.985
+D 2020-12-11T01:17:06.489
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -501,7 +501,7 @@ F src/hash.c 8d7dda241d0ebdafb6ffdeda3149a412d7df75102cecfc1021c98d6219823b19
 F src/hash.h 9d56a9079d523b648774c1784b74b89bd93fac7b365210157482e4319a468f38
 F src/hwtime.h cb1d7e3e1ed94b7aa6fde95ae2c2daccc3df826be26fc9ed7fd90d1750ae6144
 F src/in-operator.md 10cd8f4bcd225a32518407c2fb2484089112fd71
-F src/insert.c ac236526a7f239d420291fae1bf7ea56c9e54f335598315a0ddf8ebb5a110120
+F src/insert.c c8d4bc0dcde4d9d42a1761e919a6a99668000e16840814f8bb643e161894b27f
 F src/legacy.c d7874bc885906868cd51e6c2156698f2754f02d9eee1bae2d687323c3ca8e5aa
 F src/loadext.c 8c9c8cd2bd8eecdb06d9b6e89de7e9e65bae45cc8fc33609cc74023a5c296067
 F src/main.c 97e9f137354bc1f76dc9bb60a0a24f8c45cf73b33e80d3ee4c64155336fb820d
@@ -545,7 +545,7 @@ F src/shell.c.in e9f674ee4ec6c345679e8a5b16c869c6c59eb1540dd98ac69e4736ecddce009
 F src/sqlite.h.in 0e2b4259e49a0eda54d9118eb18a04fcd60e0727a2fd2c81aade0bf57520e706
 F src/sqlite3.rc 5121c9e10c3964d5755191c80dd1180c122fc3a8
 F src/sqlite3ext.h 61b38c073d5e1e96a3d45271b257aef27d0d13da2bea5347692ae579475cd95e
-F src/sqliteInt.h 72c7bfc4831f171d25b7b03c9fb152f1a3bbf55f51d5bbcdef313ebfa5873bea
+F src/sqliteInt.h aff99a1938b53530971fcfa7a6a5e6b47f986ddd673660bafb41f1c559747b72
 F src/sqliteLimit.h d7323ffea5208c6af2734574bae933ca8ed2ab728083caa117c9738581a31657
 F src/status.c 4b8bc2a6905163a38b739854a35b826c737333fab5b1f8e03fa7eb9a4799c4c1
 F src/table.c 0f141b58a16de7e2fbe81c308379e7279f4c6b50eb08efeec5892794a0ba30d1
@@ -608,7 +608,7 @@ F src/tokenize.c 01dba3023659dc6f6b1e054c14b35a0074bd35de10466b99454d33278191d97
 F src/treeview.c 4b92992176fb2caefbe06ba5bd06e0e0ebcde3d5564758da672631f17aa51cda
 F src/trigger.c 515e79206d40d1d4149129318582e79a6e9db590a7b74e226fdb5b2a6c7e1b10
 F src/update.c 9f126204a6acb96bbe47391ae48e0fc579105d8e76a6d9c4fab3271367476580
-F src/upsert.c 4b960968084a1d2a301ddbc782f625dbb1c02e0e8d0b0d6ad4a1986ecf305729
+F src/upsert.c 6471d9e0e5e05547f8a00d7d686714ac08fe89526de133abcf8e1c7a35f2cb82
 F src/utf.c ee39565f0843775cc2c81135751ddd93eceb91a673ea2c57f61c76f288b041a0
 F src/util.c c0c7977de7ef9b8cb10f6c85f2d0557889a658f817b0455909a49179ba4c8002
 F src/vacuum.c 492422c1463c076473bae1858799c7a0a5fe87a133d1223239447c422cd26286
@@ -1888,7 +1888,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 a47e35ee2d901baaa37e7229d190f934e1b0bd3510147cd4a2a49c4a1411416a
+P 64a4a91ecc5dcde3fa07d3cf038c74b9ede63d36628ecfb35203a9dfbbfe113c
-R 57e2c4174298e4f52a8ae7fd60045bd5
+R 750cd21a590114aba81487b95892f11b
 U drh
-Z 27b1defd588019eb0041967767579913
+Z dd643da8c0e77547922105a9f9bf1ab2

manifest.uuid

@@ -1 +1 @@
-64a4a91ecc5dcde3fa07d3cf038c74b9ede63d36628ecfb35203a9dfbbfe113c
+155142314feb007d526f8f67723636fd50dc52d1cd4d3a67dd93b105c9d5c2be

src/insert.c

@@ -1450,7 +1450,10 @@ static Index *indexIteratorFirst(IndexIterator *pIter, int *pIx){
 static Index *indexIteratorNext(IndexIterator *pIter, int *pIx){
   int i = ++pIter->i;
   if( pIter->eType ){
-    if( i>=pIter->u.ax.nIdx ) return 0;
+    if( i>=pIter->u.ax.nIdx ){
+      *pIx = i;
+      return 0;
+    }
     *pIx = pIter->u.ax.aIdx[i].ix;
     return pIter->u.ax.aIdx[i].p;
   }else{
@@ -1576,11 +1579,11 @@ void sqlite3GenerateConstraintChecks(
   int onError;         /* Conflict resolution strategy */
   int seenReplace = 0; /* True if REPLACE is used to resolve INT PK conflict */
   int nPkField;        /* Number of fields in PRIMARY KEY. 1 for ROWID tables */
-  Index *pUpIdx = 0;   /* Index to which to apply the upsert */
+  Upsert *pUpsertClause = 0;  /* The specific ON CONFLICT clause for pIdx */
   u8 isUpdate;           /* True if this is an UPDATE operation */
   u8 bAffinityDone = 0;  /* True if the OP_Affinity operation has been run */
-  int upsertBypass = 0;  /* Address of Goto to bypass upsert subroutine */
+  int upsertIpkReturn = 0; /* Address of Goto at end of IPK uniqueness check */
-  int upsertJump = 0;    /* Address of Goto that jumps into upsert subroutine */
+  int upsertIpkDelay = 0;  /* Address of Goto to bypass initial IPK check */
   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
@@ -1590,7 +1593,7 @@ void sqlite3GenerateConstraintChecks(
   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 */
-  IndexIterator ixi;    /* Index iterator */
+  IndexIterator sIdxIter;  /* Index iterator */
 
   isUpdate = regOldData!=0;
   db = pParse->db;
@@ -1788,24 +1791,64 @@ void sqlite3GenerateConstraintChecks(
   ** list of indexes attached to a table puts all OE_Replace indexes last
   ** in the list.  See sqlite3CreateIndex() for where that happens.
   */
-
+  sIdxIter.eType = 0;
+  sIdxIter.i = 0;
+  sIdxIter.u.lx.pIdx = pTab->pIndex;
   if( pUpsert ){
     if( pUpsert->pUpsertTarget==0 ){
-      /* An ON CONFLICT DO NOTHING clause, without a constraint-target.
+      /* There is just on ON CONFLICT clause and it has no constraint-target */
+      assert( pUpsert->pNextUpsert==0 );
+      if( pUpsert->pUpsertSet==0 ){
+        /* A single ON CONFLICT DO NOTHING clause, without a constraint-target.
         ** Make all unique constraint resolution be OE_Ignore */
-      assert( pUpsert->pUpsertSet==0 );
         overrideError = OE_Ignore;
         pUpsert = 0;
-    }else if( (pUpIdx = pUpsert->pUpsertIdx)!=0 ){
+      }else{
-      /* If the constraint-target uniqueness check must be run first.
+        /* A single ON CONFLICT DO UPDATE.  Make all resolutions OE_Update */
-      ** Jump to that uniqueness check now */
+        overrideError = OE_Update;
-      upsertJump = sqlite3VdbeAddOp0(v, OP_Goto);
+      }
-      VdbeComment((v, "UPSERT constraint goes first"));
+    }else if( pTab->pIndex!=0 ){
+      /* Otherwise, we'll need to run the IndexListTerm array version of the
+      ** iterator to ensure that all of the ON CONFLICT conditions are
+      ** checked first and in order. */
+      int nIdx, jj;
+      u64 nByte;
+      Upsert *pTerm;
+      u8 *bUsed;
+      for(nIdx=0, pIdx=pTab->pIndex; pIdx; pIdx=pIdx->pNext, nIdx++){
+         assert( aRegIdx[nIdx]>0 );
+      }
+      sIdxIter.eType = 1;
+      sIdxIter.u.ax.nIdx = nIdx;
+      nByte = (sizeof(IndexListTerm)+1)*nIdx + nIdx;
+      sIdxIter.u.ax.aIdx = sqlite3DbMallocZero(db, nByte);
+      if( sIdxIter.u.ax.aIdx==0 ) return; /* OOM */
+      bUsed = (u8*)&sIdxIter.u.ax.aIdx[nIdx];
+      pUpsert->pToFree = sIdxIter.u.ax.aIdx;
+      for(i=0, pTerm=pUpsert; pTerm; pTerm=pTerm->pNextUpsert){
+        if( pTerm->pUpsertTarget==0 ) break;
+        if( pTerm->pUpsertIdx==0 ) continue;  /* Skip ON CONFLICT for the IPK */
+        jj = 0;
+        pIdx = pTab->pIndex;
+        while( ALWAYS(pIdx!=0) && pIdx!=pTerm->pUpsertIdx ){
+           pIdx = pIdx->pNext;
+           jj++;
+        }
+        if( bUsed[jj] ) continue; /* Duplicate ON CONFLICT clause ignored */
+        bUsed[jj] = 1;
+        sIdxIter.u.ax.aIdx[i].p = pIdx;
+        sIdxIter.u.ax.aIdx[i].ix = jj;
+        i++;
+      }
+      for(jj=0, pIdx=pTab->pIndex; pIdx; pIdx=pIdx->pNext, jj++){
+        if( bUsed[jj] ) continue;
+        sIdxIter.u.ax.aIdx[i].p = pIdx;
+        sIdxIter.u.ax.aIdx[i].ix = jj;
+        i++;
+      }
+      assert( i==nIdx );
     }
   }
-  ixi.eType = 0;
-  ixi.i = 0;
-  ixi.u.lx.pIdx = pTab->pIndex;
 
   /* Determine if it is possible that triggers (either explicitly coded
   ** triggers or FK resolution actions) might run as a result of deletes
@@ -1866,13 +1909,22 @@ void sqlite3GenerateConstraintChecks(
     }
 
     /* figure out whether or not upsert applies in this case */
-    if( pUpsert && pUpsert->pUpsertIdx==0 ){
+    if( pUpsert ){
-      if( pUpsert->pUpsertSet==0 ){
+      pUpsertClause = sqlite3UpsertOfIndex(pUpsert,0);
+      if( pUpsertClause!=0 ){
+        if( pUpsertClause->pUpsertSet==0 ){
           onError = OE_Ignore;  /* DO NOTHING is the same as INSERT OR IGNORE */
         }else{
           onError = OE_Update;  /* DO UPDATE */
         }
       }
+      if( pUpsertClause!=pUpsert ){
+        /* The first ON CONFLICT clause has a conflict target other than
+        ** the IPK.  We have to jump ahead to that first ON CONFLICT clause
+        ** and then come back here and deal with the IPK afterwards */
+        upsertIpkDelay = sqlite3VdbeAddOp0(v, OP_Goto);
+      }
+    }
 
     /* If the response to a rowid conflict is REPLACE but the response
     ** to some other UNIQUE constraint is FAIL or IGNORE, then we need
@@ -1977,7 +2029,9 @@ void sqlite3GenerateConstraintChecks(
       }
     }
     sqlite3VdbeResolveLabel(v, addrRowidOk);
-    if( ipkTop ){
+    if( pUpsert && pUpsertClause!=pUpsert ){
+      upsertIpkReturn = sqlite3VdbeAddOp0(v, OP_Goto);
+    }else if( ipkTop ){
       ipkBottom = sqlite3VdbeAddOp0(v, OP_Goto);
       sqlite3VdbeJumpHere(v, ipkTop-1);
     }
@@ -1990,9 +2044,9 @@ void sqlite3GenerateConstraintChecks(
   ** This loop also handles the case of the PRIMARY KEY index for a
   ** WITHOUT ROWID table.
   */
-  for(pIdx = indexIteratorFirst(&ixi, &ix);
+  for(pIdx = indexIteratorFirst(&sIdxIter, &ix);
       pIdx;
-      pIdx = indexIteratorNext(&ixi, &ix)
+      pIdx = indexIteratorNext(&sIdxIter, &ix)
   ){
     int regIdx;          /* Range of registers hold conent for pIdx */
     int regR;            /* Range of registers holding conflicting PK */
@@ -2001,15 +2055,14 @@ void sqlite3GenerateConstraintChecks(
     int addrConflictCk;  /* First opcode in the conflict check logic */
 
     if( aRegIdx[ix]==0 ) continue;  /* Skip indices that do not change */
-    if( pUpIdx && pUpIdx->zName==pIdx->zName ){
+    if( pUpsert ){
-      addrUniqueOk = upsertJump+1;
+      pUpsertClause = sqlite3UpsertOfIndex(pUpsert, pIdx);
-      upsertBypass = sqlite3VdbeGoto(v, 0);
+      if( upsertIpkDelay && pUpsertClause==pUpsert ){
-      VdbeComment((v, "Skip upsert subroutine"));
+        sqlite3VdbeJumpHere(v, upsertIpkDelay);
-      sqlite3VdbeJumpHere(v, upsertJump);
-    }else{
-      addrUniqueOk = sqlite3VdbeMakeLabel(pParse);
       }
-    if( bAffinityDone==0 && (pUpIdx==0 || pUpIdx->zName==pIdx->zName) ){
+    }
+    addrUniqueOk = sqlite3VdbeMakeLabel(pParse);
+    if( bAffinityDone==0 ){
       sqlite3TableAffinity(v, pTab, regNewData+1);
       bAffinityDone = 1;
     }
@@ -2080,8 +2133,8 @@ void sqlite3GenerateConstraintChecks(
     }
 
     /* Figure out if the upsert clause applies to this index */
-    if( pUpIdx && pUpIdx->zName==pIdx->zName ){
+    if( pUpsertClause ){
-      if( pUpsert->pUpsertSet==0 ){
+      if( pUpsertClause->pUpsertSet==0 ){
         onError = OE_Ignore;  /* DO NOTHING is the same as INSERT OR IGNORE */
       }else{
         onError = OE_Update;  /* DO UPDATE */
@@ -2273,13 +2326,12 @@ void sqlite3GenerateConstraintChecks(
         break;
       }
     }
-    if( pUpIdx && pUpIdx->zName==pIdx->zName ){
-      sqlite3VdbeGoto(v, upsertJump+1);
-      sqlite3VdbeJumpHere(v, upsertBypass);
-    }else{
     sqlite3VdbeResolveLabel(v, addrUniqueOk);
-    }
     if( regR!=regIdx ) sqlite3ReleaseTempRange(pParse, regR, nPkField);
+    if( pUpsertClause && sqlite3UpsertNextIsIPK(pUpsertClause) ){
+      sqlite3VdbeGoto(v, upsertIpkDelay+1);
+      sqlite3VdbeJumpHere(v, upsertIpkReturn);
+    }
   }
 
   /* If the IPK constraint is a REPLACE, run it last */

src/sqliteInt.h

@@ -4841,10 +4841,14 @@ const char *sqlite3JournalModename(int);
   Upsert *sqlite3UpsertDup(sqlite3*,Upsert*);
   int sqlite3UpsertAnalyzeTarget(Parse*,SrcList*,Upsert*);
   void sqlite3UpsertDoUpdate(Parse*,Upsert*,Table*,Index*,int);
+  Upsert *sqlite3UpsertOfIndex(Upsert*,Index*);
+  int sqlite3UpsertNextIsIPK(Upsert*);
 #else
 #define sqlite3UpsertNew(u,v,w,x,y,z) ((Upsert*)0)
 #define sqlite3UpsertDelete(x,y)
 #define sqlite3UpsertDup(x,y)         ((Upsert*)0)
+#define sqlite3UpsertOfIndex(x,y)     ((Upsert*)0)
+#define sqlite3UpsertNextIsIPK(x)     0
 #endif
 
 

src/upsert.c

@@ -208,6 +208,38 @@ int sqlite3UpsertAnalyzeTarget(
   return SQLITE_OK;
 }
 
+/*
+** Return true if pUpsert is the last ON CONFLICT clause with a
+** conflict target, or if pUpsert is followed by another ON CONFLICT
+** clause that targets the INTEGER PRIMARY KEY.
+*/
+int sqlite3UpsertNextIsIPK(Upsert *pUpsert){
+  Upsert *pNext;
+  if( pUpsert==0 ) return 0;
+  pNext = pUpsert->pNextUpsert;
+  if( pNext==0 ) return 1;
+  if( pNext->pUpsertTarget==0 ) return 1;
+  if( pNext->pUpsertIdx==0 ) return 1;
+  return 0;
+}
+
+/*
+** Given the list of ON CONFLICT clauses described by pUpsert, and
+** a particular index pIdx, return a pointer to the particular ON CONFLICT
+** clause that applies to the index.  Or, if the index is not subject to
+** any ON CONFLICT clause, return NULL.
+*/
+Upsert *sqlite3UpsertOfIndex(Upsert *pUpsert, Index *pIdx){
+  while(
+      pUpsert
+   && pUpsert->pUpsertTarget!=0
+   && pUpsert->pUpsertIdx!=pIdx
+  ){
+     pUpsert = pUpsert->pNextUpsert;
+  }
+  return pUpsert;
+}
+
 /*
 ** Generate bytecode that does an UPDATE as part of an upsert.
 **
@@ -234,14 +266,7 @@ void sqlite3UpsertDoUpdate(
   assert( v!=0 );
   assert( pUpsert!=0 );
   iDataCur = pUpsert->iDataCur;
-  while(
+  pUpsert = sqlite3UpsertOfIndex(pTop, pIdx);
-      pUpsert->pUpsertTarget!=0
-   && (pUpsert->pUpsertIdx==0 ? pIdx!=0 :
-          pUpsert->pUpsertIdx->zName!=pIdx->zName)
-  ){
-    assert( pUpsert->pNextUpsert!=0 );
-    pUpsert = pUpsert->pNextUpsert;
-  }
   if( pUpsert->addrGenericUpdate>0 ){
     sqlite3VdbeAddOp2(v, OP_Goto, 0, pUpsert->addrGenericUpdate);
     return;