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

3 files changed, 135 insertions, 54 deletions

diff --git a/src/insert.c b/src/insert.c
index 44cd8a339..5e5922127 100644
--- a/src/insert.c
+++ b/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 */
-  u8 isUpdate;         /* True if this is an UPDATE operation */
+  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 upsertJump = 0;    /* Address of Goto that jumps into upsert subroutine */
+  int upsertIpkReturn = 0; /* Address of Goto at end of IPK uniqueness check */
+  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.
-      ** Make all unique constraint resolution be OE_Ignore */
-      assert( pUpsert->pUpsertSet==0 );
-      overrideError = OE_Ignore;
-      pUpsert = 0;
-    }else if( (pUpIdx = pUpsert->pUpsertIdx)!=0 ){
-      /* If the constraint-target uniqueness check must be run first.
-      ** Jump to that uniqueness check now */
-      upsertJump = sqlite3VdbeAddOp0(v, OP_Goto);
-      VdbeComment((v, "UPSERT constraint goes first"));
-    }
-  }
-  ixi.eType = 0;
-  ixi.i = 0;
-  ixi.u.lx.pIdx = pTab->pIndex;
+      /* 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 */
+        overrideError = OE_Ignore;
+        pUpsert = 0;
+      }else{
+        /* A single ON CONFLICT DO UPDATE.  Make all resolutions OE_Update */
+        overrideError = OE_Update;
+      }
+    }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 );
+    }
+  }
 
   /* Determine if it is possible that triggers (either explicitly coded
   ** triggers or FK resolution actions) might run as a result of deletes
@@ -1866,11 +1909,20 @@ void sqlite3GenerateConstraintChecks(
     }
 
     /* figure out whether or not upsert applies in this case */
-    if( pUpsert && pUpsert->pUpsertIdx==0 ){
-      if( pUpsert->pUpsertSet==0 ){
-        onError = OE_Ignore;  /* DO NOTHING is the same as INSERT OR IGNORE */
-      }else{
-        onError = OE_Update;  /* DO UPDATE */
+    if( pUpsert ){
+      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);
       }
     }
 
@@ -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 ){
-      addrUniqueOk = upsertJump+1;
-      upsertBypass = sqlite3VdbeGoto(v, 0);
-      VdbeComment((v, "Skip upsert subroutine"));
-      sqlite3VdbeJumpHere(v, upsertJump);
-    }else{
-      addrUniqueOk = sqlite3VdbeMakeLabel(pParse);
+    if( pUpsert ){
+      pUpsertClause = sqlite3UpsertOfIndex(pUpsert, pIdx);
+      if( upsertIpkDelay && pUpsertClause==pUpsert ){
+        sqlite3VdbeJumpHere(v, upsertIpkDelay);
+      }
     }
-    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( pUpsert->pUpsertSet==0 ){
+    if( pUpsertClause ){
+      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);
-    }
+    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 */
diff --git a/src/sqliteInt.h b/src/sqliteInt.h
index 83993f08c..403f31326 100644
--- a/src/sqliteInt.h
+++ b/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
 
 
diff --git a/src/upsert.c b/src/upsert.c
index 6b9bff685..0a9b67dbd 100644
--- a/src/upsert.c
+++ b/src/upsert.c
@@ -209,6 +209,38 @@ int sqlite3UpsertAnalyzeTarget(
 }
 
 /*
+** 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.
 **
 ** If pIdx is NULL, then the UNIQUE constraint that failed was the IPK.
@@ -234,14 +266,7 @@ void sqlite3UpsertDoUpdate(
   assert( v!=0 );
   assert( pUpsert!=0 );
   iDataCur = pUpsert->iDataCur;
-  while(
-      pUpsert->pUpsertTarget!=0
-   && (pUpsert->pUpsertIdx==0 ? pIdx!=0 :
-          pUpsert->pUpsertIdx->zName!=pIdx->zName)
-  ){
-    assert( pUpsert->pNextUpsert!=0 );
-    pUpsert = pUpsert->pNextUpsert;
-  }
+  pUpsert = sqlite3UpsertOfIndex(pTop, pIdx);
   if( pUpsert->addrGenericUpdate>0 ){
     sqlite3VdbeAddOp2(v, OP_Goto, 0, pUpsert->addrGenericUpdate);
     return;