From 8e4d004ba197a17da4780bc140a780c8b297abec Mon Sep 17 00:00:00 2001 From: dan Date: Mon, 5 Feb 2024 17:35:36 +0000 Subject: [PATCH] Return SQLITE_ABORT if the underlying shadow tables change in the middle of an rtree query in such a way as to invalidate an rtree internal priority queue entry. FossilOrigin-Name: 478280ef67efed854988ab4f740a38ae1937204c0434ad8da11f1869a12a6d06 --- ext/rtree/rtree.c | 23 +++------ ext/rtree/rtreeJ.test | 117 +++++++++++++++++++++++++++++++++++++++++- manifest | 19 ++++--- manifest.uuid | 2 +- 4 files changed, 135 insertions(+), 26 deletions(-) diff --git a/ext/rtree/rtree.c b/ext/rtree/rtree.c index 93a849cf0f..3b26878c9e 100644 --- a/ext/rtree/rtree.c +++ b/ext/rtree/rtree.c @@ -170,7 +170,6 @@ struct Rtree { u32 nBusy; /* Current number of users of this structure */ i64 nRowEst; /* Estimated number of rows in this table */ u32 nCursor; /* Number of open cursors */ - u32 iGeneration; /* Cursors with smaller iGeneration are stale */ u32 nNodeRef; /* Number RtreeNodes with positive nRef */ char *zReadAuxSql; /* SQL for statement to read aux data */ @@ -287,7 +286,6 @@ struct RtreeCursor { u8 atEOF; /* True if at end of search */ u8 bPoint; /* True if sPoint is valid */ u8 bAuxValid; /* True if pReadAux is valid */ - u32 iGeneration; /* Stale if too small */ int iStrategy; /* Copy of idxNum search parameter */ int nConstraint; /* Number of entries in aConstraint */ RtreeConstraint *aConstraint; /* Search constraints. */ @@ -948,6 +946,7 @@ static void nodeGetCoord( int iCoord, /* Which coordinate to extract */ RtreeCoord *pCoord /* OUT: Space to write result to */ ){ + assert( iCellzData[12 + pRtree->nBytesPerCell*iCell + 4*iCoord], pCoord); } @@ -1089,7 +1088,6 @@ static int rtreeOpen(sqlite3_vtab *pVTab, sqlite3_vtab_cursor **ppCursor){ if( pCsr ){ memset(pCsr, 0, sizeof(RtreeCursor)); pCsr->base.pVtab = pVTab; - pCsr->iGeneration = pRtree->iGeneration; rc = SQLITE_OK; pRtree->nCursor++; } @@ -1630,9 +1628,6 @@ static int rtreeStepToLeaf(RtreeCursor *pCur){ int eInt; RtreeSearchPoint x; - if( pCur->iGenerationiGeneration ){ - return SQLITE_ABORT_ROLLBACK; - } eInt = pRtree->eCoordType==RTREE_COORD_INT32; while( (p = rtreeSearchPointFirst(pCur))!=0 && p->iLevel>0 ){ u8 *pCellData; @@ -1726,7 +1721,11 @@ static int rtreeRowid(sqlite3_vtab_cursor *pVtabCursor, sqlite_int64 *pRowid){ int rc = SQLITE_OK; RtreeNode *pNode = rtreeNodeOfFirstSearchPoint(pCsr, &rc); if( rc==SQLITE_OK && ALWAYS(p) ){ - *pRowid = nodeGetRowid(RTREE_OF_CURSOR(pCsr), pNode, p->iCell); + if( p->iCell>=NCELL(pNode) ){ + rc = SQLITE_ABORT; + }else{ + *pRowid = nodeGetRowid(RTREE_OF_CURSOR(pCsr), pNode, p->iCell); + } } return rc; } @@ -1744,6 +1743,7 @@ static int rtreeColumn(sqlite3_vtab_cursor *cur, sqlite3_context *ctx, int i){ if( rc ) return rc; if( NEVER(p==0) ) return SQLITE_OK; + if( p->iCell>=NCELL(pNode) ) return SQLITE_ABORT; if( i==0 ){ sqlite3_result_int64(ctx, nodeGetRowid(pRtree, pNode, p->iCell)); }else if( i<=pRtree->nDim2 ){ @@ -1860,7 +1860,6 @@ static int rtreeFilter( /* Reset the cursor to the same state as rtreeOpen() leaves it in. */ resetCursor(pCsr); - pCsr->iGeneration = pRtree->iGeneration; pCsr->iStrategy = idxNum; if( idxNum==1 ){ @@ -3243,14 +3242,8 @@ static int rtreeEndTransaction(sqlite3_vtab *pVtab){ } static int rtreeRollback(sqlite3_vtab *pVtab){ Rtree *pRtree = (Rtree *)pVtab; - pRtree->iGeneration++; return rtreeEndTransaction(pVtab); } -static int rtreeRollbackTo(sqlite3_vtab *pVtab, int notUsed){ - Rtree *pRtree = (Rtree *)pVtab; - pRtree->iGeneration++; - return SQLITE_OK; -} /* ** The xRename method for rtree module virtual tables. @@ -3374,7 +3367,7 @@ static sqlite3_module rtreeModule = { rtreeRename, /* xRename - rename the table */ rtreeSavepoint, /* xSavepoint */ 0, /* xRelease */ - rtreeRollbackTo, /* xRollbackTo */ + 0, /* xRollbackTo */ rtreeShadowName, /* xShadowName */ rtreeIntegrity /* xIntegrity */ }; diff --git a/ext/rtree/rtreeJ.test b/ext/rtree/rtreeJ.test index 4d9c01d29b..cff77857c7 100644 --- a/ext/rtree/rtreeJ.test +++ b/ext/rtree/rtreeJ.test @@ -44,7 +44,7 @@ do_test 1.2 { } } msg] list $rc $msg -} {1 {abort due to ROLLBACK}} +} {1 {query aborted}} do_execsql_test 1.3 { SELECT * FROM t1; @@ -118,7 +118,7 @@ do_test 1.8 { } } msg] list $rc $msg -} {1 {abort due to ROLLBACK}} +} {1 {query aborted}} do_execsql_test 1.9 { COMMIT; @@ -156,4 +156,117 @@ do_execsql_test 1.12 { SELECT * FROM t1; } {1 1.0 1.0 2 2.0 2.0 3 3.0 3.0} +#---------------------------------------------------------------------- + +reset_db +do_execsql_test 2.0 { + CREATE VIRTUAL TABLE t1 USING rtree(id, x1, x2); + INSERT INTO t1 VALUES(1, 1, 1), (2, 2, 2); + CREATE TABLE t2(x); +} {} + +do_test 2.1 { + db eval { + BEGIN; + INSERT INTO t1 VALUES(3, 3, 3); + PRAGMA writable_schema = RESET; + } + + set rc [catch { + db eval { SELECT x1, x2 FROM t1 } { + if {$x1==1} { + db eval { ROLLBACK } + } + lappend res $x1 $x2 + } + } msg] + list $rc $msg +} {1 {query aborted}} + +do_execsql_test 2.1 { + CREATE TABLE bak_node(nodeno, data); + CREATE TABLE bak_parent(nodeno, parentnode); + CREATE TABLE bak_rowid(rowid, nodeno); +} +proc save_t1 {} { + db eval { + DELETE FROM bak_node; + DELETE FROM bak_parent; + DELETE FROM bak_rowid; + INSERT INTO bak_node SELECT * FROM t1_node; + INSERT INTO bak_parent SELECT * FROM t1_parent; + INSERT INTO bak_rowid SELECT * FROM t1_rowid; + } +} +proc restore_t1 {} { + db eval { + DELETE FROM t1_node; + DELETE FROM t1_parent; + DELETE FROM t1_rowid; + INSERT INTO t1_node SELECT * FROM bak_node; + INSERT INTO t1_parent SELECT * FROM bak_parent; + INSERT INTO t1_rowid SELECT * FROM bak_rowid; + } +} + +do_test 2.3 { + save_t1 + db eval { + INSERT INTO t1 VALUES(3, 3, 3); + } + set rc [catch { + db eval { SELECT rowid, x1, x2 FROM t1 } { + if {$x1==1} { + restore_t1 + } + lappend res $x1 $x2 + } + } msg] + list $rc $msg +} {1 {query aborted}} +do_execsql_test 2.4 { + SELECT * FROM t1 +} {1 1.0 1.0 2 2.0 2.0} + +do_test 2.5 { + save_t1 + db eval { + INSERT INTO t1 VALUES(3, 3, 3); + } + set rc [catch { + db eval { SELECT x1 FROM t1 } { + if {$x1==1} { + restore_t1 + } + lappend res $x1 $x2 + } + } msg] + list $rc $msg +} {1 {query aborted}} +do_execsql_test 2.6 { + SELECT * FROM t1 +} {1 1.0 1.0 2 2.0 2.0} + +do_test 2.7 { + save_t1 + db eval { + INSERT INTO t1 VALUES(3, 3, 3); + } + set ::res [list] + set rc [catch { + db eval { SELECT 'abc' FROM t1 } { + if {$::res==[list]} { + restore_t1 + set ::bDone 1 + } + lappend res abc + } + } msg] + set res +} {abc abc abc} +do_execsql_test 2.6 { + SELECT * FROM t1 +} {1 1.0 1.0 2 2.0 2.0} + + finish_test diff --git a/manifest b/manifest index ff359ab048..b07bcfff0d 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Extend\s[d294a23ed6d]\sto\sapply\sto\sall\swasm\sspeedtest1\sbuilds. -D 2024-02-05T03:56:02.476 +C Return\sSQLITE_ABORT\sif\sthe\sunderlying\sshadow\stables\schange\sin\sthe\smiddle\sof\san\srtree\squery\sin\ssuch\sa\sway\sas\sto\sinvalidate\san\srtree\sinternal\spriority\squeue\sentry. +D 2024-02-05T17:35:36.153 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -495,7 +495,7 @@ F ext/repair/test/checkindex01.test b530f141413b587c9eb78ff734de6bb79bc3515c3350 F ext/repair/test/test.tcl 686d76d888dffd021f64260abf29a55c57b2cedfa7fc69150b42b1d6119aac3c F ext/rtree/README 6315c0d73ebf0ec40dedb5aa0e942bc8b54e3761 F ext/rtree/geopoly.c 0dd4775e896cee6067979d67aff7c998e75c2c9d9cd8d62a1a790c09cde7adca -F ext/rtree/rtree.c 32e67b122e37138694d9f499cdc72241e372058560ac2d4577cfd68ccd692953 +F ext/rtree/rtree.c d96c5d34eafcd46bc4b7645c26a4bcc3169d9e70e9df707ef91c2e90870d00d9 F ext/rtree/rtree.h 4a690463901cb5e6127cf05eb8e642f127012fd5003830dbc974eca5802d9412 F ext/rtree/rtree1.test 2b5b8c719c6a4abe377f57766f428a49af36a93061cb146cccfdc3b30000c0a4 F ext/rtree/rtree2.test 9d9deddbb16fd0c30c36e6b4fdc3ee3132d765567f0f9432ee71e1303d32603d @@ -515,7 +515,7 @@ F ext/rtree/rtreeF.test 81ffa7ef51c4e4618d497a57328c265bf576990c7070633b623b23cd F ext/rtree/rtreeG.test 1b9ca6e3effb48f4161edaa463ddeaa8fca4b2526d084f9cbf5dbe4e0184939c F ext/rtree/rtreeH.test 0885151ee8429242625600ae47142cca935332c70a06737f35af53a7bd7aaf90 F ext/rtree/rtreeI.test 608e77f7fde9be5a12eae316baef640fffaafcfa90a3d67443e78123e19c4ca4 -F ext/rtree/rtreeJ.test bafa7616d6b29448bf19132ce4963a69b629b9ca6ab29f4b76889c8d542b8dfc +F ext/rtree/rtreeJ.test ba4e25c409ebed4b96bf1270e72760c1344d0a464478d221dbe91e8471ac9ac6 F ext/rtree/rtree_perf.tcl 6c18c1f23cd48e0f948930c98dfdd37dfccb5195 F ext/rtree/rtree_util.tcl 202ca70df1f0645ef9d5a2170e62d378a28098d9407f0569e85c9c1cf1bd020a F ext/rtree/rtreecheck.test 934546ad9b563e090ee0c5cbdc69ad014189ad76e5df7320526797a9a345661f @@ -2162,8 +2162,11 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P f8a8b9ee2eddf5f875c7c4399e750ccf1941f767560ebc2c88c083560f5aaae0 -R 8054a7da52bc11a97b67caaa9fc96227 -U stephan -Z 47b7f55b1ab54c273898a455c5270730 +P 26f848e5e0ac34e545d2f27cf33abc46eac13e04ed9cd71084b0f7d47136ff97 +R ac07ea666046dd35038bc77784e8d419 +T *branch * rtree-fix +T *sym-rtree-fix * +T -sym-trunk * +U dan +Z 6f11e0396699443c7473bed77f9c3651 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 16ae9edaa3..19b20648c9 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -26f848e5e0ac34e545d2f27cf33abc46eac13e04ed9cd71084b0f7d47136ff97 \ No newline at end of file +478280ef67efed854988ab4f740a38ae1937204c0434ad8da11f1869a12a6d06 \ No newline at end of file -- 2.39.5