From 24a0c4534aa0f8be54ecd47050fb0bd71e9cb7bf Mon Sep 17 00:00:00 2001 From: dan Date: Wed, 21 Mar 2018 17:29:53 +0000 Subject: [PATCH] Fix rebasing of UPDATE changes against a set of remote changesets that feature both OMIT and REPLACE conflict resolution on different fields of the same row. FossilOrigin-Name: d8bc3fdb6ba165ca8d7cab857ede8e7e6e2fac24ad59580c5e1db1a4942d295c --- ext/session/sessionrebase.test | 2 - ext/session/sqlite3session.c | 109 +++++++++++++++++++++++---------- ext/session/sqlite3session.h | 27 ++++++-- manifest | 16 ++--- manifest.uuid | 2 +- 5 files changed, 107 insertions(+), 49 deletions(-) diff --git a/ext/session/sessionrebase.test b/ext/session/sessionrebase.test index 8f5314c5e6..e2e613910f 100644 --- a/ext/session/sessionrebase.test +++ b/ext/session/sessionrebase.test @@ -120,10 +120,8 @@ proc do_rebase_test {tn sql1 sql2 conflict_handler {testsql ""} {testres ""}} { #puts [changeset_to_list [lindex $rebase 0]] ; breakpoint #puts [llength $rebase] -if {$i==2 && $tn=="3.3.1"} breakpoint sqlite3rebaser_create R foreach r $rebase { -puts [changeset_to_list $r] R configure $r } set c1r [R rebase $c1] diff --git a/ext/session/sqlite3session.c b/ext/session/sqlite3session.c index 5922bd4e20..1e3820a8f6 100644 --- a/ext/session/sqlite3session.c +++ b/ext/session/sqlite3session.c @@ -516,7 +516,7 @@ static int sessionPreupdateHash( static int sessionSerialLen(u8 *a){ int e = *a; int n; - if( e==0 ) return 1; + if( e==0 || e==0xFF ) return 1; if( e==SQLITE_NULL ) return 1; if( e==SQLITE_INTEGER || e==SQLITE_FLOAT ) return 9; return sessionVarintGet(&a[1], &n) + 1 + n; @@ -4527,6 +4527,7 @@ static int sessionChangeMerge( SessionChange **ppNew /* OUT: Merged change */ ){ SessionChange *pNew = 0; + int rc = SQLITE_OK; if( !pExist ){ pNew = (SessionChange *)sqlite3_malloc(sizeof(SessionChange) + nRec); @@ -4536,16 +4537,66 @@ static int sessionChangeMerge( memset(pNew, 0, sizeof(SessionChange)); pNew->op = op2; pNew->bIndirect = bIndirect; - pNew->nRecord = nRec; pNew->aRecord = (u8*)&pNew[1]; - memcpy(pNew->aRecord, aRec, nRec); - }else if( bRebase){ - /* - ** op1=INSERT/R, op2=INSERT/R -> - ** op1=INSERT/R, op2=INSERT/O -> - ** op1=INSERT/O, op2=INSERT/R -> - ** op1=INSERT/O, op2=INSERT/O -> - */ + if( bIndirect==0 || bRebase==0 ){ + pNew->nRecord = nRec; + memcpy(pNew->aRecord, aRec, nRec); + }else{ + int i; + u8 *pIn = aRec; + u8 *pOut = pNew->aRecord; + for(i=0; inCol; i++){ + int nIn = sessionSerialLen(pIn); + if( *pIn==0 ){ + *pOut++ = 0; + }else if( pTab->abPK[i]==0 ){ + *pOut++ = 0xFF; + }else{ + memcpy(pOut, pIn, nIn); + pOut += nIn; + } + pIn += nIn; + } + pNew->nRecord = pOut - pNew->aRecord; + } + }else if( bRebase ){ + if( pExist->op==SQLITE_DELETE && pExist->bIndirect ){ + *ppNew = pExist; + }else{ + int nByte = nRec + pExist->nRecord + sizeof(SessionChange); + pNew = (SessionChange*)sqlite3_malloc(nByte); + if( pNew==0 ){ + rc = SQLITE_NOMEM; + }else{ + int i; + u8 *a1 = pExist->aRecord; + u8 *a2 = aRec; + u8 *pOut; + + memset(pNew, 0, nByte); + pNew->bIndirect = bIndirect || pExist->bIndirect; + pNew->op = op2; + pOut = pNew->aRecord = (u8*)&pNew[1]; + + for(i=0; inCol; i++){ + int n1 = sessionSerialLen(a1); + int n2 = sessionSerialLen(a2); + if( *a1==0xFF || *a2==0xFF ){ + *pOut++ = 0xFF; + }else if( *a2==0 ){ + memcpy(pOut, a1, n1); + pOut += n1; + }else{ + memcpy(pOut, a2, n2); + pOut += n2; + } + a1 += n1; + a2 += n2; + } + pNew->nRecord = pOut - pNew->aRecord; + } + sqlite3_free(pExist); + } }else{ int op1 = pExist->op; @@ -4639,7 +4690,7 @@ static int sessionChangeMerge( } *ppNew = pNew; - return SQLITE_OK; + return rc; } /* @@ -4999,7 +5050,7 @@ static void sessionAppendRecordMerge( pOut += nn1; } }else{ - if( *a1==0 ){ + if( *a1==0 || *a1==0xFF ){ memcpy(pOut, a2, nn2); pOut += nn2; }else{ @@ -5017,11 +5068,11 @@ static void sessionAppendRecordMerge( } static void sessionAppendPartialUpdate( - SessionBuffer *pBuf, - sqlite3_changeset_iter *pIter, - u8 *aRec, int nRec, - u8 *aChange, int nChange, - int *pRc + SessionBuffer *pBuf, /* Append record here */ + sqlite3_changeset_iter *pIter, /* Iterator pointed at local change */ + u8 *aRec, int nRec, /* Local change */ + u8 *aChange, int nChange, /* Record to rebase against */ + int *pRc /* IN/OUT: Return Code */ ){ sessionBufferGrow(pBuf, 2+nRec+nChange, pRc); if( *pRc==SQLITE_OK ){ @@ -5040,6 +5091,10 @@ static void sessionAppendPartialUpdate( if( !pIter->abPK[i] ) bData = 1; memcpy(pOut, a1, n1); pOut += n1; + }else if( a2[0]!=0xFF ){ + bData = 1; + memcpy(pOut, a2, n2); + pOut += n2; }else{ *pOut++ = '\0'; } @@ -5051,7 +5106,7 @@ static void sessionAppendPartialUpdate( for(i=0; inCol; i++){ int n1 = sessionSerialLen(a1); int n2 = sessionSerialLen(a2); - if( pIter->abPK[i] || a2[0]==0 ){ + if( pIter->abPK[i] || a2[0]!=0xFF ){ memcpy(pOut, a1, n1); pOut += n1; }else{ @@ -5065,7 +5120,6 @@ static void sessionAppendPartialUpdate( } } - static int sessionRebase( sqlite3_rebaser *p, /* Rebaser hash table */ sqlite3_changeset_iter *pIter, /* Input data */ @@ -5139,20 +5193,9 @@ static int sessionRebase( ); } }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 - ); - } + sessionAppendPartialUpdate(&sOut, pIter, + aRec, nRec, pChange->aRecord, pChange->nRecord, &rc + ); } break; diff --git a/ext/session/sqlite3session.h b/ext/session/sqlite3session.h index 33a1c8cff8..038061e0f0 100644 --- a/ext/session/sqlite3session.h +++ b/ext/session/sqlite3session.h @@ -1256,14 +1256,14 @@ int sqlite3changeset_apply_v2( ** 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 change are updated +** operation was an UPDATE, then the old.* fields of change are updated ** to reflect the new.* values in the UPDATE. ** **
Local UPDATE
** 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 the old.* values from +** from the update change are filled in using the old.* values from ** the conflicting DELETE. Or, if the conflict resolution was REPLACE, ** the UPDATE change is simply omitted from the rebased changeset. ** @@ -1271,12 +1271,29 @@ int sqlite3changeset_apply_v2( ** 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. +** the conflicting remote UPDATE removed. If this means no columns would +** be updated, the change is omitted. ** ** ** A local change may be rebased against multiple remote changes -** simultaneously. +** simultaneously. If a single key is modified by multiple remote +** changesets, they are combined as follows before the local changeset +** is rebased: +** +**
    +**
  • If there has been one or more REPLACE resolutions on a +** key, it is rebased according to a REPLACE. +** +**
  • If there have been no REPLACE resolutions on a key, then +** the local changeset is rebased according to the most recent +** of the OMIT resolutions. +**
+** +** Note that conflict resolutions from multiple remote changesets are +** combined on a per-field basis, not per-row. This means that in the +** case of multiple remote UPDATE operations, some fields of a single +** local change may be rebased for REPLACE while others are rebased for +** OMIT. */ typedef struct sqlite3_rebaser sqlite3_rebaser; diff --git a/manifest b/manifest index 7041f65588..558be02e5b 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Add\sfurther\stests\sand\sdocumentation\sfor\sthe\ssessions\srebase\sfeature. -D 2018-03-20T20:27:03.438 +C Fix\srebasing\sof\sUPDATE\schanges\sagainst\sa\sset\sof\sremote\schangesets\sthat\sfeature\nboth\sOMIT\sand\sREPLACE\sconflict\sresolution\son\sdifferent\sfields\sof\sthe\ssame\srow. +D 2018-03-21T17:29:53.542 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 883c8919ebcf6c140ba86b480bc14ae642ee9969c009e0b355c8981a5266f9ed -F ext/session/sessionrebase.test a150289bf25176f14983fbd519cdd97921fd52de682d0c75849f44daf51d37e4 +F ext/session/sessionrebase.test 5dd3d468710631fb389262d46b4768684bf3e5c43a8671992be8ce78e2e14cba F ext/session/sessionstat1.test 41cd97c2e48619a41cdf8ae749e1b25f34719de638689221aa43971be693bf4e F ext/session/sessionwor.test 2f3744236dc8b170a695b7d8ddc8c743c7e79fdc -F ext/session/sqlite3session.c b51365d4fe085409bab2e19d7c5f796a3ac6c5e205b0ac3e409dad4e8b9df1b8 -F ext/session/sqlite3session.h cc09a873386bdb95079746f17e2c8d7261a11fab6a01e52fc1c8237adfa5a145 +F ext/session/sqlite3session.c c2256416b2fa5ffdcf18175ca92506e6776f831ee4f59633b5195845c1ca3dfe +F ext/session/sqlite3session.h be3ec8ab03e1b6251d45d38349e0c57b1a745e291c6f76c2ffd33dadd44d5db3 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 f7bf71f1d47044e3cbc74018294b8af5ad52c2bb84954e99bbd4e9b8c36fc077 -R 09a1cb643bb8175d8449dc1dfe2a3ce4 +P 7475a363ebb272ae23c0796fe7587714a156dc6a3a4a57ed948ed6f69d3c1218 +R 3c5c6f568034d51df9227a4faa7f0dcd U dan -Z deee606c0fac26e4bea6293ad6f81334 +Z cbf4222d0200eaa670f79abf0e02a69a diff --git a/manifest.uuid b/manifest.uuid index 3f0f5f6084..68e0370bbc 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -7475a363ebb272ae23c0796fe7587714a156dc6a3a4a57ed948ed6f69d3c1218 \ No newline at end of file +d8bc3fdb6ba165ca8d7cab857ede8e7e6e2fac24ad59580c5e1db1a4942d295c \ No newline at end of file