From: drh <>
Date: Tue, 6 Feb 2024 13:36:29 +0000 (+0000)
Subject: Return SQLITE_ABORT if the underlying shadow tables change in the middle of an rtree... 
X-Git-Tag: version-3.45.2~29
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ae819b3a507ee946b636b811fa30c0113f2b153e;p=thirdparty%2Fsqlite.git

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. This replaces the SQLITE_ABORT_ROLLBACK mechanism added in [97cffff331b].

FossilOrigin-Name: 061af0d74a436efa24b348e9a0f9fe6d6d52c02e5c03929e6f9c7f0e2ef3e2a0
---

diff --git a/ext/rtree/rtree.c b/ext/rtree/rtree.c
index 93a849cf0f..578e6e8623 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( iCell<NCELL(pNode) );
   readCoord(&pNode->zData[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->iGeneration<pRtree->iGeneration ){
-    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 ){
@@ -3242,15 +3241,8 @@ static int rtreeEndTransaction(sqlite3_vtab *pVtab){
   return SQLITE_OK;
 }
 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 +3366,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 69be5e618f..889478cb71 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Following\sa\sROLLBACK\sthat\sreverts\schanges\sto\san\sRTREE,\sany\spending\squeries\nagainst\sthat\ssame\sRTREE\sabort\swith\scode\sSQLITE_ABORT_ROLLBACK.
-D 2024-02-03T19:41:34.769
+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.\sThis\sreplaces\sthe\sSQLITE_ABORT_ROLLBACK\smechanism\sadded\sin\s[97cffff331b].
+D 2024-02-06T13:36:29.981
 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 920884cfeb88e8a77ef23d70bc696c1a59f2de03b8e877c0966ae81ea61aa41a
 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
@@ -2160,9 +2160,9 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P e876e51a0ed5c5b3126f52e532044363a014bc594cfefa87ffb5b82257cc467a
-Q +af5c425114f32c2f84aea20edd4fa46eb1bfdeb3747fce357540e15978a070c8
-R 4edd5e2f0f8e230b9675b1cd352d83c5
+P 97cffff331b5f95abed5451f9d0cef085c3fc8911efbcd2422c1c78eac17b669
+Q +32f85a5ce8e32506ad0cf309c86589958f38d924b4b3de532bec8bdb8e385fad
+R f8ca2c60344dc198d39fb61edc525866
 U drh
-Z 79e364f00ac0aac7cc3cc3b3acce496b
+Z f54a4393317cb6a06f178568d4a34e97
 # Remove this line to create a well-formed Fossil manifest.
diff --git a/manifest.uuid b/manifest.uuid
index 69baa008b5..be1b7e38c3 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-97cffff331b5f95abed5451f9d0cef085c3fc8911efbcd2422c1c78eac17b669
\ No newline at end of file
+061af0d74a436efa24b348e9a0f9fe6d6d52c02e5c03929e6f9c7f0e2ef3e2a0
\ No newline at end of file