From 6f9c5669e2d54fb4e480b2f1f39fba6907560cc1 Mon Sep 17 00:00:00 2001 From: drh Date: Thu, 13 Nov 2014 13:42:39 +0000 Subject: [PATCH] When a transaction or savepoint rollback occurs, save the positions of all open read-cursors so that they can be restored following the rollback operation. Cherry-pick of check-in [dd03a2802f3f27] FossilOrigin-Name: 402780a9c8df9e7ea898bdca49c1191042fe387a --- manifest | 19 ++++++------ manifest.uuid | 2 +- src/btree.c | 65 +++++++++++++++++++++++++++------------ src/btree.h | 2 +- src/vdbe.c | 3 +- test/rollback2.test | 74 +++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 133 insertions(+), 32 deletions(-) create mode 100644 test/rollback2.test diff --git a/manifest b/manifest index a2c5fb1bb8..1d9fe4d78f 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Fix\sthe\s%c\sformat\scharacter\sin\ssqlite3VXPrintf()\sso\sthat\sit\scorrectly\shandles\sprecisions\slarger\sthan\s70. -D 2014-11-12T14:12:28.051 +C When\sa\stransaction\sor\ssavepoint\srollback\soccurs,\ssave\sthe\spositions\sof\sall\sopen\sread-cursors\sso\sthat\sthey\scan\sbe\srestored\sfollowing\sthe\srollback\soperation.\s\sCherry-pick\sof\scheck-in\s[dd03a2802f3f27] +D 2014-11-13T13:42:39.738 F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f F Makefile.in cf57f673d77606ab0f2d9627ca52a9ba1464146a F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23 @@ -172,8 +172,8 @@ F src/auth.c d8abcde53426275dab6243b441256fcd8ccbebb2 F src/backup.c 8cdfeb0c8a6d8bdad3faefae418eb3dc767051b6 F src/bitvec.c 19a4ba637bd85f8f63fc8c9bae5ade9fb05ec1cb F src/btmutex.c 49ca66250c7dfa844a4d4cb8272b87420d27d3a5 -F src/btree.c 944b84f72fd9048acde31400d89439976bf766b0 -F src/btree.h 97c4d0ef860f151e096e97eef78b87f7b4a8d899 +F src/btree.c 2c15850c5c9a26b10cdf92f9a29c74e299dc3674 +F src/btree.h a4afc6b06f5a1dd2076d15aa168baec44fc0121b F src/btreeInt.h 026d0129724e8f265fdc60d44ec240cf5a4e6179 F src/build.c 9dc2bd94347b878c89627000c92b0c8d97ec2919 F src/callback.c 7b44ce59674338ad48b0e84e7b72f935ea4f68b0 @@ -289,7 +289,7 @@ F src/update.c 3c4ecc282accf12d39edb8d524cf089645e55a13 F src/utf.c fc6b889ba0779b7722634cdeaa25f1930d93820c F src/util.c 4006c01772bd8d8ac4306d523bbcee41d3e392d8 F src/vacuum.c 59f03f92bcff57faa6a8ca256eb29ccddfb0614a -F src/vdbe.c 0b4ffa1aeeb7a10fefc6497dd0159bf5d9828464 +F src/vdbe.c a6b604364c7cbb079c083418e7359d1d665f2ef0 F src/vdbe.h 09f5b4e3719fa454f252322b1cdab5cf1f361327 F src/vdbeInt.h e2a060a55ee18a6ab973353a5e2ec7ee569bf787 F src/vdbeapi.c 37a6c6ae284a97bcace365f2f0a225680c0499d9 @@ -786,6 +786,7 @@ F test/releasetest.mk 2eced2f9ae701fd0a29e714a241760503ccba25a F test/releasetest.tcl a4279c890698584feb2ffc86735857a4e4474180 F test/resolver01.test 33abf37ff8335e6bf98f2b45a0af3e06996ccd9a F test/rollback.test 458fe73eb3ffdfdf9f6ba3e9b7350a6220414dea +F test/rollback2.test 552abaab8e721b6060a727d639896427059e51ec F test/rowhash.test 0bc1d31415e4575d10cacf31e1a66b5cc0f8be81 F test/rowid.test 742b5741584a8a44fd83e856cc2896688401d645 F test/rtree.test 0c8d9dd458d6824e59683c19ab2ffa9ef946f798 @@ -1204,8 +1205,8 @@ F tool/vdbe_profile.tcl 67746953071a9f8f2f668b73fe899074e2c6d8c1 F tool/warnings-clang.sh f6aa929dc20ef1f856af04a730772f59283631d4 F tool/warnings.sh 0abfd78ceb09b7f7c27c688c8e3fe93268a13b32 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f -P e1017745e183f5d7429ce787ec2feef946a24f0f -Q +08a27440f19b7fc884464832e6105af1bf008172 -R 3a2fc5ef3a825f22ce8bbd1b4d69c828 +P 839a6df9f98b90fb593534a62145d9c913540bae +Q +dd03a2802f3f276525f3cef9a93f825dd8606626 +R 0801523eb21d9763755a3afd3e84ae8b U drh -Z 3e707181121665e3dbca925048596208 +Z 995ce8d43edc83cec464fe7495c36085 diff --git a/manifest.uuid b/manifest.uuid index 55a1630790..38b0ca2922 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -839a6df9f98b90fb593534a62145d9c913540bae \ No newline at end of file +402780a9c8df9e7ea898bdca49c1191042fe387a \ No newline at end of file diff --git a/src/btree.c b/src/btree.c index 9d7a05157d..41e097af53 100644 --- a/src/btree.c +++ b/src/btree.c @@ -3472,29 +3472,52 @@ int sqlite3BtreeCommit(Btree *p){ ** that belong to other database connections that happen to be ** sharing the cache with pBtree. ** -** This routine gets called when a rollback occurs. The writeOnly -** flag is set to 1 if the transaction did not make any schema -** changes, in which case the read cursors can continue operating. -** If schema changes did occur in the transaction, then both read -** and write cursors must both be tripped. -*/ -void sqlite3BtreeTripAllCursors(Btree *pBtree, int errCode, int writeOnly){ +** This routine gets called when a rollback occurs. If the writeOnly +** flag is true, then only write-cursors need be tripped - read-only +** cursors save their current positions so that they may continue +** following the rollback. Or, if writeOnly is false, all cursors are +** tripped. In general, writeOnly is false if the transaction being +** rolled back modified the database schema. In this case b-tree root +** pages may be moved or deleted from the database altogether, making +** it unsafe for read cursors to continue. +** +** If the writeOnly flag is true and an error is encountered while +** saving the current position of a read-only cursor, all cursors, +** including all read-cursors are tripped. +** +** SQLITE_OK is returned if successful, or if an error occurs while +** saving a cursor position, an SQLite error code. +*/ +int sqlite3BtreeTripAllCursors(Btree *pBtree, int errCode, int writeOnly){ BtCursor *p; + int rc = SQLITE_OK; + assert( (writeOnly==0 || writeOnly==1) && BTCF_WriteFlag==1 ); - if( pBtree==0 ) return; - sqlite3BtreeEnter(pBtree); - for(p=pBtree->pBt->pCursor; p; p=p->pNext){ - int i; - if( writeOnly && (p->curFlags & BTCF_WriteFlag)==0 ) continue; - sqlite3BtreeClearCursor(p); - p->eState = CURSOR_FAULT; - p->skipNext = errCode; - for(i=0; i<=p->iPage; i++){ - releasePage(p->apPage[i]); - p->apPage[i] = 0; + if( pBtree ){ + sqlite3BtreeEnter(pBtree); + for(p=pBtree->pBt->pCursor; p; p=p->pNext){ + int i; + if( writeOnly && (p->curFlags & BTCF_WriteFlag)==0 ){ + if( p->eState==CURSOR_VALID ){ + int rc = saveCursorPosition(p); + if( rc!=SQLITE_OK ){ + (void)sqlite3BtreeTripAllCursors(pBtree, rc, 0); + break; + } + } + }else{ + sqlite3BtreeClearCursor(p); + p->eState = CURSOR_FAULT; + p->skipNext = errCode; + } + for(i=0; i<=p->iPage; i++){ + releasePage(p->apPage[i]); + p->apPage[i] = 0; + } } + sqlite3BtreeLeave(pBtree); } - sqlite3BtreeLeave(pBtree); + return rc; } /* @@ -3523,7 +3546,9 @@ int sqlite3BtreeRollback(Btree *p, int tripCode, int writeOnly){ rc = SQLITE_OK; } if( tripCode ){ - sqlite3BtreeTripAllCursors(p, tripCode, writeOnly); + int rc2 = sqlite3BtreeTripAllCursors(p, tripCode, writeOnly); + assert( rc==SQLITE_OK || (writeOnly==0 && rc2==SQLITE_OK) ); + if( rc2!=SQLITE_OK ) rc = rc2; } btreeIntegrity(p); diff --git a/src/btree.h b/src/btree.h index f724269a3e..fabedd9a53 100644 --- a/src/btree.h +++ b/src/btree.h @@ -116,7 +116,7 @@ int sqlite3BtreeIncrVacuum(Btree *); int sqlite3BtreeDropTable(Btree*, int, int*); int sqlite3BtreeClearTable(Btree*, int, int*); int sqlite3BtreeClearTableOfCursor(BtCursor*); -void sqlite3BtreeTripAllCursors(Btree*, int, int); +int sqlite3BtreeTripAllCursors(Btree*, int, int); void sqlite3BtreeGetMeta(Btree *pBtree, int idx, u32 *pValue); int sqlite3BtreeUpdateMeta(Btree*, int idx, u32 value); diff --git a/src/vdbe.c b/src/vdbe.c index 9462bd71f8..1ad8aab753 100644 --- a/src/vdbe.c +++ b/src/vdbe.c @@ -2827,8 +2827,9 @@ case OP_Savepoint: { if( p1==SAVEPOINT_ROLLBACK ){ isSchemaChange = (db->flags & SQLITE_InternChanges)!=0; for(ii=0; iinDb; ii++){ - sqlite3BtreeTripAllCursors(db->aDb[ii].pBt, SQLITE_ABORT, + rc = sqlite3BtreeTripAllCursors(db->aDb[ii].pBt, SQLITE_ABORT, isSchemaChange==0); + if( rc!=SQLITE_OK ) goto abort_due_to_error; } }else{ isSchemaChange = 0; diff --git a/test/rollback2.test b/test/rollback2.test new file mode 100644 index 0000000000..9637f0c0e5 --- /dev/null +++ b/test/rollback2.test @@ -0,0 +1,74 @@ +# 2014 November 12 +# +# The author disclaims copyright to this source code. In place of +# a legal notice, here is a blessing: +# +# May you do good and not evil. +# May you find forgiveness for yourself and forgive others. +# May you share freely, never taking more than you give. +# +#*********************************************************************** +# + +set testdir [file dirname $argv0] +source $testdir/tester.tcl +set ::testprefix rollback2 + +proc int2hex {i} { format %.2X $i } +db func int2hex int2hex + +do_execsql_test 1.0 { + SELECT int2hex(0), int2hex(100), int2hex(255) +} {00 64 FF} +do_execsql_test 1.1 { + CREATE TABLE t1(i, h); + CREATE INDEX i1 ON t1(h); + WITH data(a, b) AS ( + SELECT 1, int2hex(1) + UNION ALL + SELECT a+1, int2hex(a+1) FROM data WHERE a<40 + ) + INSERT INTO t1 SELECT * FROM data; +} {} + + +proc do_rollback_test {tn args} { + set A(-setup) "" + set A(-select) "" + set A(-result) "" + set A(-rollback) ROLLBACK + + array set O $args + foreach k [array names O] { + if {[info exists A($k)]==0} { error "unknown option: $k" } + set A($k) $O($k) + } + + for {set iRollback 0} 1 {incr iRollback} { + catch { db eval ROLLBACK } + set res [list] + db eval $A(-setup) + + set i 0 + db eval $A(-select) x { + if {$i==$iRollback} { db eval $A(-rollback) } + foreach k $x(*) { lappend res $x($k) } + incr i + } + + do_test $tn.$iRollback [list set {} $res] [list {*}$A(-result)] + if {$i < $iRollback} break + } +} + +do_rollback_test 2 -setup { + BEGIN; + DELETE FROM t1 WHERE (i%2)==1; +} -select { + SELECT i FROM t1 WHERE (i%2)==0 +} -result { + 2 4 6 8 10 12 14 16 18 20 22 24 26 28 30 32 34 36 38 40 +} + +finish_test + -- 2.47.2