From f01d3a7ef77ea7fbddc95befa13c78062168fcf7 Mon Sep 17 00:00:00 2001
From: dan <dan@noemail.net>
Date: Fri, 16 Mar 2018 18:02:47 +0000
Subject: [PATCH] Fix a problem with handling rebasing UPDATE changes for
 REPLACE conflict resolution.

FossilOrigin-Name: f7bf71f1d47044e3cbc74018294b8af5ad52c2bb84954e99bbd4e9b8c36fc077
---
 ext/session/sessionrebase.test |  74 +++++++++++++--
 ext/session/sqlite3session.c   | 165 ++++++++++++++++++++++++++-------
 ext/session/sqlite3session.h   |  35 +++++++
 manifest                       |  16 ++--
 manifest.uuid                  |   2 +-
 5 files changed, 243 insertions(+), 49 deletions(-)

diff --git a/ext/session/sessionrebase.test b/ext/session/sessionrebase.test
index 9085068922..815c9356bd 100644
--- a/ext/session/sessionrebase.test
+++ b/ext/session/sessionrebase.test
@@ -73,6 +73,7 @@ do_apply_v2_test 1.1.1 {
 } {
   {INSERT t1 0 X. {} {i 1 t {value B}}}
 }
+
 do_apply_v2_test 1.1.2 {
   UPDATE t1 SET b = 'value B' WHERE a=1;
 } {
@@ -80,7 +81,7 @@ do_apply_v2_test 1.1.2 {
 } {
   REPLACE
 } {
-  {DELETE t1 0 X. {i 1 {} {}} {}}
+  {INSERT t1 1 X. {} {i 1 t {value B}}}
 }
 
 do_apply_v2_test 1.2.1 {
@@ -99,7 +100,7 @@ do_apply_v2_test 1.2.2 {
 } {
   REPLACE
 } {
-  {DELETE t1 0 X. {i 2 {} {}} {}}
+  {INSERT t1 1 X. {} {i 2 t first}}
 }
 
 do_apply_v2_test 1.3.1 {
@@ -109,7 +110,7 @@ do_apply_v2_test 1.3.1 {
 } {
   OMIT
 } {
-  {INSERT t1 1 X. {} {i 1 {} {}}}
+  {DELETE t1 0 X. {i 1 t {value A}} {}}
 }
 do_apply_v2_test 1.3.2 {
   DELETE FROM t1 WHERE a=1;
@@ -118,7 +119,7 @@ do_apply_v2_test 1.3.2 {
 } {
   REPLACE
 } {
-  {DELETE t1 0 X. {i 1 {} {}} {}}
+  {DELETE t1 1 X. {i 1 t {value A}} {}}
 }
 
 #-------------------------------------------------------------------------
@@ -130,6 +131,11 @@ do_apply_v2_test 1.3.2 {
 #    2.1.4 - 1d2u1r
 #    2.1.5 - 1d2u2r !!
 #    2.1.6 - 1u2d1r
+#    2.1.7 - 1u2d2r
+#
+#    2.1.8 - 1i2i2r
+#    2.1.9 - 1i2i1r
+#
 
 proc xConflictAbort {args} {
   return "ABORT"
@@ -164,13 +170,13 @@ proc do_rebase_test {tn sql1 sql2 conflict_handler {testsql ""} {testres ""}} {
 
   set ::lConflict $conflict_handler
   set rebase [sqlite3changeset_apply_v2 db $c2 xConflict]
-  #puts [changeset_to_list $rebase]
+  #if {$tn=="2.1.4"} { puts [changeset_to_list $rebase] ; breakpoint }
 
   sqlite3rebaser_create R
   R configure $rebase
   set c1r [R rebase $c1]
   R delete
-  #puts [changeset_to_list $c1r]
+  #if {$tn=="2.1.4"} { puts [changeset_to_list $c1r] }
 
   sqlite3changeset_apply_v2 db2 $c1r xConflictAbort
 
@@ -224,11 +230,11 @@ do_rebase_test 2.1.4 {
 } { SELECT * FROM t1 } {2 two 3 three}
 
 #do_rebase_test 2.1.5 {
-  #DELETE FROM t1 WHERE a=1;
+#  DELETE FROM t1 WHERE a=1;
 #} {
-  #UPDATE t1 SET b='one.2' WHERE a=1
+#  UPDATE t1 SET b='one.2' WHERE a=1
 #} {
-  #REPLACE
+#  REPLACE
 #} { SELECT * FROM t1 } {2 two 3 three}
 
 do_rebase_test 2.1.6 {
@@ -247,5 +253,55 @@ do_rebase_test 2.1.7 {
   REPLACE
 } { SELECT * FROM t1 } {1 one 2 two}
 
+do_rebase_test 2.1.8 {
+  INSERT INTO t1 VALUES(4, 'four.1');
+} {
+  INSERT INTO t1 VALUES(4, 'four.2');
+} {
+  REPLACE
+} { SELECT * FROM t1 } {1 one 2 two 3 three 4 four.2}
+
+do_rebase_test 2.1.9 {
+  INSERT INTO t1 VALUES(4, 'four.1');
+} {
+  INSERT INTO t1 VALUES(4, 'four.2');
+} {
+  OMIT
+} { SELECT * FROM t1 } {1 one 2 two 3 three 4 four.1}
+
+do_execsql_test 2.2.0 {
+  CREATE TABLE t2(x, y, z PRIMARY KEY);
+  INSERT INTO t2 VALUES('i', 'a', 'A');
+  INSERT INTO t2 VALUES('ii', 'b', 'B');
+  INSERT INTO t2 VALUES('iii', 'c', 'C');
+
+  CREATE TABLE t3(a INTEGER PRIMARY KEY, b, c);
+  INSERT INTO t3 VALUES(-1, 'z', 'Z');
+  INSERT INTO t3 VALUES(-2, 'y', 'Y');
+}
+
+do_rebase_test 2.2.1 {
+  UPDATE t2 SET x=1 WHERE z='A';
+} {
+  UPDATE t2 SET y='one' WHERE z='A';
+} {
+} { SELECT * FROM t2 WHERE z='A' } { 1 one A }
+
+do_rebase_test 2.2.2 {
+  UPDATE t2 SET x=1, y='one' WHERE z='B';
+} {
+  UPDATE t2 SET y='two' WHERE z='B';
+} {
+  REPLACE
+} { SELECT * FROM t2 WHERE z='B' } { 1 two B }
+
+do_rebase_test 2.2.3 {
+  UPDATE t2 SET x=1, y='one' WHERE z='B';
+} {
+  UPDATE t2 SET y='two' WHERE z='B';
+} {
+  OMIT
+} { SELECT * FROM t2 WHERE z='B' } { 1 one B }
+
 finish_test
 
diff --git a/ext/session/sqlite3session.c b/ext/session/sqlite3session.c
index 45803c7b2c..005b2308d8 100644
--- a/ext/session/sqlite3session.c
+++ b/ext/session/sqlite3session.c
@@ -232,8 +232,8 @@ struct SessionTable {
 ** statement.
 **
 ** For a DELETE change, all fields within the record except those associated
-** with PRIMARY KEY columns are set to "undefined". The PRIMARY KEY fields
-** contain the values identifying the row to delete.
+** with PRIMARY KEY columns are omitted. The PRIMARY KEY fields contain the
+** values identifying the row to delete.
 **
 ** For an UPDATE change, all fields except those associated with PRIMARY KEY
 ** columns and columns that are modified by the UPDATE are set to "undefined".
@@ -3816,34 +3816,18 @@ static int sessionRebaseAdd(
   assert( eType==SQLITE_CHANGESET_REPLACE||eType==SQLITE_CHANGESET_OMIT );
   assert( eOp==SQLITE_DELETE || eOp==SQLITE_INSERT || eOp==SQLITE_UPDATE );
 
-  if( eType==SQLITE_CHANGESET_REPLACE ){
-    sessionAppendByte(&p->rebase, SQLITE_DELETE, &rc);
-    sessionAppendByte(&p->rebase, 0, &rc);
-    for(i=0; i<p->nCol; i++){
-      if( p->abPK[i]==0 ){
-        sessionAppendByte(&p->rebase, 0, &rc);
-      }else{
-        sqlite3_value *pVal = 0;
-        if( eOp==SQLITE_INSERT ){
-          sqlite3changeset_new(pIter, i, &pVal);
-        }else{
-          sqlite3changeset_old(pIter, i, &pVal);
-        }
-        sessionAppendValue(&p->rebase, pVal, &rc);
-      }
-    }
-  }else{
-    sessionAppendByte(&p->rebase, SQLITE_INSERT, &rc);
-    sessionAppendByte(&p->rebase, eOp==SQLITE_DELETE, &rc);
-    for(i=0; i<p->nCol; i++){
-      sqlite3_value *pVal = 0;
-      if( eOp!=SQLITE_INSERT && p->abPK[i] ){
-        sqlite3changeset_old(pIter, i, &pVal);
-      }else{
-        sqlite3changeset_new(pIter, i, &pVal);
-      }
-      sessionAppendValue(&p->rebase, pVal, &rc);
+  sessionAppendByte(&p->rebase, 
+      (eOp==SQLITE_DELETE ? SQLITE_DELETE : SQLITE_INSERT), &rc
+  );
+  sessionAppendByte(&p->rebase, (eType==SQLITE_CHANGESET_REPLACE), &rc);
+  for(i=0; i<p->nCol; i++){
+    sqlite3_value *pVal = 0;
+    if( eOp==SQLITE_DELETE || (eOp==SQLITE_UPDATE && p->abPK[i]) ){
+      sqlite3changeset_old(pIter, i, &pVal);
+    }else{
+      sqlite3changeset_new(pIter, i, &pVal);
     }
+    sessionAppendValue(&p->rebase, pVal, &rc);
   }
 
   return rc;
@@ -5026,6 +5010,56 @@ static void sessionAppendRecordMerge(
   }
 }
 
+static void sessionAppendPartialUpdate(
+  SessionBuffer *pBuf,
+  sqlite3_changeset_iter *pIter,
+  u8 *aRec, int nRec,
+  u8 *aChange, int nChange,
+  int *pRc
+){
+  sessionBufferGrow(pBuf, 2+nRec+nChange, pRc);
+  if( *pRc==SQLITE_OK ){
+    int bData = 0;
+    u8 *pOut = &pBuf->aBuf[pBuf->nBuf];
+    int i;
+    u8 *a1 = aRec;
+    u8 *a2 = aChange;
+
+    *pOut++ = SQLITE_UPDATE;
+    *pOut++ = pIter->bIndirect;
+    for(i=0; i<pIter->nCol; i++){
+      int n1 = sessionSerialLen(a1);
+      int n2 = sessionSerialLen(a2);
+      if( pIter->abPK[i] || a2[0]==0 ){
+        if( !pIter->abPK[i] ) bData = 1;
+        memcpy(pOut, a1, n1);
+        pOut += n1;
+      }else{
+        *pOut++ = '\0';
+      }
+      a1 += n1;
+      a2 += n2;
+    }
+    if( bData ){
+      a2 = aChange;
+      for(i=0; i<pIter->nCol; i++){
+        int n1 = sessionSerialLen(a1);
+        int n2 = sessionSerialLen(a2);
+        if( pIter->abPK[i] || a2[0]==0 ){
+          memcpy(pOut, a1, n1);
+          pOut += n1;
+        }else{
+          *pOut++ = '\0';
+        }
+        a1 += n1;
+        a2 += n2;
+      }
+      pBuf->nBuf = (pOut - pBuf->aBuf);
+    }
+  }
+}
+
+
 static int sessionRebase(
   sqlite3_rebaser *p,             /* Rebaser hash table */
   sqlite3_changeset_iter *pIter,  /* Input data */
@@ -5043,6 +5077,7 @@ static int sessionRebase(
 
   while( SQLITE_ROW==sessionChangesetNext(pIter, &aRec, &nRec, &bNew) ){
     SessionChange *pChange = 0;
+    int bDone = 0;
 
     if( bNew ){
       const char *zTab = pIter->zTab;
@@ -5071,6 +5106,70 @@ static int sessionRebase(
 
     if( pChange ){
       assert( pChange->op==SQLITE_DELETE || pChange->op==SQLITE_INSERT );
+      switch( pIter->op ){
+        case SQLITE_INSERT:
+          if( pChange->op==SQLITE_INSERT ){
+            bDone = 1;
+            if( pChange->bIndirect==0 ){
+              sessionAppendByte(&sOut, SQLITE_UPDATE, &rc);
+              sessionAppendByte(&sOut, pIter->bIndirect, &rc);
+              sessionAppendBlob(&sOut, pChange->aRecord, pChange->nRecord, &rc);
+              sessionAppendBlob(&sOut, aRec, nRec, &rc);
+            }
+          }
+          break;
+
+        case SQLITE_UPDATE:
+          bDone = 1;
+          if( pChange->op==SQLITE_DELETE ){
+            if( pChange->bIndirect==0 ){
+              u8 *pCsr = aRec;
+              sessionSkipRecord(&pCsr, pIter->nCol);
+              sessionAppendByte(&sOut, SQLITE_INSERT, &rc);
+              sessionAppendByte(&sOut, pIter->bIndirect, &rc);
+              sessionAppendRecordMerge(&sOut, pIter->nCol, 1,
+                  pCsr, nRec-(pCsr-aRec), 
+                  pChange->aRecord, pChange->nRecord, &rc
+              );
+            }
+          }else{
+            if( pChange->bIndirect==0 ){
+              u8 *pCsr = aRec;
+              sessionAppendByte(&sOut, SQLITE_UPDATE, &rc);
+              sessionAppendByte(&sOut, pIter->bIndirect, &rc);
+              sessionAppendRecordMerge(&sOut, pIter->nCol, 0,
+                  aRec, nRec, pChange->aRecord, pChange->nRecord, &rc
+              );
+              sessionSkipRecord(&pCsr, pIter->nCol);
+              sessionAppendBlob(&sOut, pCsr, nRec - (pCsr-aRec), &rc);
+            }else{
+              sessionAppendPartialUpdate(&sOut, pIter,
+                 aRec, nRec, pChange->aRecord, pChange->nRecord, &rc
+              );
+            }
+          }
+          break;
+
+        default:
+          assert( pIter->op==SQLITE_DELETE );
+          bDone = 1;
+          if( pChange->op==SQLITE_INSERT ){
+            sessionAppendByte(&sOut, SQLITE_DELETE, &rc);
+            sessionAppendByte(&sOut, pIter->bIndirect, &rc);
+            sessionAppendRecordMerge(&sOut, pIter->nCol, 1,
+                pChange->aRecord, pChange->nRecord, aRec, nRec, &rc
+            );
+          }
+          break;
+      }
+    }
+
+    if( bDone==0 ){
+      sessionAppendByte(&sOut, pIter->op, &rc);
+      sessionAppendByte(&sOut, pIter->bIndirect, &rc);
+      sessionAppendBlob(&sOut, aRec, nRec, &rc);
+    }
+#if 0
       /* If pChange is an INSERT, then rebase the change. If it is a
       ** DELETE, omit the change from the output altogether.  */
       if( pChange->op==SQLITE_INSERT ){
@@ -5097,12 +5196,15 @@ static int sessionRebase(
             }
           }
         }else{
-          sessionAppendByte(&sOut, pIter->op, &rc);
-          sessionAppendByte(&sOut, pIter->bIndirect, &rc);
           if( pIter->op==SQLITE_INSERT ){
+            sessionAppendByte(&sOut, SQLITE_UPDATE, &rc);
+            sessionAppendByte(&sOut, pIter->bIndirect, &rc);
+            sessionAppendBlob(&sOut, pChange->aRecord, pChange->nRecord, &rc);
             sessionAppendBlob(&sOut, aRec, nRec, &rc);
           }else{
             u8 *pCsr = aRec;
+            sessionAppendByte(&sOut, pIter->op, &rc);
+            sessionAppendByte(&sOut, pIter->bIndirect, &rc);
             sessionAppendRecordMerge(&sOut, pIter->nCol, 0,
                 aRec, nRec, pChange->aRecord, pChange->nRecord, &rc
             );
@@ -5118,6 +5220,7 @@ static int sessionRebase(
       sessionAppendByte(&sOut, pIter->bIndirect, &rc);
       sessionAppendBlob(&sOut, aRec, nRec, &rc);
     }
+#endif
 
     if( rc==SQLITE_OK && xOutput && sOut.nBuf>SESSIONS_STRM_CHUNK_SIZE ){
       rc = xOutput(pOut, sOut.aBuf, sOut.nBuf);
diff --git a/ext/session/sqlite3session.h b/ext/session/sqlite3session.h
index ee002b008f..933b3b92f3 100644
--- a/ext/session/sqlite3session.h
+++ b/ext/session/sqlite3session.h
@@ -1216,6 +1216,41 @@ int sqlite3changeset_apply_v2(
 #define SQLITE_CHANGESET_REPLACE    1
 #define SQLITE_CHANGESET_ABORT      2
 
+/* 
+** CAPI3REF: Rebasing changesets
+**
+** Changes are rebased as follows:
+**
+** <dl>
+** <dt>INSERT<dd>
+**   This may only conflict with a remote INSERT. If the conflict 
+**   resolution was OMIT, then add an UPDATE change to the rebased
+**   changeset. Or, if the conflict resolution was REPLACE, add
+**   nothing to the rebased changeset.
+**
+** <dt>DELETE<dd>
+**   This may conflict with a remote UPDATE or DELETE. In both cases the
+**   only possible resolution is OMIT. If the remote operation was a
+**   DELETE, then add no change to the rebased changeset. If the remote
+**   operation was an UPDATE, then the old.* fields of the are updated to
+**   reflect the new.* values in the UPDATE.
+**
+** <dt>UPDATE<dd>
+**   This may conflict with a remote UPDATE or DELETE. If it conflicts
+**   with a DELETE, and the conflict resolution was OMIT, then the update
+**   is changed into an INSERT. Any undefined values in the new.* record
+**   from the update change are filled in using hte old.* values from 
+**   the conflicting DELETE. Or, if the conflict resolution was REPLACE,
+**   the UPDATE change is simply omitted from the rebased changeset.
+**
+**   If conflict is with a remote UPDATE and the resolution is OMIT, then
+**   the old.* values are rebased using the new.* values in the remote
+**   change. Or, if the resolution is REPLACE, then the change is copied
+**   into the rebased changeset with updates to columns also updated by
+**   the conflicting UPDATE removed. If this means no columns would be
+**   updated, the change is omitted.
+** </dl>
+*/
 typedef struct sqlite3_rebaser sqlite3_rebaser;
 
 /* Create a new rebaser object */
diff --git a/manifest b/manifest
index 4948fb7f53..35b68f97e6 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Add\ssimple\stests\sfor\sthe\ssessions\smodule\srebase\sAPI.
-D 2018-03-15T19:25:40.859
+C Fix\sa\sproblem\swith\shandling\srebasing\sUPDATE\schanges\sfor\sREPLACE\sconflict\nresolution.
+D 2018-03-16T18:02:47.093
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F Makefile.in 7016fc56c6b9bfe5daac4f34be8be38d8c0b5fab79ccbfb764d3b23bf1c6fff3
@@ -400,11 +400,11 @@ F ext/session/sessionat.test efe88965e74ff1bc2af9c310b28358c02d420c1fb2705cc7a28
 F ext/session/sessiondiff.test ad13dd65664bae26744e1f18eb3cbd5588349b7e9118851d8f9364248d67bcec
 F ext/session/sessionfault.test da273f2712b6411e85e71465a1733b8501dbf6f7
 F ext/session/sessionfault2.test 04aa0bc9aa70ea43d8de82c4f648db4de1e990b0
-F ext/session/sessionrebase.test d3a33c733e5564afe517252167d8f456a04601b047246d85f1e84bf319c2897f
+F ext/session/sessionrebase.test 35dede926077b0bbb5c323e278ed575a24fa333b3c2f45f3b6842b532054e463
 F ext/session/sessionstat1.test 41cd97c2e48619a41cdf8ae749e1b25f34719de638689221aa43971be693bf4e
 F ext/session/sessionwor.test 2f3744236dc8b170a695b7d8ddc8c743c7e79fdc
-F ext/session/sqlite3session.c 94b960a94d5e6b2117215a7b1057b3db74ca1222dded2a94588f8ccac5a7d929
-F ext/session/sqlite3session.h 8cb9992411344b9e906a394d2213f58da7b3942ae57e7936d1ec3fe26277dfc0
+F ext/session/sqlite3session.c 84f4786c93b12701cf72073064ea70740a7d43508f0bfb96563c49cfd7df644c
+F ext/session/sqlite3session.h a1c66a6497c36246d2242223663c2e7c1906bd28099556d7e5148214c2d9902a
 F ext/session/test_session.c f253742ea01b089326f189b5ae15a5b55c1c9e97452e4a195ee759ba51b404d5
 F ext/userauth/sqlite3userauth.h 7f3ea8c4686db8e40b0a0e7a8e0b00fac13aa7a3
 F ext/userauth/user-auth.txt e6641021a9210364665fe625d067617d03f27b04
@@ -1713,7 +1713,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 39915b683b3f8d3bf872af1dede96bf2818b488a8638a1d248395023fc4bd0ef
-R 749942968b245595723def17d4b1a08a
+P cf0d1abb44cf170d747e9c11f49ec03a29f00ab4821c613ca1e05b883a568211
+R 120c1710087879bab403c404357c60c1
 U dan
-Z 85fdd8cb2cf7c970dbbe7369d1e2a257
+Z 77da19287ec96a71252949a7a74af667
diff --git a/manifest.uuid b/manifest.uuid
index 02df08f087..2baa51d4e3 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-cf0d1abb44cf170d747e9c11f49ec03a29f00ab4821c613ca1e05b883a568211
\ No newline at end of file
+f7bf71f1d47044e3cbc74018294b8af5ad52c2bb84954e99bbd4e9b8c36fc077
\ No newline at end of file