From: dan Date: Fri, 7 Oct 2022 18:57:15 +0000 (+0000) Subject: Fix a problem causing the seek-scan optimization to skip over valid rows that could... X-Git-Tag: version-3.40.0~176 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=73c586bcbfb4adbd275c46cc151198b099a90a70;p=thirdparty%2Fsqlite.git Fix a problem causing the seek-scan optimization to skip over valid rows that could occur when it is used with expressions of the form (a IN (?,?..) AND b >= ?). dbsqlfuzz ab1db6dc0efb04cba1cd3431ee6da4894fdc4520. FossilOrigin-Name: 63d9efe277759d4daa29794846b60c6f55491496618f423f61468df72d0a4633 --- diff --git a/manifest b/manifest index a75bc14a8f..cdd6fae9bf 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Improved\stracing\soutput\sfor\sshowing\sall\sregisters\sused\sby\sthe\nOP_SeekGE\sopcode.\s\sThis\sapplies\sto\sdebugging\sbuilds\sonly. -D 2022-10-07T15:55:35.220 +C Fix\sa\sproblem\scausing\sthe\sseek-scan\soptimization\sto\sskip\sover\svalid\srows\sthat\scould\soccur\swhen\sit\sis\sused\swith\sexpressions\sof\sthe\sform\s(a\sIN\s(?,?..)\sAND\sb\s>=\s?).\sdbsqlfuzz\sab1db6dc0efb04cba1cd3431ee6da4894fdc4520. +D 2022-10-07T18:57:15.710 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -660,7 +660,7 @@ F src/upsert.c 8789047a8f0a601ea42fa0256d1ba3190c13746b6ba940fe2d25643a7e991937 F src/utf.c ee39565f0843775cc2c81135751ddd93eceb91a673ea2c57f61c76f288b041a0 F src/util.c 0be191521ff6d2805995f4910f0b6231b42843678b2efdc1abecaf39929a673f F src/vacuum.c bb346170b0b54c6683bba4a5983aea40485597fdf605c87ec8bc2e199fe88cd8 -F src/vdbe.c 34846c9ef200eff7ed07155ce64bef751ee72dd631a497c7577350e7709583aa +F src/vdbe.c a26eef5a6c0ebb97f83ec18bd483e0d17f0c6b079e7650085f2ef6a82fd78d99 F src/vdbe.h 64619af62603dc3c4f5ff6ff6d2c8f389abd667a29ce6007ed44bd22b3211cd0 F src/vdbeInt.h 17b7461ffcf9ee760d1341731715a419f6b8c763089a7ece25c2e8098d702b3f F src/vdbeapi.c fc3183daf72808b4311b228989120fdbc2dc44972fb0d77d5c453460cc0e5b2c @@ -677,7 +677,7 @@ F src/wal.h c3aa7825bfa2fe0d85bef2db94655f99870a285778baa36307c0a16da32b226a F src/walker.c f890a3298418d7cba3b69b8803594fdc484ea241206a8dfa99db6dd36f8cbb3b F src/where.c 63e712bcad47f70e94c2150976cd7da5040933699e3938d4189d064acbe40891 F src/whereInt.h 70cd30de9ed784aa33fa6bd1245f060617de7a00d992469b6d8e419eed915743 -F src/wherecode.c 7ec238362d5fab3a67e3a77704f1cfdd2172f6da6f03065706922c114d8f39c3 +F src/wherecode.c bb88be457df3a6a01c844074ab79a9ba74279e73185f1362383aa697e3cae5dc F src/whereexpr.c 55a39f42aaf982574fbf52906371a84cceed98a994422198dfd03db4fce4cc46 F src/window.c 928e215840e2f2d9a2746e018c9643ef42c66c4ab6630ef0df7fa388fa145e86 F test/8_3_names.test ebbb5cd36741350040fd28b432ceadf495be25b2 @@ -1408,6 +1408,7 @@ F test/schema6.test e4bd1f23d368695eb9e7b51ef6e02ca0642ea2ab4a52579959826b5e7dce F test/schemafault.test 1936bceca55ac82c5efbcc9fc91a1933e45c8d1e1d106b9a7e56c972a5a2a51e F test/securedel.test 2f70b2449186a1921bd01ec9da407fbfa98c3a7a5521854c300c194b2ff09384 F test/securedel2.test 2d54c28e46eb1fd6902089958b20b1b056c6f1c5 +F test/seekscan1.test d79c97de5bb1dd1fd466687f3014add514fddf8248c57baf51d749c7dfd573d8 F test/select1.test 692e84cfa29c405854c69e8a4027183d64c22952866a123fabbce741a379e889 F test/select2.test 352480e0e9c66eda9c3044e412abdf5be0215b56 F test/select3.test 8d04b66df7475275a65f7e4a786d6a724c30bd9929f8ae5bd59c8d3d6e75e6cd @@ -2001,8 +2002,8 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P 8e14c351b29bb434ac4e2510681e95716424a73e340d39feff4919f0431c2e00 -R c90008f816e3229c412b35647f403e13 -U drh -Z 9f1360d8af6b1aae16a9d54a400e1988 +P 0aa6dee7f72279114a47cbb69fdbda0b916a2c365f7a147946869f167bd018b7 +R 9de0d5489b8b6fdeba7d984032a52e92 +U dan +Z eb03544a31e11a7d40cf257fa4d437b7 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 7fc252ac93..617778f89b 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -0aa6dee7f72279114a47cbb69fdbda0b916a2c365f7a147946869f167bd018b7 \ No newline at end of file +63d9efe277759d4daa29794846b60c6f55491496618f423f61468df72d0a4633 \ No newline at end of file diff --git a/src/vdbe.c b/src/vdbe.c index c2c829b1a5..1797aca006 100644 --- a/src/vdbe.c +++ b/src/vdbe.c @@ -4721,7 +4721,7 @@ seek_not_found: } -/* Opcode: SeekScan P1 P2 * * * +/* Opcode: SeekScan P1 P2 * * P5 ** Synopsis: Scan-ahead up to P1 rows ** ** This opcode is a prefix opcode to OP_SeekGE. In other words, this @@ -4731,8 +4731,8 @@ seek_not_found: ** This opcode uses the P1 through P4 operands of the subsequent ** OP_SeekGE. In the text that follows, the operands of the subsequent ** OP_SeekGE opcode are denoted as SeekOP.P1 through SeekOP.P4. Only -** the P1 and P2 operands of this opcode are also used, and are called -** This.P1 and This.P2. +** the P1, P2 and P5 operands of this opcode are also used, and are called +** This.P1, This.P2 and This.P5. ** ** This opcode helps to optimize IN operators on a multi-column index ** where the IN operator is on the later terms of the index by avoiding @@ -4742,29 +4742,51 @@ seek_not_found: ** ** The SeekGE.P3 and SeekGE.P4 operands identify an unpacked key which ** is the desired entry that we want the cursor SeekGE.P1 to be pointing -** to. Call this SeekGE.P4/P5 row the "target". +** to. Call this SeekGE.P3/P4 row the "target". ** ** If the SeekGE.P1 cursor is not currently pointing to a valid row, ** then this opcode is a no-op and control passes through into the OP_SeekGE. ** ** If the SeekGE.P1 cursor is pointing to a valid row, then that row ** might be the target row, or it might be near and slightly before the -** target row. This opcode attempts to position the cursor on the target -** row by, perhaps by invoking sqlite3BtreeStep() on the cursor -** between 0 and This.P1 times. -** -** There are three possible outcomes from this opcode:
    -** -**
  1. If after This.P1 steps, the cursor is still pointing to a place that -** is earlier in the btree than the target row, then fall through -** into the subsquence OP_SeekGE opcode. -** -**
  2. If the cursor is successfully moved to the target row by 0 or more -** sqlite3BtreeNext() calls, then jump to This.P2, which will land just -** past the OP_IdxGT or OP_IdxGE opcode that follows the OP_SeekGE. -** -**
  3. If the cursor ends up past the target row (indicating that the target -** row does not exist in the btree) then jump to SeekOP.P2. +** target row, or it might be after the target row. If the cursor is +** currently before the target row, then this opcode attempts to position +** the cursor on or after the target row by invoking sqlite3BtreeStep() +** on the cursor between 1 and This.P1 times. +** +** The This.P5 parameter is a flag that indicates what to do if the +** cursor ends up pointing at a valid row that is past the target +** row. If This.P5 is false (0) then a jump is made to SeekGE.P2. If +** This.P5 is true (non-zero) then a jump is made to This.P2. The P5==0 +** case occurs when there are no inequality constraints to the right of +** the IN constraing. The jump to SeekGE.P2 ends the loop. The P5!=0 case +** occurs when there are inequality constraints to the right of the IN +** operator. In that case, the This.P2 will point either directly to or +** to setup code prior to the OP_IdxGT or OP_IdxGE opcode that checks for +** loop terminate. +** +** Possible outcomes from this opcode:
      +** +**
    1. If the cursor is initally not pointed to any valid row, then +** fall through into the subsequent OP_SeekGE opcode. +** +**
    2. If the cursor is left pointing to a row that is before the target +** row, even after making as many as This.P1 calls to +** sqlite3BtreeNext(), then also fall through into OP_SeekGE. +** +**
    3. If the cursor is left pointing at the target row, either because it +** was at the target row to begin with or because one or more +** sqlite3BtreeNext() calls moved the cursor to the target row, +** then jump to This.P2.., +** +**
    4. If the cursor started out before the target row and a call to +** to sqlite3BtreeNext() moved the cursor off the end of the index +** (indicating that the target row definitely does not exist in the +** btree) then jump to SeekGE.P2, ending the loop. +** +**
    5. If the cursor ends up on a valid row that is past the target row +** (indicating that the target row does not exist in the btree) then +** jump to SeekOP.P2 if This.P5==0 or to This.P2 if This.P5>0. **
    */ case OP_SeekScan: { @@ -4775,14 +4797,25 @@ case OP_SeekScan: { assert( pOp[1].opcode==OP_SeekGE ); - /* pOp->p2 points to the first instruction past the OP_IdxGT that - ** follows the OP_SeekGE. */ + /* If pOp->p5 is clear, then pOp->p2 points to the first instruction past the + ** OP_IdxGT that follows the OP_SeekGE. Otherwise, it points to the first + ** opcode past the OP_SeekGE itself. */ assert( pOp->p2>=(int)(pOp-aOp)+2 ); - assert( aOp[pOp->p2-1].opcode==OP_IdxGT || aOp[pOp->p2-1].opcode==OP_IdxGE ); - testcase( aOp[pOp->p2-1].opcode==OP_IdxGE ); - assert( pOp[1].p1==aOp[pOp->p2-1].p1 ); - assert( pOp[1].p2==aOp[pOp->p2-1].p2 ); - assert( pOp[1].p3==aOp[pOp->p2-1].p3 ); +#ifdef SQLITE_DEBUG + if( pOp->p5==0 ){ + /* There are no inequality constraints following the IN constraint. */ + assert( pOp[1].p1==aOp[pOp->p2-1].p1 ); + assert( pOp[1].p2==aOp[pOp->p2-1].p2 ); + assert( pOp[1].p3==aOp[pOp->p2-1].p3 ); + assert( aOp[pOp->p2-1].opcode==OP_IdxGT + || aOp[pOp->p2-1].opcode==OP_IdxGE ); + testcase( aOp[pOp->p2-1].opcode==OP_IdxGE ); + }else{ + /* There are inequality constraints. */ + assert( pOp->p2==(int)(pOp-aOp)+2 ); + assert( aOp[pOp->p2-1].opcode==OP_SeekGE ); + } +#endif assert( pOp->p1>0 ); pC = p->apCsr[pOp[1].p1]; @@ -4816,8 +4849,9 @@ case OP_SeekScan: { while(1){ rc = sqlite3VdbeIdxKeyCompare(db, pC, &r, &res); if( rc ) goto abort_due_to_error; - if( res>0 ){ + if( res>0 && pOp->p5==0 ){ seekscan_search_fail: + /* Jump to SeekGE.P2, ending the loop */ #ifdef SQLITE_DEBUG if( db->flags&SQLITE_VdbeTrace ){ printf("... %d steps and then skip\n", pOp->p1 - nStep); @@ -4827,7 +4861,8 @@ case OP_SeekScan: { pOp++; goto jump_to_p2; } - if( res==0 ){ + if( res>=0 ){ + /* Jump to This.P2, bypassing the OP_SeekGE opcode */ #ifdef SQLITE_DEBUG if( db->flags&SQLITE_VdbeTrace ){ printf("... %d steps and then success\n", pOp->p1 - nStep); diff --git a/src/wherecode.c b/src/wherecode.c index 67df73c663..85d9be5011 100644 --- a/src/wherecode.c +++ b/src/wherecode.c @@ -2038,6 +2038,11 @@ Bitmask sqlite3WhereCodeOneLoopStart( ** guess. */ addrSeekScan = sqlite3VdbeAddOp1(v, OP_SeekScan, (pIdx->aiRowLogEst[0]+9)/10); + if( pRangeStart ){ + sqlite3VdbeChangeP5(v, 1); + sqlite3VdbeChangeP2(v, addrSeekScan, sqlite3VdbeCurrentAddr(v)+1); + addrSeekScan = 0; + } VdbeCoverage(v); } sqlite3VdbeAddOp4Int(v, op, iIdxCur, addrNxt, regBase, nConstraint); diff --git a/test/seekscan1.test b/test/seekscan1.test new file mode 100644 index 0000000000..e68d71449d --- /dev/null +++ b/test/seekscan1.test @@ -0,0 +1,63 @@ +# 2022 October 7 +# +# 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. +# +#*********************************************************************** +# This file implements regression tests for SQLite library. +# + +set testdir [file dirname $argv0] +source $testdir/tester.tcl +set testprefix seekscan1 + +do_execsql_test 1.0 { + CREATE TABLE t1(a TEXT, b INT, c INT NOT NULL, PRIMARY KEY(a,b,c)); + WITH RECURSIVE c(x) AS (VALUES(0) UNION ALL SELECT x+1 FROM c WHERE x<1997) + INSERT INTO t1(a,b,c) SELECT printf('xyz%d',x/10),x/6,x FROM c; + INSERT INTO t1 VALUES('abc',234,6); + INSERT INTO t1 VALUES('abc',345,7); + ANALYZE; +} + +do_execsql_test 1.1 { + SELECT a,b,c FROM t1 + WHERE b IN (234, 345) AND c BETWEEN 6 AND 6.5 AND a='abc' + ORDER BY a, b; +} { + abc 234 6 +} + +do_execsql_test 1.2 { + SELECT a,b,c FROM t1 + WHERE b IN (234, 345) AND c BETWEEN 6 AND 7 AND a='abc' + ORDER BY a, b; +} { + abc 234 6 + abc 345 7 +} + +do_execsql_test 1.3 { + SELECT a,b,c FROM t1 + WHERE b IN (234, 345) AND c >=6 AND a='abc' + ORDER BY a, b; +} { + abc 234 6 + abc 345 7 +} + +do_execsql_test 1.4 { + SELECT a,b,c FROM t1 + WHERE b IN (234, 345) AND c<=7 AND a='abc' + ORDER BY a, b; +} { + abc 234 6 + abc 345 7 +} + + +finish_test