From 20822819adf299e3a7609a26206bf2b43ac402ff Mon Sep 17 00:00:00 2001 From: drh <> Date: Fri, 23 Oct 2026 00:49:30 +0000 Subject: [PATCH] Fix two minor issues with the geopoly extension. Bugs [bugs:/info/2026-10-21T02:53:43Z] and [bugs:/info/2026-10-21T05:30:42Z]. FossilOrigin-Name: d6f1df8c0bbc2c04d9cdbbf33ff6ff9ba35d80f6e33ddab688827e7e2fe568d5 --- ext/rtree/geopoly.c | 50 +++++++++++--- ext/rtree/rtree.c | 22 ++++-- ext/rtree/rtree1.test | 157 ++++++++++++++++++++++++++++++++++++++++++ manifest | 17 +++-- manifest.uuid | 2 +- 5 files changed, 223 insertions(+), 25 deletions(-) diff --git a/ext/rtree/geopoly.c b/ext/rtree/geopoly.c index dd2ea83e6c..ac289f3e33 100644 --- a/ext/rtree/geopoly.c +++ b/ext/rtree/geopoly.c @@ -1389,9 +1389,13 @@ static int geopolyFilter( /* Special case - lookup by rowid. */ RtreeNode *pLeaf; /* Leaf on which the required cell resides */ RtreeSearchPoint *p; /* Search point for the leaf */ - i64 iRowid = sqlite3_value_int64(argv[0]); + i64 iRowid; i64 iNode = 0; - rc = findLeafNode(pRtree, iRowid, &pLeaf, &iNode); + if( rtreeRowidEqValue(argv[0], &iRowid) ){ + rc = findLeafNode(pRtree, iRowid, &pLeaf, &iNode); + }else{ + pLeaf = 0; + } if( rc==SQLITE_OK && pLeaf!=0 ){ p = rtreeSearchPointNew(pCsr, RTREE_ZERO, 0); assert( p!=0 ); /* Always returns pCsr->sPoint */ @@ -1621,6 +1625,8 @@ static int geopolyUpdate( i64 newRowid; /* The new rowid */ int newRowidValid; /* True if newRowid is valid */ int coordChange = 0; /* Change in coordinates */ + sqlite3_value *pShape = 0; /* New _shape value */ + sqlite3_value *pOldShape = 0; /* Copy of the unchanged _shape */ if( pRtree->nNodeRef ){ /* Unable to write to the btree while another cursor is reading from it, @@ -1637,13 +1643,38 @@ static int geopolyUpdate( newRowid = newRowidValid ? sqlite3_value_int64(aData[1]) : 0; cell.iRowid = newRowid; + if( nData>1 ){ + pShape = aData[2]; + if( oldRowidValid && oldRowid!=newRowid && sqlite3_value_nochange(pShape) ){ + /* The rowid changes but _shape does not. aData[2] holds no value in + ** this case, so read the current _shape from the %_rowid table. It + ** is needed for the bounding box of the new r-tree entry and for the + ** %_rowid record of the new rowid. */ + sqlite3_stmt *pRead = 0; + rc = sqlite3_prepare_v3(pRtree->db, pRtree->zReadAuxSql, -1, 0, + &pRead, 0); + if( rc==SQLITE_OK ){ + int rc2; + sqlite3_bind_int64(pRead, 1, oldRowid); + if( sqlite3_step(pRead)==SQLITE_ROW ){ + pOldShape = sqlite3_value_dup(sqlite3_column_value(pRead, 2)); + if( pOldShape==0 ) rc = SQLITE_NOMEM; + pShape = pOldShape; + } + rc2 = sqlite3_finalize(pRead); + if( rc==SQLITE_OK ) rc = rc2; + } + if( rc ) goto geopoly_update_end; + } + } + if( nData>1 /* not a DELETE */ && (!oldRowidValid /* INSERT */ - || !sqlite3_value_nochange(aData[2]) /* UPDATE _shape */ + || !sqlite3_value_nochange(pShape) /* UPDATE _shape */ || oldRowid!=newRowid) /* Rowid change */ ){ - assert( aData[2]!=0 ); - geopolyBBox(0, aData[2], cell.aCoord, &rc); + assert( pShape!=0 ); + geopolyBBox(0, pShape, cell.aCoord, &rc); if( rc ){ if( rc==SQLITE_ERROR ){ pVtab->zErrMsg = @@ -1709,17 +1740,17 @@ static int geopolyUpdate( int nChange = 0; sqlite3_bind_int64(pUp, 1, cell.iRowid); assert( pRtree->nAux>=1 ); - if( sqlite3_value_nochange(aData[2]) ){ + if( sqlite3_value_nochange(pShape) ){ sqlite3_bind_null(pUp, 2); }else{ GeoPoly *p = 0; - if( sqlite3_value_type(aData[2])==SQLITE_TEXT - && (p = geopolyFuncParam(0, aData[2], &rc))!=0 + if( sqlite3_value_type(pShape)==SQLITE_TEXT + && (p = geopolyFuncParam(0, pShape, &rc))!=0 && rc==SQLITE_OK ){ sqlite3_bind_blob(pUp, 2, p->hdr, 4+8*p->nVertex, SQLITE_TRANSIENT); }else{ - sqlite3_bind_value(pUp, 2, aData[2]); + sqlite3_bind_value(pUp, 2, pShape); } sqlite3_free(p); nChange = 1; @@ -1735,6 +1766,7 @@ static int geopolyUpdate( } geopoly_update_end: + sqlite3_value_free(pOldShape); rtreeRelease(pRtree); return rc; } diff --git a/ext/rtree/rtree.c b/ext/rtree/rtree.c index 7de56813ec..eb44b2266b 100644 --- a/ext/rtree/rtree.c +++ b/ext/rtree/rtree.c @@ -1908,6 +1908,20 @@ static int deserializeGeometry(sqlite3_value *pValue, RtreeConstraint *pCons){ int sqlite3IntFloatCompare(i64,double); +/* +** Argument pVal is the right-hand side of a "rowid = ?" constraint. If +** there is an integer that compares equal to pVal, set *piRowid to it and +** return 1. Otherwise - pVal is NULL, a blob, a non-numeric text value or +** a real value that is not an integer - return 0. +*/ +static int rtreeRowidEqValue(sqlite3_value *pVal, i64 *piRowid){ + int eType = sqlite3_value_numeric_type(pVal); + *piRowid = sqlite3_value_int64(pVal); + return eType==SQLITE_INTEGER + || (eType==SQLITE_FLOAT + && 0==sqlite3IntFloatCompare(*piRowid, sqlite3_value_double(pVal))); +} + /* ** Rtree virtual table module xFilter method. */ @@ -1933,13 +1947,9 @@ static int rtreeFilter( /* Special case - lookup by rowid. */ RtreeNode *pLeaf; /* Leaf on which the required cell resides */ RtreeSearchPoint *p; /* Search point for the leaf */ - i64 iRowid = sqlite3_value_int64(argv[0]); + i64 iRowid; i64 iNode = 0; - int eType = sqlite3_value_numeric_type(argv[0]); - if( eType==SQLITE_INTEGER - || (eType==SQLITE_FLOAT - && 0==sqlite3IntFloatCompare(iRowid,sqlite3_value_double(argv[0]))) - ){ + if( rtreeRowidEqValue(argv[0], &iRowid) ){ rc = findLeafNode(pRtree, iRowid, &pLeaf, &iNode); }else{ rc = SQLITE_OK; diff --git a/ext/rtree/rtree1.test b/ext/rtree/rtree1.test index 3e53d4c246..ac40479da3 100644 --- a/ext/rtree/rtree1.test +++ b/ext/rtree/rtree1.test @@ -973,5 +973,162 @@ foreach {tn mod} {1 rtree 2 rtree_i32} { } {ok} } +#------------------------------------------------------------------------- +# Bug 2026-10-21T05:30:42Z. In a geopoly table, an UPDATE that changed +# the rowid without assigning _shape failed with "_shape does not +# contain a valid polygon". +# +ifcapable geopoly { + reset_db + do_execsql_test 29.0 { + CREATE VIRTUAL TABLE g USING geopoly(a); + INSERT INTO g(rowid, _shape, a) VALUES + (1, '[[0,0],[1,0],[0,1],[0,0]]', 'x'), + (2, '[[5,5],[6,5],[5,6],[5,5]]', 'y'); + } + + do_execsql_test 29.1 { + UPDATE g SET rowid = 7 WHERE rowid = 1; + UPDATE g SET rowid = 8, _shape = _shape WHERE rowid = 2; + SELECT rowid, a, geopoly_json(_shape) FROM g ORDER BY rowid; + } {7 x {[[0.0,0.0],[1.0,0.0],[0.0,1.0],[0.0,0.0]]} + 8 y {[[5.0,5.0],[6.0,5.0],[5.0,6.0],[5.0,5.0]]}} + + # The moved row is found by the r-tree using its new rowid. + do_execsql_test 29.2 { + SELECT rowid FROM g + WHERE geopoly_overlap(_shape, '[[0,0],[2,0],[0,2],[0,0]]'); + SELECT rowid FROM g + WHERE geopoly_within(_shape, '[[-1,-1],[3,-1],[-1,3],[-1,-1]]'); + } {7 7} + + # Rowid and an auxiliary column. + do_execsql_test 29.3 { + UPDATE g SET rowid = 9, a = 'z' WHERE rowid = 7; + SELECT rowid, a, geopoly_area(_shape) FROM g ORDER BY rowid; + } {8 y 0.5 9 z 0.5} + + # Auxiliary column only, and the rowid set to its current value. + do_execsql_test 29.4 { + UPDATE g SET a = 'w' WHERE rowid = 9; + UPDATE g SET rowid = 8 WHERE rowid = 8; + SELECT rowid, a, geopoly_area(_shape) FROM g ORDER BY rowid; + } {8 y 0.5 9 w 0.5} + + # A multi-row UPDATE, which uses the two-pass strategy. + do_execsql_test 29.5 { + UPDATE g SET rowid = rowid + 100; + SELECT rowid, a, geopoly_area(_shape) FROM g ORDER BY rowid; + SELECT rowid FROM g + WHERE geopoly_overlap(_shape, '[[4,4],[7,4],[4,7],[4,4]]'); + } {108 y 0.5 109 w 0.5 108} + + # A new rowid that is already in use. The error names the first + # column, as it does for an UPDATE that also assigns _shape. + do_catchsql_test 29.6 { + UPDATE g SET rowid = 108 WHERE rowid = 109; + } {1 {UNIQUE constraint failed: g._shape}} + do_execsql_test 29.7 { + UPDATE OR IGNORE g SET rowid = 108 WHERE rowid = 109; + SELECT rowid, a FROM g ORDER BY rowid; + } {108 y 109 w} + do_execsql_test 29.8 { + UPDATE OR REPLACE g SET rowid = 108 WHERE rowid = 109; + SELECT rowid, a, geopoly_area(_shape) FROM g ORDER BY rowid; + } {108 w 0.5} + + # UPDATE ... FROM, and an UPDATE inside a trigger. + do_execsql_test 29.9 { + INSERT INTO g(rowid, _shape, a) VALUES + (3, '[[5,5],[7,5],[5,7],[5,5]]', 'v'); + CREATE TABLE m(o, n); + INSERT INTO m VALUES(108, 18), (3, 13); + UPDATE g SET rowid = m.n FROM m WHERE m.o = g.rowid; + SELECT rowid, a, geopoly_area(_shape) FROM g ORDER BY rowid; + } {13 v 2.0 18 w 0.5} + do_execsql_test 29.10 { + CREATE TABLE t(x); + CREATE TRIGGER tr AFTER INSERT ON t BEGIN + UPDATE g SET rowid = new.x WHERE rowid = 13; + END; + INSERT INTO t VALUES(23); + SELECT rowid, a, geopoly_area(_shape) FROM g ORDER BY rowid; + } {18 w 0.5 23 v 2.0} + + do_execsql_test 29.11 { + PRAGMA integrity_check; + } {ok} +} + +#------------------------------------------------------------------------- +# Bug 2026-10-21T02:53:43Z. On a geopoly table, "rowid = ?" with a value +# that is not equal to any integer found the row whose rowid was the +# integer part of the value. R-Tree tables got this right, except that +# a text value in exponent form, like '1e1', was converted to an +# integer using only the digits before the "e". +# +# Each lookup is compared with the same lookup on an ordinary table. +# +reset_db +do_execsql_test 30.0 { + CREATE TABLE o(a); + CREATE VIRTUAL TABLE r USING rtree(id, x1, x2); + INSERT INTO o(rowid) VALUES + (0), (1), (6), (7), (10), + (9223372036854775807), (-9223372036854775808); + INSERT INTO r SELECT rowid, 0, 1 FROM o; +} +ifcapable geopoly { + do_execsql_test 30.0.1 { + CREATE VIRTUAL TABLE g USING geopoly(); + INSERT INTO g(rowid, _shape) + SELECT rowid, '[[0,0],[1,0],[0,1],[0,0]]' FROM o; + } +} + +set tn 0 +foreach v { + 6 6.0 -0.0 '6' {' 6 '} '6.0' '6e0' '0.6e1' '1e1' '10.0e0' + 6.5 6.000000000000001 '6.5' '6abc' 'x' '0x6' x'36' NULL + 1e19 -1e19 9223372036854775807.0 '9223372036854775808' + 9223372036854775807 '9223372036854775807' -9223372036854775808 +} { + incr tn + set expect [db eval "SELECT rowid FROM o WHERE rowid = $v"] + do_execsql_test 30.$tn.1 " + SELECT id FROM r WHERE id = $v + " $expect + do_execsql_test 30.$tn.2 " + SELECT id FROM r WHERE rowid IN ($v, 7) + " [lsort -integer [concat $expect 7]] + ifcapable geopoly { + do_execsql_test 30.$tn.3 " + SELECT rowid FROM g WHERE rowid = $v + " $expect + do_execsql_test 30.$tn.4 " + SELECT rowid FROM g WHERE rowid IN ($v, 7) + " [lsort -integer [concat $expect 7]] + } +} + +ifcapable geopoly { + do_execsql_test 30.100 { + CREATE TABLE t1(x); + INSERT INTO t1 VALUES (6), (6.5), ('7abc'), ('1e1'); + SELECT t1.x, g.rowid FROM t1 LEFT JOIN g ON g.rowid = t1.x; + } {6 6 6.5 {} 7abc {} 1e1 10} + do_execsql_test 30.101 { + DELETE FROM g WHERE rowid = 7.5; + UPDATE g SET rowid = 70 WHERE rowid = '7abc'; + SELECT rowid FROM g WHERE rowid BETWEEN 0 AND 100; + } {0 1 6 7 10} + do_execsql_test 30.102 { + DELETE FROM g WHERE rowid = 7.0; + SELECT rowid FROM g WHERE rowid BETWEEN 0 AND 100; + } {0 1 6 10} +} +do_execsql_test 30.103 { + SELECT t1.x, r.id FROM t1 LEFT JOIN r ON r.id = t1.x; +} {6 6 6.5 {} 7abc {} 1e1 10} finish_test diff --git a/manifest b/manifest index 6fef537974..31545509bd 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Performance\simprovements\sin\sPRAGMA\sintegrity_check\sand\sin\smaterialized\sCTEs. -D 2026-10-22T20:03:37.533 +C Fix\stwo\sminor\sissues\swith\sthe\sgeopoly\sextension.\nBugs\s[bugs:/info/2026-10-21T02:53:43Z]\nand\s[bugs:/info/2026-10-21T05:30:42Z]. +D 2026-10-23T00:49:30.472 F .fossil-settings/binary-glob 61195414528fb3ea9693577e1980230d78a1f8b0a54c78cf1b9b24d0a409ed6a x F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea @@ -498,10 +498,10 @@ F ext/recover/sqlite3recover.c 7a049049c9844837a82a28bed0885ea4515cb44b60c227eba F ext/recover/sqlite3recover.h 011c799f02deb70ab685916f6f538e6bb32c4e0025e79bfd0e24ff9c74820959 F ext/recover/test_recover.c 3d0fb1df7823f5bc22a0b93955034d16a2dfa2eb1e443e9a0123a77f120599a3 F ext/rtree/README 734aa36238bcd2dee91db5dba107d5fcbdb02396612811377a8ad50f1272b1c1 -F ext/rtree/geopoly.c 5d8bdbd239c806bd71de10b4aacd351309550719bb2776f80239ddb962b175d7 -F ext/rtree/rtree.c c3f17641d455e1cf4b2e104696ebe3b4b5476b67fb9ed5ee036aba19c84bfc7d +F ext/rtree/geopoly.c b15c2be7449c4f37c0bc0e6150f2c810dbc50b670af75691430c9bc7e9b52166 +F ext/rtree/rtree.c 116771ac0a5eaef23bea8fbbbeee99ad8e8b677ed758c9084cad0344fd2b5f28 F ext/rtree/rtree.h 4a690463901cb5e6127cf05eb8e642f127012fd5003830dbc974eca5802d9412 -F ext/rtree/rtree1.test 1b8861ed86b5856dc2a94f0e72d6fe011667e8066e923a02e6b351b706d14d05 +F ext/rtree/rtree1.test ae7af664671b73187d4a947601a551de7b6095a6a92d410104cecdc5b77beff1 F ext/rtree/rtree2.test 9d9deddbb16fd0c30c36e6b4fdc3ee3132d765567f0f9432ee71e1303d32603d F ext/rtree/rtree3.test 272594f88c344e973864008bbe4c71fd3a41a264c097d568593ee7886d83d409 F ext/rtree/rtree4.test 304de65d484540111b896827e4261815e5dca4ce28eeecd58be648cd73452c4b @@ -2297,9 +2297,8 @@ F tool/warnings-clang.sh bbf6a1e685e534c92ec2bfba5b1745f34fb6f0bc2a362850723a9ee F tool/warnings.sh a554d13f6e5cf3760f041b87939e3d616ec6961859c3245e8ef701d1eafc2ca2 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f F tool/winmain.c 00c8fb88e365c9017db14c73d3c78af62194d9644feaf60e220ab0f411f3604c -P 14d924c33ff11babef13e68f178c715bb8744a0134ee580715486b601d5738d8 d0b5bc53a9d828c425ea29bc418688aa7df20e8cc4b69b81acb0ac1a7c169b6a -R 9c27d1388fc21c7d73384c44f57158ba -T +closed d0b5bc53a9d828c425ea29bc418688aa7df20e8cc4b69b81acb0ac1a7c169b6a +P 52d0c0b3d18b032e2d2c006a5d60df4c65da3b95200eaaffddb0ede9ed63f29f +R d78d70775b4d143ef12b7c5ee188934a U drh -Z 8abe2cfbcb64db54b4e6f596a898f36f +Z 7b38a1ec45246b3ad601d2365201294e # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 38e490acfe..3ad0d9ffb3 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -52d0c0b3d18b032e2d2c006a5d60df4c65da3b95200eaaffddb0ede9ed63f29f +d6f1df8c0bbc2c04d9cdbbf33ff6ff9ba35d80f6e33ddab688827e7e2fe568d5