Index: src/expr.c ================================================================== --- src/expr.c +++ src/expr.c @@ -2367,11 +2367,12 @@ int iAddr = sqlite3VdbeAddOp0(v, OP_Once); VdbeCoverage(v); sqlite3OpenTable(pParse, iTab, iDb, pTab, OP_OpenRead); eType = IN_INDEX_ROWID; - + ExplainQueryPlan((pParse, 0, + "USING ROWID SEARCH ON TABLE %s FOR IN-OPERATOR",pTab->zName)); sqlite3VdbeJumpHere(v, iAddr); }else{ Index *pIdx; /* Iterator variable */ int affinity_ok = 1; int i; Index: src/main.c ================================================================== --- src/main.c +++ src/main.c @@ -4207,15 +4207,33 @@ if( db->autoCommit==0 ){ int iDb; iDb = sqlite3FindDbName(db, zDb); if( iDb==0 || iDb>1 ){ Btree *pBt = db->aDb[iDb].pBt; - if( 0==sqlite3BtreeIsInReadTrans(pBt) ){ - rc = sqlite3PagerSnapshotOpen(sqlite3BtreePager(pBt), pSnapshot); + if( sqlite3BtreeIsInTrans(pBt)==0 ){ + Pager *pPager = sqlite3BtreePager(pBt); + int bUnlock = 0; + if( sqlite3BtreeIsInReadTrans(pBt) ){ + if( db->nVdbeActive==0 ){ + rc = sqlite3PagerSnapshotCheck(pPager, pSnapshot); + if( rc==SQLITE_OK ){ + bUnlock = 1; + rc = sqlite3BtreeCommit(pBt); + } + } + }else{ + rc = SQLITE_OK; + } + if( rc==SQLITE_OK ){ + rc = sqlite3PagerSnapshotOpen(pPager, pSnapshot); + } if( rc==SQLITE_OK ){ rc = sqlite3BtreeBeginTrans(pBt, 0, 0); - sqlite3PagerSnapshotOpen(sqlite3BtreePager(pBt), 0); + sqlite3PagerSnapshotOpen(pPager, 0); + } + if( bUnlock ){ + sqlite3PagerSnapshotUnlock(pPager); } } } } Index: src/os_unix.c ================================================================== --- src/os_unix.c +++ src/os_unix.c @@ -2109,11 +2109,11 @@ /* unixFile.pInode is always valid here. Otherwise, a different close ** routine (e.g. nolockClose()) would be called instead. */ assert( pFile->pInode->nLock>0 || pFile->pInode->bProcessLock==0 ); sqlite3_mutex_enter(pInode->pLockMutex); - if( pFile->pInode->nLock ){ + if( pInode->nLock ){ /* If there are outstanding locks, do not actually close the file just ** yet because that would clear those locks. Instead, add the file ** descriptor to pInode->pUnused list. It will be automatically closed ** when the last lock is cleared. */ Index: src/pager.c ================================================================== --- src/pager.c +++ src/pager.c @@ -7651,10 +7651,42 @@ }else{ rc = SQLITE_ERROR; } return rc; } + +/* +** The caller currently has a read transaction open on the database. +** If this is not a WAL database, SQLITE_ERROR is returned. Otherwise, +** this function takes a SHARED lock on the CHECKPOINTER slot and then +** checks if the snapshot passed as the second argument is still +** available. If so, SQLITE_OK is returned. +** +** If the snapshot is not available, SQLITE_ERROR is returned. Or, if +** the CHECKPOINTER lock cannot be obtained, SQLITE_BUSY. If any error +** occurs (any value other than SQLITE_OK is returned), the CHECKPOINTER +** lock is released before returning. +*/ +int sqlite3PagerSnapshotCheck(Pager *pPager, sqlite3_snapshot *pSnapshot){ + int rc; + if( pPager->pWal ){ + rc = sqlite3WalSnapshotCheck(pPager->pWal, pSnapshot); + }else{ + rc = SQLITE_ERROR; + } + return rc; +} + +/* +** Release a lock obtained by an earlier successful call to +** sqlite3PagerSnapshotCheck(). +*/ +void sqlite3PagerSnapshotUnlock(Pager *pPager){ + assert( pPager->pWal ); + return sqlite3WalSnapshotUnlock(pPager->pWal); +} + #endif /* SQLITE_ENABLE_SNAPSHOT */ #endif /* !SQLITE_OMIT_WAL */ #ifdef SQLITE_ENABLE_ZIPVFS /* Index: src/pager.h ================================================================== --- src/pager.h +++ src/pager.h @@ -184,10 +184,12 @@ # endif # ifdef SQLITE_ENABLE_SNAPSHOT int sqlite3PagerSnapshotGet(Pager *pPager, sqlite3_snapshot **ppSnapshot); int sqlite3PagerSnapshotOpen(Pager *pPager, sqlite3_snapshot *pSnapshot); int sqlite3PagerSnapshotRecover(Pager *pPager); + int sqlite3PagerSnapshotCheck(Pager *pPager, sqlite3_snapshot *pSnapshot); + void sqlite3PagerSnapshotUnlock(Pager *pPager); # endif #else # define sqlite3PagerUseWal(x,y) 0 #endif Index: src/sqlite.h.in ================================================================== --- src/sqlite.h.in +++ src/sqlite.h.in @@ -9033,26 +9033,37 @@ /* ** CAPI3REF: Start a read transaction on an historical snapshot ** METHOD: sqlite3_snapshot ** -** ^The [sqlite3_snapshot_open(D,S,P)] interface starts a -** read transaction for schema S of -** [database connection] D such that the read transaction -** refers to historical [snapshot] P, rather than the most -** recent change to the database. -** ^The [sqlite3_snapshot_open()] interface returns SQLITE_OK on success -** or an appropriate [error code] if it fails. -** -** ^In order to succeed, a call to [sqlite3_snapshot_open(D,S,P)] must be -** the first operation following the [BEGIN] that takes the schema S -** out of [autocommit mode]. -** ^In other words, schema S must not currently be in -** a transaction for [sqlite3_snapshot_open(D,S,P)] to work, but the -** database connection D must be out of [autocommit mode]. -** ^A [snapshot] will fail to open if it has been overwritten by a -** [checkpoint]. +** ^The [sqlite3_snapshot_open(D,S,P)] interface either starts a new read +** transaction or upgrades an existing one for schema S of +** [database connection] D such that the read transaction refers to +** historical [snapshot] P, rather than the most recent change to the +** database. ^The [sqlite3_snapshot_open()] interface returns SQLITE_OK +** on success or an appropriate [error code] if it fails. +** +** ^In order to succeed, the database connection must not be in +** [autocommit mode] when [sqlite3_snapshot_open(D,S,P)] is called. If there +** is already a read transaction open on schema S, then the database handle +** must have no active statements (SELECT statements that have been passed +** to sqlite3_step() but not sqlite3_reset() or sqlite3_finalize()). +** SQLITE_ERROR is returned if either of these conditions is violated, or +** if schema S does not exist, or if the snapshot object is invalid. +** +** ^A call to sqlite3_snapshot_open() will fail to open if the specified +** snapshot has been overwritten by a [checkpoint]. In this case +** SQLITE_BUSY_SNAPSHOT is returned. +** +** If there is already a read transaction open when this function is +** invoked, then the same read transaction remains open (on the same +** database snapshot) if SQLITE_ERROR, SQLITE_BUSY or SQLITE_BUSY_SNAPSHOT +** is returned. If another error code - for example SQLITE_PROTOCOL or an +** SQLITE_IOERR error code - is returned, then the final state of the +** read transaction is undefined. If SQLITE_OK is returned, then the +** read transaction is now open on database snapshot P. +** ** ^(A call to [sqlite3_snapshot_open(D,S,P)] will fail if the ** database connection D does not know that the database file for ** schema S is in [WAL mode]. A database connection might not know ** that the database file is in [WAL mode] if there has been no prior ** I/O on that database connection, or if the database entered [WAL mode] Index: src/test1.c ================================================================== --- src/test1.c +++ src/test1.c @@ -2391,10 +2391,12 @@ rc = sqlite3_snapshot_open(db, zName, pSnapshot); if( rc!=SQLITE_OK ){ Tcl_SetObjResult(interp, Tcl_NewStringObj(sqlite3ErrName(rc), -1)); return TCL_ERROR; + }else{ + Tcl_ResetResult(interp); } return TCL_OK; } #endif /* SQLITE_ENABLE_SNAPSHOT */ Index: src/vdbemem.c ================================================================== --- src/vdbemem.c +++ src/vdbemem.c @@ -1780,15 +1780,15 @@ const void *pRec, /* Pointer to buffer containing record */ int nRec, /* Size of buffer pRec in bytes */ int iCol, /* Column to extract */ sqlite3_value **ppVal /* OUT: Extracted value */ ){ - u32 t; /* a column type code */ + u32 t = 0; /* a column type code */ int nHdr; /* Size of the header in the record */ int iHdr; /* Next unread header byte */ int iField; /* Next unread data byte */ - int szField; /* Size of the current data field */ + int szField = 0; /* Size of the current data field */ int i; /* Column index */ u8 *a = (u8*)pRec; /* Typecast byte array */ Mem *pMem = *ppVal; /* Write result into this Mem object */ assert( iCol>0 ); Index: src/wal.c ================================================================== --- src/wal.c +++ src/wal.c @@ -3767,10 +3767,47 @@ if( pHdr1->aSalt[0]>pHdr2->aSalt[0] ) return +1; if( pHdr1->mxFramemxFrame ) return -1; if( pHdr1->mxFrame>pHdr2->mxFrame ) return +1; return 0; } + +/* +** The caller currently has a read transaction open on the database. +** This function takes a SHARED lock on the CHECKPOINTER slot and then +** checks if the snapshot passed as the second argument is still +** available. If so, SQLITE_OK is returned. +** +** If the snapshot is not available, SQLITE_ERROR is returned. Or, if +** the CHECKPOINTER lock cannot be obtained, SQLITE_BUSY. If any error +** occurs (any value other than SQLITE_OK is returned), the CHECKPOINTER +** lock is released before returning. +*/ +int sqlite3WalSnapshotCheck(Wal *pWal, sqlite3_snapshot *pSnapshot){ + int rc; + rc = walLockShared(pWal, WAL_CKPT_LOCK); + if( rc==SQLITE_OK ){ + WalIndexHdr *pNew = (WalIndexHdr*)pSnapshot; + if( memcmp(pNew->aSalt, pWal->hdr.aSalt, sizeof(pWal->hdr.aSalt)) + || pNew->mxFramenBackfillAttempted + ){ + rc = SQLITE_BUSY_SNAPSHOT; + walUnlockShared(pWal, WAL_CKPT_LOCK); + } + } + return rc; +} + +/* +** Release a lock obtained by an earlier successful call to +** sqlite3WalSnapshotCheck(). +*/ +void sqlite3WalSnapshotUnlock(Wal *pWal){ + assert( pWal ); + walUnlockShared(pWal, WAL_CKPT_LOCK); +} + + #endif /* SQLITE_ENABLE_SNAPSHOT */ #ifdef SQLITE_ENABLE_ZIPVFS /* ** If the argument is not NULL, it points to a Wal object that holds a Index: src/wal.h ================================================================== --- src/wal.h +++ src/wal.h @@ -130,10 +130,12 @@ #ifdef SQLITE_ENABLE_SNAPSHOT int sqlite3WalSnapshotGet(Wal *pWal, sqlite3_snapshot **ppSnapshot); void sqlite3WalSnapshotOpen(Wal *pWal, sqlite3_snapshot *pSnapshot); int sqlite3WalSnapshotRecover(Wal *pWal); +int sqlite3WalSnapshotCheck(Wal *pWal, sqlite3_snapshot *pSnapshot); +void sqlite3WalSnapshotUnlock(Wal *pWal); #endif #ifdef SQLITE_ENABLE_ZIPVFS /* If the WAL file is not empty, return the number of bytes of content ** stored in each frame (i.e. the db page-size when the WAL was created). Index: test/eqp.test ================================================================== --- test/eqp.test +++ test/eqp.test @@ -741,12 +741,86 @@ det 8.2.4 "SELECT count(*) FROM t1" { QUERY PLAN `--SCAN TABLE t1 } - - - - - +# 2018-08-16: While working on Fossil I discovered that EXPLAIN QUERY PLAN +# did not describe IN operators implemented using a ROWID lookup. These +# test cases ensure that problem as been fixed. +# +do_execsql_test 9.0 { + -- Schema from Fossil 2018-08-16 + CREATE TABLE forumpost( + fpid INTEGER PRIMARY KEY, + froot INT, + fprev INT, + firt INT, + fmtime REAL + ); + CREATE INDEX forumthread ON forumpost(froot,fmtime); + CREATE TABLE blob( + rid INTEGER PRIMARY KEY, + rcvid INTEGER, + size INTEGER, + uuid TEXT UNIQUE NOT NULL, + content BLOB, + CHECK( length(uuid)>=40 AND rid>0 ) + ); + CREATE TABLE event( + type TEXT, + mtime DATETIME, + objid INTEGER PRIMARY KEY, + tagid INTEGER, + uid INTEGER REFERENCES user, + bgcolor TEXT, + euser TEXT, + user TEXT, + ecomment TEXT, + comment TEXT, + brief TEXT, + omtime DATETIME + ); + CREATE INDEX event_i1 ON event(mtime); + CREATE TABLE private(rid INTEGER PRIMARY KEY); +} +do_eqp_test 9.1 { + WITH thread(age,duration,cnt,root,last) AS ( + SELECT + julianday('now') - max(fmtime) AS age, + max(fmtime) - min(fmtime) AS duration, + sum(fprev IS NULL) AS msg_count, + froot, + (SELECT fpid FROM forumpost + WHERE froot=x.froot + AND fpid NOT IN private + ORDER BY fmtime DESC LIMIT 1) + FROM forumpost AS x + WHERE fpid NOT IN private --- Ensure this table mentioned in EQP output! + GROUP BY froot + ORDER BY 1 LIMIT 26 OFFSET 5 + ) + SELECT + thread.age, + thread.duration, + thread.cnt, + blob.uuid, + substr(event.comment,instr(event.comment,':')+1) + FROM thread, blob, event + WHERE blob.rid=thread.last + AND event.objid=thread.last + ORDER BY 1; +} { + QUERY PLAN + |--MATERIALIZE xxxxxx + | |--SCAN TABLE forumpost AS x USING INDEX forumthread + | |--USING ROWID SEARCH ON TABLE private FOR IN-OPERATOR + | |--CORRELATED SCALAR SUBQUERY + | | |--SEARCH TABLE forumpost USING COVERING INDEX forumthread (froot=?) + | | `--USING ROWID SEARCH ON TABLE private FOR IN-OPERATOR + | `--USE TEMP B-TREE FOR ORDER BY + |--SCAN SUBQUERY xxxxxx + |--SEARCH TABLE blob USING INTEGER PRIMARY KEY (rowid=?) + |--SEARCH TABLE event USING INTEGER PRIMARY KEY (rowid=?) + `--USE TEMP B-TREE FOR ORDER BY +} finish_test Index: test/snapshot.test ================================================================== --- test/snapshot.test +++ test/snapshot.test @@ -215,13 +215,23 @@ execsql { BEGIN; SELECT * FROM t2; } } {a b c d e f} - do_test $tn.3.2.2 { - list [catch {snapshot_open db main $snapshot } msg] $msg + + # Update - it is no longer an error to have a read-transaction open, + # provided there are no active SELECT statements. + do_test $tn.3.2.2a { + db eval "SELECT * FROM t2" { + set res [list [catch {snapshot_open db main $snapshot } msg] $msg] + break + } + set res } {1 SQLITE_ERROR} + do_test $tn.3.2.2b { + snapshot_open db main $snapshot + } {} do_test $tn.3.2.3 { execsql { COMMIT; BEGIN; @@ -229,15 +239,20 @@ } list [catch {snapshot_open db main $snapshot } msg] $msg } {1 SQLITE_ERROR} do_execsql_test $tn.3.2.4 COMMIT - do_test $tn.3.3.1 { + do_test $tn.3.3.1a { execsql { PRAGMA journal_mode = DELETE } execsql { BEGIN } list [catch {snapshot_open db main $snapshot } msg] $msg } {1 SQLITE_ERROR} + + do_test $tn.3.3.1b { + execsql { COMMIT ; BEGIN ; SELECT * FROM t2 } + list [catch {snapshot_open db main $snapshot } msg] $msg + } {1 SQLITE_ERROR} do_test $tn.$tn.3.3.2 { snapshot_free $snapshot execsql COMMIT } {} ADDED test/snapshot_up.test Index: test/snapshot_up.test ================================================================== --- test/snapshot_up.test +++ test/snapshot_up.test @@ -0,0 +1,184 @@ +# 2018 August 6 +# +# 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. +# +#*********************************************************************** +# +# Tests for calling sqlite3_snapshot_open() when there is already +# a read transaction open on the database. +# + +set testdir [file dirname $argv0] +source $testdir/tester.tcl +ifcapable !snapshot {finish_test; return} +set testprefix snapshot_up + +# This test does not work with the inmemory_journal permutation. The reason +# is that each connection opened as part of this permutation executes +# "PRAGMA journal_mode=memory", which fails if the database is in wal mode +# and there are one or more existing connections. +if {[permutation]=="inmemory_journal"} { + finish_test + return +} + +do_execsql_test 1.0 { + CREATE TABLE t1(a, b, c); + PRAGMA journal_mode = wal; + INSERT INTO t1 VALUES(1, 2, 3); + INSERT INTO t1 VALUES(4, 5, 6); + INSERT INTO t1 VALUES(7, 8, 9); +} {wal} + +do_test 1.1 { + execsql BEGIN + set ::snap1 [sqlite3_snapshot_get db main] + execsql COMMIT + execsql { INSERT INTO t1 VALUES(10, 11, 12); } + execsql BEGIN + set ::snap2 [sqlite3_snapshot_get db main] + execsql COMMIT + execsql { INSERT INTO t1 VALUES(13, 14, 15); } + execsql BEGIN + set ::snap3 [sqlite3_snapshot_get db main] + execsql COMMIT +} {} + +do_execsql_test 1.2 { + BEGIN; + SELECT * FROM t1 +} {1 2 3 4 5 6 7 8 9 10 11 12 13 14 15} + +do_test 1.3 { + sqlite3_snapshot_open db main $::snap1 + execsql { SELECT * FROM t1 } +} {1 2 3 4 5 6 7 8 9} + +do_test 1.4 { + sqlite3_snapshot_open db main $::snap2 + execsql { SELECT * FROM t1 } +} {1 2 3 4 5 6 7 8 9 10 11 12} + +do_test 1.5 { + sqlite3 db2 test.db + execsql { PRAGMA wal_checkpoint } db2 +} {0 5 4} + +do_execsql_test 1.6 { + SELECT * FROM t1 +} {1 2 3 4 5 6 7 8 9 10 11 12} + +do_test 1.7 { + list [catch { sqlite3_snapshot_open db main $::snap1 } msg] $msg +} {1 SQLITE_BUSY_SNAPSHOT} + +do_execsql_test 1.8 { + SELECT * FROM t1 +} {1 2 3 4 5 6 7 8 9 10 11 12} + +do_test 1.9 { + execsql { COMMIT ; BEGIN } + list [catch { sqlite3_snapshot_open db main $::snap1 } msg] $msg +} {1 SQLITE_BUSY_SNAPSHOT} + +do_test 1.10 { + execsql { COMMIT } + execsql { + PRAGMA wal_checkpoint; + DELETE FROM t1 WHERE a = 1; + } db2 + execsql BEGIN + set ::snap4 [sqlite3_snapshot_get db main] + execsql COMMIT + execsql { + DELETE FROM t1 WHERE a = 4; + } db2 +} {} + +do_test 1.11 { + execsql { + BEGIN; + SELECT * FROM t1 + } +} {7 8 9 10 11 12 13 14 15} +do_test 1.12 { + sqlite3_snapshot_open db main $::snap4 + execsql { SELECT * FROM t1 } +} {4 5 6 7 8 9 10 11 12 13 14 15} + +do_test 1.13 { + list [catch { sqlite3_snapshot_open db main $::snap3 } msg] $msg +} {1 SQLITE_BUSY_SNAPSHOT} +do_test 1.14 { + execsql { SELECT * FROM t1 } +} {4 5 6 7 8 9 10 11 12 13 14 15} + +db close +db2 close +sqlite3 db test.db +do_execsql_test 1.15 { + BEGIN; + SELECT * FROM t1 +} {7 8 9 10 11 12 13 14 15} +do_test 1.16 { + list [catch { sqlite3_snapshot_open db main $::snap4 } msg] $msg +} {1 SQLITE_BUSY_SNAPSHOT} +do_execsql_test 1.17 { COMMIT } + +sqlite3_snapshot_free $::snap1 +sqlite3_snapshot_free $::snap2 +sqlite3_snapshot_free $::snap3 +sqlite3_snapshot_free $::snap4 + +#------------------------------------------------------------------------- +catch { db close } +sqlite3 db test.db +sqlite3 db2 test.db +sqlite3 db3 test.db + +proc xBusy {args} { return 1 } +db3 busy xBusy + +do_test 2.1 { + execsql { INSERT INTO t1 VALUES(16, 17, 18) } db2 + execsql BEGIN + set ::snap1 [sqlite3_snapshot_get db main] + execsql COMMIT + execsql { INSERT INTO t1 VALUES(19, 20, 21) } db2 + execsql BEGIN + set ::snap2 [sqlite3_snapshot_get db main] + execsql COMMIT + set {} {} +} {} + +do_execsql_test -db db2 2.2 { + BEGIN; + INSERT INTO t1 VALUES(19, 20, 21); +} + +do_test 2.3 { + execsql BEGIN + sqlite3_snapshot_open db main $::snap1 + execsql { SELECT * FROM t1 } +} {7 8 9 10 11 12 13 14 15 16 17 18} + +proc xBusy {args} { + set ::res [list [catch { sqlite3_snapshot_open db main $::snap2 } msg] $msg] + return 1 +} +db3 busy xBusy +do_test 2.4 { + execsql {PRAGMA wal_checkpoint = restart} db3 + set ::res +} {1 SQLITE_BUSY} + +sqlite3_snapshot_free $::snap1 +sqlite3_snapshot_free $::snap2 + +finish_test +