Prevent the sessions rebaser from generating changesets containing UPDATE records...

FossilOrigin-Name: f9cd23dffba06b1982c0a5e5362dba53eba768120a2daa985b4f649d3fea1427

diff --git a/ext/session/sqlite3session.c b/ext/session/sqlite3session.c
index a892804b49bcac6737830b24cce44dce9b378790..fd06f3b4d612966313f7dda8baea5ef7deb04ca2 100644 (file)
--- a/ext/session/sqlite3session.c
+++ b/ext/session/sqlite3session.c
@@ -3327,6 +3327,22 @@ static int sessionChangesetNextOne(
       if( p->op==SQLITE_INSERT ) p->op = SQLITE_DELETE;
       else if( p->op==SQLITE_DELETE ) p->op = SQLITE_INSERT;
     }
+
+    /* If this is an UPDATE that is part of a changeset, then check that
+    ** there are no fields in the old.* record that are not (a) PK fields,
+    ** or (b) also present in the new.* record. 
+    **
+    ** Such records are technically corrupt, but the rebaser was at one
+    ** point generating them. Under most circumstances this is benign, but
+    ** can cause spurious SQLITE_RANGE errors when applying the changeset. */
+    if( p->bPatchset==0 && p->op==SQLITE_UPDATE){
+      for(i=0; i<p->nCol; i++){
+        if( p->abPK[i]==0 && p->apValue[i+p->nCol]==0 ){
+          sqlite3ValueFree(p->apValue[i]);
+          p->apValue[i] = 0;
+        }
+      }
+    }
   }
 
   return SQLITE_ROW;
@@ -5523,7 +5539,7 @@ static void sessionAppendPartialUpdate(
         if( !pIter->abPK[i] && a1[0] ) bData = 1;
         memcpy(pOut, a1, n1);
         pOut += n1;
-      }else if( a2[0]!=0xFF ){
+      }else if( a2[0]!=0xFF && a1[0] ){
         bData = 1;
         memcpy(pOut, a2, n2);
         pOut += n2;

diff --git a/ext/session/test_session.c b/ext/session/test_session.c
index f2fc77eee3f596fe0b6df982af03f081b78ea051..c1feb7802c7d25f35b2d46041cd538460404d3ed 100644 (file)
--- a/ext/session/test_session.c
+++ b/ext/session/test_session.c
@@ -98,6 +98,19 @@ int sql_exec_changeset(
 }
 /************************************************************************/
 
+
+#ifdef SQLITE_DEBUG
+static int sqlite3_test_changeset(int, void *, char **);
+static void assert_changeset_is_ok(int n, void *p){
+  int rc = 0;
+  char *z = 0;
+  rc = sqlite3_test_changeset(n, p, &z);
+  assert( z==0 );
+}
+#else
+# define assert_changeset_is_ok(n,p)
+#endif
+
 /*
 ** Tclcmd: sql_exec_changeset DB SQL
 */
@@ -127,6 +140,7 @@ static int SQLITE_TCLAPI test_sql_exec_changeset(
     return TCL_ERROR;
   }
 
+  assert_changeset_is_ok(nChangeset, pChangeset);
   Tcl_SetObjResult(interp, Tcl_NewByteArrayObj(pChangeset, nChangeset));
   sqlite3_free(pChangeset);
   return TCL_OK;
@@ -295,6 +309,7 @@ static int SQLITE_TCLAPI test_session_cmd(
         }
       }
       if( rc==SQLITE_OK ){
+        assert_changeset_is_ok(o.n, o.p);
         Tcl_SetObjResult(interp, Tcl_NewByteArrayObj(o.p, o.n)); 
       }
       sqlite3_free(o.p);
@@ -953,6 +968,7 @@ static int SQLITE_TCLAPI test_sqlite3changeset_invert(
   if( rc!=SQLITE_OK ){
     rc = test_session_error(interp, rc, 0);
   }else{
+    assert_changeset_is_ok(sOut.n, sOut.p);
     Tcl_SetObjResult(interp,Tcl_NewByteArrayObj((unsigned char*)sOut.p,sOut.n));
   }
   sqlite3_free(sOut.p);
@@ -1001,6 +1017,7 @@ static int SQLITE_TCLAPI test_sqlite3changeset_concat(
   if( rc!=SQLITE_OK ){
     rc = test_session_error(interp, rc, 0);
   }else{
+    assert_changeset_is_ok(sOut.n, sOut.p);
     Tcl_SetObjResult(interp,Tcl_NewByteArrayObj((unsigned char*)sOut.p,sOut.n));
   }
   sqlite3_free(sOut.p);
@@ -1236,6 +1253,7 @@ static int SQLITE_TCLAPI test_rebaser_cmd(
       }
 
       if( rc==SQLITE_OK ){
+        assert_changeset_is_ok(sOut.n, sOut.p);
         Tcl_SetObjResult(interp, Tcl_NewByteArrayObj(sOut.p, sOut.n));
       }
       sqlite3_free(sOut.p);
@@ -1282,6 +1300,99 @@ static int SQLITE_TCLAPI test_sqlite3rebaser_create(
   return TCL_OK;
 }
 
+/*
+**
+*/
+static int sqlite3_test_changeset(
+  int nChangeset,
+  void *pChangeset,
+  char **pzErr
+){
+  sqlite3_changeset_iter *pIter = 0;
+  char *zErr = 0;
+  int rc = SQLITE_OK;
+  int bPatch = (nChangeset>0 && ((char*)pChangeset)[0]=='P');
+
+  rc = sqlite3changeset_start(&pIter, nChangeset, pChangeset);
+  if( rc==SQLITE_OK ){
+    int rc2;
+    while( rc==SQLITE_OK && SQLITE_ROW==sqlite3changeset_next(pIter) ){
+      unsigned char *aPk = 0;
+      int nCol = 0;
+      int op = 0;
+      const char *zTab = 0;
+
+      sqlite3changeset_pk(pIter, &aPk, &nCol);
+      sqlite3changeset_op(pIter, &zTab, &nCol, &op, 0);
+
+      if( op==SQLITE_UPDATE ){
+        int iCol;
+        for(iCol=0; iCol<nCol; iCol++){
+          sqlite3_value *pNew = 0;
+          sqlite3_value *pOld = 0;
+          sqlite3changeset_new(pIter, iCol, &pNew);
+          sqlite3changeset_old(pIter, iCol, &pOld);
+
+          if( aPk[iCol] ){
+            if( pOld==0 ) rc = SQLITE_ERROR;
+          }else if( bPatch ){
+            if( pOld ) rc = SQLITE_ERROR;
+          }else{
+            if( (pOld==0)!=(pNew==0) ) rc = SQLITE_ERROR;
+          }
+
+          if( rc!=SQLITE_OK ){
+            zErr = sqlite3_mprintf(
+                "unexpected SQLITE_UPDATE (bPatch=%d pk=%d pOld=%d pNew=%d)",
+                bPatch, (int)aPk[iCol], pOld!=0, pNew!=0
+            );
+            break;
+          }
+        }
+      }
+    }
+    rc2 = sqlite3changeset_finalize(pIter);
+    if( rc==SQLITE_OK ){
+      rc = rc2;
+    }
+  }
+
+  *pzErr = zErr;
+  return rc;
+}
+
+/*
+** test_changeset CHANGESET
+*/
+static int SQLITE_TCLAPI test_changeset(
+  void * clientData,
+  Tcl_Interp *interp,
+  int objc,
+  Tcl_Obj *CONST objv[]
+){
+  void *pChangeset = 0;           /* Buffer containing changeset */
+  int nChangeset = 0;             /* Size of buffer aChangeset in bytes */
+  int rc = SQLITE_OK;
+  char *z = 0;
+
+  if( objc!=2 ){
+    Tcl_WrongNumArgs(interp, 1, objv, "CHANGESET");
+    return TCL_ERROR;
+  }
+  pChangeset = (void *)Tcl_GetByteArrayFromObj(objv[1], &nChangeset);
+
+  Tcl_ResetResult(interp);
+  rc = sqlite3_test_changeset(nChangeset, pChangeset, &z);
+  if( rc!=SQLITE_OK ){
+    char *zErr = sqlite3_mprintf("(%d) - \"%s\"", rc, z);
+    Tcl_SetObjResult(interp, Tcl_NewStringObj(zErr, -1));
+    sqlite3_free(zErr);
+  }
+  sqlite3_free(z);
+
+  return rc ? TCL_ERROR : TCL_OK;
+}
+
 /*
 ** tclcmd: sqlite3rebaser_configure OP VALUE
 */
@@ -1337,6 +1448,7 @@ int TestSession_Init(Tcl_Interp *interp){
     { "sql_exec_changeset", test_sql_exec_changeset },
     { "sqlite3rebaser_create", test_sqlite3rebaser_create },
     { "sqlite3session_config", test_sqlite3session_config },
+    { "test_changeset", test_changeset },
   };
   int i;
 

diff --git a/manifest b/manifest
index e7f602c2248dac4b0d13329b2cdf615454f00d52..a8379478a920a1f53df673a3486f61ecdb470fd2 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Add\sextra\stest\scases\sfor\sapplying\ssessions\smodule\schangesets\sto\sdatabases\sthat\shave\sbeen\smodified\susing\s"ALTER\sTABLE\s..\sADD\sCOLUMN".
-D 2022-12-13T14:59:28.942
+C Prevent\sthe\ssessions\srebaser\sfrom\sgenerating\schangesets\scontaining\sUPDATE\srecords\sfor\swhich\snon-PK\sfields\sare\spresent\sin\sthe\sold.*\sbut\snot\sthe\snew.*\srecord.\sAlso\supdate\sthe\schangeset\siterator\sto\swork\saround\ssuch\schangesets.
+D 2022-12-13T19:40:56.072
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -484,9 +484,9 @@ F ext/session/sessionrebase.test ccfa716b23bd1d3b03217ee58cfd90c78d4b99f53e6a9a2
 F ext/session/sessionsize.test 6f644aff31c7f1e4871e9ff3542766e18da68fc7e587b83a347ea9820a002dd8
 F ext/session/sessionstat1.test 218d351cf9fcd6648f125a26b607b140310160184723c2666091b54450a68fb5
 F ext/session/sessionwor.test 6fd9a2256442cebde5b2284936ae9e0d54bde692d0f5fd009ecef8511f4cf3fc
-F ext/session/sqlite3session.c 1d019c5caf51936ef24c761db63552b06e0e0d951c8740bba9639b17fa0cb107
+F ext/session/sqlite3session.c b4254dd6e785cdd206c9ca7118796cf82273627fe2d4fd647597f08c2f821f96
 F ext/session/sqlite3session.h 0907de79bc13a2e3af30a6dc29acc60792a3eaf7d33d44cf52500d0f3c2b2171
-F ext/session/test_session.c f433f68a8a8c64b0f5bc74dc725078f12483301ad4ae8375205eef790274a787
+F ext/session/test_session.c 6aa7e97407987e81431b26730889db4719efebebbb6fe7502802d19d58b35798
 F ext/userauth/sqlite3userauth.h 7f3ea8c4686db8e40b0a0e7a8e0b00fac13aa7a3
 F ext/userauth/user-auth.txt e6641021a9210364665fe625d067617d03f27b04
 F ext/userauth/userauth.c 7f00cded7dcaa5d47f54539b290a43d2e59f4b1eb5f447545fa865f002fc80cb
@@ -2067,8 +2067,8 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P 01cf3278c9c00dddcac67aa4b22ca26a52c31932cba35daa634a56b4c264bdeb
-R 2168ace2713fa491abfdf32af2ae3084
+P a7b404f21f657f395eddb32e218eae14e09df08fa03f68ca8bba79ea322ce8ba
+R f531fac65fd257fc9f795cdff2ea1ee6
 U dan
-Z f44dc60adae3e752289931c06520949b
+Z 6ffdc33d3ed4ae836f64773585b07c14
 # Remove this line to create a well-formed Fossil manifest.

diff --git a/manifest.uuid b/manifest.uuid
index 5209b155bf8e16e24e4fae0123d19ef243006506..62e36cd33a3294d4848e774c4155d2cb59d5ad45 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-a7b404f21f657f395eddb32e218eae14e09df08fa03f68ca8bba79ea322ce8ba
\ No newline at end of file
+f9cd23dffba06b1982c0a5e5362dba53eba768120a2daa985b4f649d3fea1427
\ No newline at end of file