Silently ignore redundant ON CONFLICT clauses in an UPSERT. Only the first

ON CONFLICT for each index is active.  Do not issue an error, since that might
break legacy queries.  But ignore the redundant ON CONFLICT clauses to prevent
problems such as described in [forum:/forumpost/919c6579c8|forum post 919c6579c8].

FossilOrigin-Name: d0ea6b6ba64dba9d68c2b391ccf1171ea96fcdd7409dafdb2b697accb00246b8

3 files changed, 20 insertions, 6 deletions

diff --git a/src/insert.c b/src/insert.c
index 1c31ca233..095298b90 100644
--- a/src/insert.c
+++ b/src/insert.c
@@ -1086,7 +1086,7 @@ void sqlite3Insert(
       pNx->iDataCur = iDataCur;
       pNx->iIdxCur = iIdxCur;
       if( pNx->pUpsertTarget ){
-        if( sqlite3UpsertAnalyzeTarget(pParse, pTabList, pNx) ){
+        if( sqlite3UpsertAnalyzeTarget(pParse, pTabList, pNx, pUpsert) ){
           goto insert_cleanup;
         }
       }
diff --git a/src/sqliteInt.h b/src/sqliteInt.h
index f5920748b..91cf17304 100644
--- a/src/sqliteInt.h
+++ b/src/sqliteInt.h
@@ -3476,6 +3476,7 @@ struct Upsert {
   Expr *pUpsertWhere;       /* WHERE clause for the ON CONFLICT UPDATE */
   Upsert *pNextUpsert;      /* Next ON CONFLICT clause in the list */
   u8 isDoUpdate;            /* True for DO UPDATE.  False for DO NOTHING */
+  u8 isDup;                 /* True if 2nd or later with same pUpsertIdx */
   /* Above this point is the parse tree for the ON CONFLICT clauses.
   ** The next group of fields stores intermediate data. */
   void *pToFree;            /* Free memory when deleting the Upsert object */
@@ -5552,7 +5553,7 @@ const char *sqlite3JournalModename(int);
   Upsert *sqlite3UpsertNew(sqlite3*,ExprList*,Expr*,ExprList*,Expr*,Upsert*);
   void sqlite3UpsertDelete(sqlite3*,Upsert*);
   Upsert *sqlite3UpsertDup(sqlite3*,Upsert*);
-  int sqlite3UpsertAnalyzeTarget(Parse*,SrcList*,Upsert*);
+  int sqlite3UpsertAnalyzeTarget(Parse*,SrcList*,Upsert*,Upsert*);
   void sqlite3UpsertDoUpdate(Parse*,Upsert*,Table*,Index*,int);
   Upsert *sqlite3UpsertOfIndex(Upsert*,Index*);
   int sqlite3UpsertNextIsIPK(Upsert*);
diff --git a/src/upsert.c b/src/upsert.c
index be0d0550d..f74d4fabf 100644
--- a/src/upsert.c
+++ b/src/upsert.c
@@ -90,7 +90,8 @@ Upsert *sqlite3UpsertNew(
 int sqlite3UpsertAnalyzeTarget(
   Parse *pParse,     /* The parsing context */
   SrcList *pTabList, /* Table into which we are inserting */
-  Upsert *pUpsert    /* The ON CONFLICT clauses */
+  Upsert *pUpsert,   /* The ON CONFLICT clauses */
+  Upsert *pAll       /* Complete list of all ON CONFLICT clauses */
 ){
   Table *pTab;            /* That table into which we are inserting */
   int rc;                 /* Result code */
@@ -193,6 +194,14 @@ int sqlite3UpsertAnalyzeTarget(
         continue;
       }
       pUpsert->pUpsertIdx = pIdx;
+      if( sqlite3UpsertOfIndex(pAll,pIdx)!=pUpsert ){
+        /* Really this should be an error.  The isDup ON CONFLICT clause will
+        ** never fire.  But this problem was not discovered until three years
+        ** after multi-CONFLICT upsert was added, and so we silently ignore
+        ** the problem to prevent breaking applications that might actually
+        ** have redundant ON CONFLICT clauses. */
+        pUpsert->isDup = 1;
+      }
       break;
     }
     if( pUpsert->pUpsertIdx==0 ){
@@ -219,9 +228,13 @@ int sqlite3UpsertNextIsIPK(Upsert *pUpsert){
   Upsert *pNext;
   if( NEVER(pUpsert==0) ) return 0;
   pNext = pUpsert->pNextUpsert;
-  if( pNext==0 ) return 1;
-  if( pNext->pUpsertTarget==0 ) return 1;
-  if( pNext->pUpsertIdx==0 ) return 1;
+  while( 1 /*exit-by-return*/ ){
+    if( pNext==0 ) return 1;
+    if( pNext->pUpsertTarget==0 ) return 1;
+    if( pNext->pUpsertIdx==0 ) return 1;
+    if( !pNext->isDup ) return 0;
+    pNext = pNext->pNextUpsert;
+  }
   return 0;
 }