Index: src/backup.c ================================================================== --- src/backup.c +++ src/backup.c @@ -566,11 +566,11 @@ } *pp = p->pNext; } /* If a transaction is still open on the Btree, roll it back. */ - sqlite3BtreeRollback(p->pDest); + sqlite3BtreeRollback(p->pDest, SQLITE_OK); /* Set the error code of the destination database handle. */ rc = (p->rc==SQLITE_DONE) ? SQLITE_OK : p->rc; sqlite3Error(p->pDestDb, rc, 0); Index: src/btree.c ================================================================== --- src/btree.c +++ src/btree.c @@ -2039,11 +2039,11 @@ /* Rollback any active transaction and free the handle structure. ** The call to sqlite3BtreeRollback() drops any table-locks held by ** this handle. */ - sqlite3BtreeRollback(p); + sqlite3BtreeRollback(p, SQLITE_OK); sqlite3BtreeLeave(p); /* If there are still other outstanding references to the shared-btree ** structure, return now. The remainder of this procedure cleans ** up the shared-btree. @@ -3277,10 +3277,11 @@ ** save the state of the cursor. The cursor must be ** invalidated. */ void sqlite3BtreeTripAllCursors(Btree *pBtree, int errCode){ BtCursor *p; + if( pBtree==0 ) return; sqlite3BtreeEnter(pBtree); for(p=pBtree->pBt->pCursor; p; p=p->pNext){ int i; sqlite3BtreeClearCursor(p); p->eState = CURSOR_FAULT; @@ -3300,29 +3301,24 @@ ** in an error. ** ** This will release the write lock on the database file. If there ** are no active cursors, it also releases the read lock. */ -int sqlite3BtreeRollback(Btree *p){ +int sqlite3BtreeRollback(Btree *p, int tripCode){ int rc; BtShared *pBt = p->pBt; MemPage *pPage1; sqlite3BtreeEnter(p); - rc = saveAllCursors(pBt, 0, 0); -#ifndef SQLITE_OMIT_SHARED_CACHE - if( rc!=SQLITE_OK ){ - /* This is a horrible situation. An IO or malloc() error occurred whilst - ** trying to save cursor positions. If this is an automatic rollback (as - ** the result of a constraint, malloc() failure or IO error) then - ** the cache may be internally inconsistent (not contain valid trees) so - ** we cannot simply return the error to the caller. Instead, abort - ** all queries that may be using any of the cursors that failed to save. - */ - sqlite3BtreeTripAllCursors(p, rc); - } -#endif + if( tripCode==SQLITE_OK ){ + rc = tripCode = saveAllCursors(pBt, 0, 0); + }else{ + rc = SQLITE_OK; + } + if( tripCode ){ + sqlite3BtreeTripAllCursors(p, tripCode); + } btreeIntegrity(p); if( p->inTrans==TRANS_WRITE ){ int rc2; Index: src/btree.h ================================================================== --- src/btree.h +++ src/btree.h @@ -75,11 +75,11 @@ int sqlite3BtreeGetAutoVacuum(Btree *); int sqlite3BtreeBeginTrans(Btree*,int); int sqlite3BtreeCommitPhaseOne(Btree*, const char *zMaster); int sqlite3BtreeCommitPhaseTwo(Btree*, int); int sqlite3BtreeCommit(Btree*); -int sqlite3BtreeRollback(Btree*); +int sqlite3BtreeRollback(Btree*,int); int sqlite3BtreeBeginStmt(Btree*,int); int sqlite3BtreeCreateTable(Btree*, int*, int flags); int sqlite3BtreeIsInTrans(Btree*); int sqlite3BtreeIsInReadTrans(Btree*); int sqlite3BtreeIsInBackup(Btree*); Index: src/main.c ================================================================== --- src/main.c +++ src/main.c @@ -847,23 +847,27 @@ sqlite3_free(db); return SQLITE_OK; } /* -** Rollback all database files. +** Rollback all database files. If tripCode is not SQLITE_OK, then +** any open cursors are invalidated ("tripped" - as in "tripping a circuit +** breaker") and made to return tripCode if there are any further +** attempts to use that cursor. */ -void sqlite3RollbackAll(sqlite3 *db){ +void sqlite3RollbackAll(sqlite3 *db, int tripCode){ int i; int inTrans = 0; assert( sqlite3_mutex_held(db->mutex) ); sqlite3BeginBenignMalloc(); for(i=0; inDb; i++){ - if( db->aDb[i].pBt ){ - if( sqlite3BtreeIsInTrans(db->aDb[i].pBt) ){ + Btree *p = db->aDb[i].pBt; + if( p ){ + if( sqlite3BtreeIsInTrans(p) ){ inTrans = 1; } - sqlite3BtreeRollback(db->aDb[i].pBt); + sqlite3BtreeRollback(p, tripCode); db->aDb[i].inTrans = 0; } } sqlite3VtabRollback(db); sqlite3EndBenignMalloc(); @@ -914,16 +918,25 @@ /* SQLITE_AUTH */ "authorization denied", /* SQLITE_FORMAT */ "auxiliary database format error", /* SQLITE_RANGE */ "bind or column index out of range", /* SQLITE_NOTADB */ "file is encrypted or is not a database", }; - rc &= 0xff; - if( ALWAYS(rc>=0) && rc<(int)(sizeof(aMsg)/sizeof(aMsg[0])) && aMsg[rc]!=0 ){ - return aMsg[rc]; - }else{ - return "unknown error"; + const char *zErr = "unknown error"; + switch( rc ){ + case SQLITE_ABORT_ROLLBACK: { + zErr = "abort due to ROLLBACK"; + break; + } + default: { + rc &= 0xff; + if( ALWAYS(rc>=0) && rczErrMsg, db, "no such savepoint: %s", zName); rc = SQLITE_ERROR; - }else if( - db->writeVdbeCnt>0 || (p1==SAVEPOINT_ROLLBACK && db->activeVdbeCnt>1) - ){ + }else if( db->writeVdbeCnt>0 && p1==SAVEPOINT_RELEASE ){ /* It is not possible to release (commit) a savepoint if there are - ** active write statements. It is not possible to rollback a savepoint - ** if there are any active statements at all. + ** active write statements. */ sqlite3SetString(&p->zErrMsg, db, - "cannot %s savepoint - SQL statements in progress", - (p1==SAVEPOINT_ROLLBACK ? "rollback": "release") + "cannot release savepoint - SQL statements in progress" ); rc = SQLITE_BUSY; }else{ /* Determine whether or not this is a transaction savepoint. If so, @@ -2716,10 +2712,13 @@ } db->isTransactionSavepoint = 0; rc = p->rc; }else{ iSavepoint = db->nSavepoint - iSavepoint - 1; + for(ii=0; iinDb; ii++){ + sqlite3BtreeTripAllCursors(db->aDb[ii].pBt, SQLITE_ABORT); + } for(ii=0; iinDb; ii++){ rc = sqlite3BtreeSavepoint(db->aDb[ii].pBt, p1, iSavepoint); if( rc!=SQLITE_OK ){ goto abort_due_to_error; } @@ -2784,29 +2783,32 @@ turnOnAC = desiredAutoCommit && !db->autoCommit; assert( desiredAutoCommit==1 || desiredAutoCommit==0 ); assert( desiredAutoCommit==1 || iRollback==0 ); assert( db->activeVdbeCnt>0 ); /* At least this one VM is active */ +#if 0 if( turnOnAC && iRollback && db->activeVdbeCnt>1 ){ /* If this instruction implements a ROLLBACK and other VMs are ** still running, and a transaction is active, return an error indicating ** that the other VMs must complete first. */ sqlite3SetString(&p->zErrMsg, db, "cannot rollback transaction - " "SQL statements in progress"); rc = SQLITE_BUSY; - }else if( turnOnAC && !iRollback && db->writeVdbeCnt>0 ){ + }else +#endif + if( turnOnAC && !iRollback && db->writeVdbeCnt>0 ){ /* If this instruction implements a COMMIT and other VMs are writing ** return an error indicating that the other VMs must complete first. */ sqlite3SetString(&p->zErrMsg, db, "cannot commit transaction - " "SQL statements in progress"); rc = SQLITE_BUSY; }else if( desiredAutoCommit!=db->autoCommit ){ if( iRollback ){ assert( desiredAutoCommit==1 ); - sqlite3RollbackAll(db); + sqlite3RollbackAll(db, SQLITE_ABORT_ROLLBACK); db->autoCommit = 1; }else if( (rc = sqlite3VdbeCheckFk(p, 1))!=SQLITE_OK ){ goto vdbe_return; }else{ db->autoCommit = (u8)desiredAutoCommit; Index: src/vdbeaux.c ================================================================== --- src/vdbeaux.c +++ src/vdbeaux.c @@ -2000,36 +2000,10 @@ } #else #define checkActiveVdbeCnt(x) #endif -/* -** For every Btree that in database connection db which -** has been modified, "trip" or invalidate each cursor in -** that Btree might have been modified so that the cursor -** can never be used again. This happens when a rollback -*** occurs. We have to trip all the other cursors, even -** cursor from other VMs in different database connections, -** so that none of them try to use the data at which they -** were pointing and which now may have been changed due -** to the rollback. -** -** Remember that a rollback can delete tables complete and -** reorder rootpages. So it is not sufficient just to save -** the state of the cursor. We have to invalidate the cursor -** so that it is never used again. -*/ -static void invalidateCursorsOnModifiedBtrees(sqlite3 *db){ - int i; - for(i=0; inDb; i++){ - Btree *p = db->aDb[i].pBt; - if( p && sqlite3BtreeIsInTrans(p) ){ - sqlite3BtreeTripAllCursors(p, SQLITE_ABORT); - } - } -} - /* ** If the Vdbe passed as the first argument opened a statement-transaction, ** close it now. Argument eOp must be either SAVEPOINT_ROLLBACK or ** SAVEPOINT_RELEASE. If it is SAVEPOINT_ROLLBACK, then the statement ** transaction is rolled back. If eOp is SAVEPOINT_RELEASE, then the @@ -2190,12 +2164,11 @@ eStatementOp = SAVEPOINT_ROLLBACK; }else{ /* We are forced to roll back the active transaction. Before doing ** so, abort any other statements this handle currently has active. */ - invalidateCursorsOnModifiedBtrees(db); - sqlite3RollbackAll(db); + sqlite3RollbackAll(db, SQLITE_ABORT_ROLLBACK); sqlite3CloseSavepoints(db); db->autoCommit = 1; } } } @@ -2233,27 +2206,26 @@ if( rc==SQLITE_BUSY && p->readOnly ){ sqlite3VdbeLeave(p); return SQLITE_BUSY; }else if( rc!=SQLITE_OK ){ p->rc = rc; - sqlite3RollbackAll(db); + sqlite3RollbackAll(db, SQLITE_OK); }else{ db->nDeferredCons = 0; sqlite3CommitInternalChanges(db); } }else{ - sqlite3RollbackAll(db); + sqlite3RollbackAll(db, SQLITE_OK); } db->nStatement = 0; }else if( eStatementOp==0 ){ if( p->rc==SQLITE_OK || p->errorAction==OE_Fail ){ eStatementOp = SAVEPOINT_RELEASE; }else if( p->errorAction==OE_Abort ){ eStatementOp = SAVEPOINT_ROLLBACK; }else{ - invalidateCursorsOnModifiedBtrees(db); - sqlite3RollbackAll(db); + sqlite3RollbackAll(db, SQLITE_ABORT_ROLLBACK); sqlite3CloseSavepoints(db); db->autoCommit = 1; } } @@ -2269,12 +2241,11 @@ if( p->rc==SQLITE_OK || p->rc==SQLITE_CONSTRAINT ){ p->rc = rc; sqlite3DbFree(db, p->zErrMsg); p->zErrMsg = 0; } - invalidateCursorsOnModifiedBtrees(db); - sqlite3RollbackAll(db); + sqlite3RollbackAll(db, SQLITE_ABORT_ROLLBACK); sqlite3CloseSavepoints(db); db->autoCommit = 1; } } Index: test/capi3.test ================================================================== --- test/capi3.test +++ test/capi3.test @@ -892,39 +892,40 @@ } 0 do_test capi3-11.9.2 { catchsql { ROLLBACK; } -} {1 {cannot rollback transaction - SQL statements in progress}} +} {0 {}} do_test capi3-11.9.3 { sqlite3_get_autocommit $DB -} 0 +} 1 do_test capi3-11.10 { sqlite3_step $STMT -} {SQLITE_ROW} +} {SQLITE_ERROR} do_test capi3-11.11 { sqlite3_step $STMT } {SQLITE_ROW} do_test capi3-11.12 { + sqlite3_step $STMT sqlite3_step $STMT } {SQLITE_DONE} do_test capi3-11.13 { sqlite3_finalize $STMT } {SQLITE_OK} do_test capi3-11.14 { execsql { SELECT a FROM t2; } -} {1 2 3} +} {1 2} do_test capi3-11.14.1 { sqlite3_get_autocommit $DB -} 0 +} 1 do_test capi3-11.15 { catchsql { ROLLBACK; } -} {0 {}} +} {1 {cannot rollback - no transaction is active}} do_test capi3-11.15.1 { sqlite3_get_autocommit $DB } 1 do_test capi3-11.16 { execsql { Index: test/capi3c.test ================================================================== --- test/capi3c.test +++ test/capi3c.test @@ -847,39 +847,40 @@ } 0 do_test capi3c-11.9.2 { catchsql { ROLLBACK; } -} {1 {cannot rollback transaction - SQL statements in progress}} +} {0 {}} do_test capi3c-11.9.3 { sqlite3_get_autocommit $DB -} 0 +} 1 do_test capi3c-11.10 { sqlite3_step $STMT -} {SQLITE_ROW} +} {SQLITE_ABORT} do_test capi3c-11.11 { sqlite3_step $STMT } {SQLITE_ROW} do_test capi3c-11.12 { + sqlite3_step $STMT sqlite3_step $STMT } {SQLITE_DONE} do_test capi3c-11.13 { sqlite3_finalize $STMT } {SQLITE_OK} do_test capi3c-11.14 { execsql { SELECT a FROM t2; } -} {1 2 3} +} {1 2} do_test capi3c-11.14.1 { sqlite3_get_autocommit $DB -} 0 +} 1 do_test capi3c-11.15 { catchsql { ROLLBACK; } -} {0 {}} +} {1 {cannot rollback - no transaction is active}} do_test capi3c-11.15.1 { sqlite3_get_autocommit $DB } 1 do_test capi3c-11.16 { execsql { Index: test/incrblob.test ================================================================== --- test/incrblob.test +++ test/incrblob.test @@ -471,19 +471,13 @@ seek $::blob 0 puts -nonewline $::blob "invocation" flush $::blob } {} - # At this point rollback should be illegal (because - # there is an open blob channel). But commit is also illegal because - # the open blob is read-write. - # - do_test incrblob-6.10 { - catchsql { - ROLLBACK; - } db2 - } {1 {cannot rollback transaction - SQL statements in progress}} + # At this point commit should be illegal (because + # there is an open blob channel). + # do_test incrblob-6.11 { catchsql { COMMIT; } db2 } {1 {cannot commit transaction - SQL statements in progress}} Index: test/savepoint.test ================================================================== --- test/savepoint.test +++ test/savepoint.test @@ -301,15 +301,23 @@ do_test savepoint-5.3.1 { execsql {SAVEPOINT abc} catchsql {ROLLBACK TO def} } {1 {no such savepoint: def}} - do_test savepoint-5.3.2 { + do_test savepoint-5.3.2.1 { execsql {SAVEPOINT def} set fd [db incrblob -readonly blobs x 1] + set rc [catch {seek $fd 0;read $fd} res] + lappend rc $res + } {0 {hellontyeight character blob}} + do_test savepoint-5.3.2.2 { catchsql {ROLLBACK TO def} - } {1 {cannot rollback savepoint - SQL statements in progress}} + } {0 {}} + do_test savepoint-5.3.2.3 { + set rc [catch {seek $fd 0; read $fd} res] + set rc + } {1} do_test savepoint-5.3.3 { catchsql {RELEASE def} } {0 {}} do_test savepoint-5.3.4 { close $fd Index: test/shared2.test ================================================================== --- test/shared2.test +++ test/shared2.test @@ -77,52 +77,10 @@ } } list $a $count } {32 64} -#--------------------------------------------------------------------------- -# These tests, shared2.2.*, test the outcome when data is added to or -# removed from a table due to a rollback while a read-uncommitted -# cursor is scanning it. -# -do_test shared2-2.1 { - execsql { - INSERT INTO numbers VALUES(1, 'Medium length text field'); - INSERT INTO numbers VALUES(2, 'Medium length text field'); - INSERT INTO numbers VALUES(3, 'Medium length text field'); - INSERT INTO numbers VALUES(4, 'Medium length text field'); - BEGIN; - DELETE FROM numbers WHERE (a%2)=0; - } db1 - set res [list] - db2 eval { - SELECT a FROM numbers ORDER BY a; - } { - lappend res $a - if {$a==3} { - execsql {ROLLBACK} db1 - } - } - set res -} {1 3 4} -do_test shared2-2.2 { - execsql { - BEGIN; - INSERT INTO numbers VALUES(5, 'Medium length text field'); - INSERT INTO numbers VALUES(6, 'Medium length text field'); - } db1 - set res [list] - db2 eval { - SELECT a FROM numbers ORDER BY a; - } { - lappend res $a - if {$a==5} { - execsql {ROLLBACK} db1 - } - } - set res -} {1 2 3 4 5} db1 close db2 close do_test shared2-3.2 { Index: test/tkt-f777251dc7a.test ================================================================== --- test/tkt-f777251dc7a.test +++ test/tkt-f777251dc7a.test @@ -40,11 +40,11 @@ do_test tkt-f7772-1.2 { catchsql { BEGIN IMMEDIATE; SELECT x, force_rollback(), EXISTS(SELECT 1 FROM t3 WHERE w=x) FROM t2; } -} {1 {callback requested query abort}} +} {1 {abort due to ROLLBACK}} do_test tkt-f7772-1.3 { sqlite3_get_autocommit db } {1} do_test tkt-f7772-2.1 { Index: test/trans3.test ================================================================== --- test/trans3.test +++ test/trans3.test @@ -62,16 +62,15 @@ error $errmsg } } } errmsg] lappend x $errmsg -} {1 {cannot rollback transaction - SQL statements in progress}} +} {1 {abort due to ROLLBACK}} do_test trans3-1.6 { set ::ecode -} {SQLITE_BUSY} +} {} do_test trans3-1.7 { - db eval COMMIT db eval {SELECT * FROM t1} -} {1 2 3 4 5} +} {1 2 3 4} unset -nocomplain ecode finish_test