From: drh <> Date: Fri, 16 Aug 2024 11:26:21 +0000 (+0000) Subject: Fix handling of COLLATE. Add test cases for the same. Code cleanup for X-Git-Tag: version-3.47.0~226^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fheads%2Forder-by-subquery;p=thirdparty%2Fsqlite.git Fix handling of COLLATE. Add test cases for the same. Code cleanup for improved understandability and maintainability. FossilOrigin-Name: 41a41c173a9d15d94f23d73a5c04bfb1616cb9223bc81d41808f9b4d00817fbf --- diff --git a/manifest b/manifest index b8037df583..3d86a9c04b 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Bug\sfix\sin\sthe\ssubquery\sORDER\sBY\spropagator. -D 2024-08-16T02:19:17.392 +C Fix\shandling\sof\sCOLLATE.\s\sAdd\stest\scases\sfor\sthe\ssame.\s\sCode\scleanup\sfor\nimproved\sunderstandability\sand\smaintainability. +D 2024-08-16T11:26:21.307 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -846,7 +846,7 @@ F src/vxworks.h d2988f4e5a61a4dfe82c6524dd3d6e4f2ce3cdb9 F src/wal.c 887fc4ca3f020ebb2e376f222069570834ac63bf50111ef0cbf3ae417048ed89 F src/wal.h ba252daaa94f889f4b2c17c027e823d9be47ce39da1d3799886bbd51f0490452 F src/walker.c 7c7ea0115345851c3da4e04e2e239a29983b61fb5b038b94eede6aba462640e2 -F src/where.c 154533a45da7b54f460d7415b0ea3aea59855a58460cf1356175b0045ff30c26 +F src/where.c f5be664f3379c9f930696e339ec4ef4c1187af860cca727411156101fae6b677 F src/whereInt.h 6444b888ce395cb80511284b8a73b63472d34247fcb1b125ee06a54fa6ae878e F src/wherecode.c c9cac0b0b8e809c5e7e79d7796918907fb685ad99be2aaa9737f9787aa47349c F src/whereexpr.c 7d0d34b42b9edfd8e8ca66beb3a6ef63fe211c001af54caf2ccbcd989b783290 @@ -1493,6 +1493,7 @@ F test/orderby7.test 3d1383d52ade5b9eb3a173b3147fdd296f0202da F test/orderby8.test 23ef1a5d72bd3adcc2f65561c654295d1b8047bd F test/orderby9.test 87fb9548debcc2cd141c5299002dd94672fa76a3 F test/orderbyA.test df608e59efc2ef50c1eddf1a773b272de3252e9401bfec86d04b52fd973866d5 +F test/orderbyB.test 32576c7b138105bc72f7fbf33bd320ca3a7d303641fc939e0e56af6cba884b3d F test/oserror.test 1fc9746b83d778e70d115049747ba19c7fba154afce7cc165b09feb6ca6abbc5 F test/ossfuzz.c 9636dad2092a05a32110df0ca06713038dd0c43dd89a77dabe4b8b0d71096715 F test/ossshell.c f125c5bd16e537a2549aa579b328dd1c59905e7ab1338dfc210e755bb7b69f17 @@ -1706,7 +1707,7 @@ F test/temptable.test d2c9b87a54147161bcd1822e30c1d1cd891e5b30 F test/temptable2.test 76821347810ecc88203e6ef0dd6897b6036ac788e9dd3e6b04fd4d1631311a16 F test/temptable3.test d11a0974e52b347e45ee54ef1923c91ed91e4637 F test/temptrigger.test 38f0ca479b1822d3117069e014daabcaacefffcc -F test/tester.tcl 640106bf8f7785d0ac67cda2837577eb9f2d936033bacedf9e705ca5451958ef +F test/tester.tcl e88c498c369cff6bf0898db6d04088685066730be51821ef775ef13fd2b1d077 F test/testrunner.tcl 5d02deeba7a53baeadae6aa7641d90aac58fdfa3a7bcac85cfcfd752b1aab87c F test/testrunner_data.tcl c5ae2b1f9a99210b0600d002fb3af1fee350997cee9416551e83b93501360ebf F test/thread001.test a0985c117eab62c0c65526e9fa5d1360dd1cac5b03bde223902763274ce21899 @@ -2204,8 +2205,8 @@ F vsixtest/vsixtest.tcl 6195aba1f12a5e10efc2b8c0009532167be5e301abe5b31385638080 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P b82421e3f5811946e9d60b845fc882b6ea5c53c648695654c6900754427bf9bf -R 41797d101dab015774684ce7d95aeefd +P 5a9a3b8af7ac0aa1c04ad2d735e341c92d67952acb9a1d30217c0471e92cd468 +R 7950e16a6ca37805979b1472a80cc190 U drh -Z c79ab6ee6fe9513aaffefd02f00f8786 +Z 14a0d323dd6fe70b6ed40930f5fa77fd # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 77af5ed578..b684817cd4 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -5a9a3b8af7ac0aa1c04ad2d735e341c92d67952acb9a1d30217c0471e92cd468 +41a41c173a9d15d94f23d73a5c04bfb1616cb9223bc81d41808f9b4d00817fbf diff --git a/src/where.c b/src/where.c index 442904a654..631b7581fb 100644 --- a/src/where.c +++ b/src/where.c @@ -4847,7 +4847,8 @@ static int whereLoopAddAll(WhereLoopBuilder *pBuilder){ return rc; } -/* +/* Implementation of the order-by-subquery optimization: +** ** WhereLoop pLoop, which the iLoop-th term of the nested loop, is really ** a subquery or CTE that has an ORDER BY clause. See if any of the terms ** in the subquery ORDER BY clause will satisfy pOrderBy from the outer @@ -4862,43 +4863,67 @@ static int whereLoopAddAll(WhereLoopBuilder *pBuilder){ ** SELECT * FROM t3 JOIN t1 ON a=q ORDER BY p, b; ** ** The CTE named "t3" comes out in the natural order of "p", so the first -** first them of "ORDER BY p,b" is satisfied by a sequential scan of "t3". -** +** first them of "ORDER BY p,b" is satisfied by a sequential scan of "t3" +** and sorting only needs to occur on the second term "b". +** +** Limitations: +** +** (1) The optimization is not applied if the outer ORDER BY contains +** a COLLATE clause. The optimization might be applied if the +** outer ORDER BY uses NULLS FIRST, NULLS LAST, ASC, and/or DESC as +** long as the subquery ORDER BY does the same. But if the +** outer ORDER BY uses COLLATE, even a redundant COLLATE, the +** optimization is bypassed. +** +** (2) The subquery ORDER BY terms must exactly match subquery result +** columns, including any COLLATE annotations. This routine relies +** on iOrderByCol to do matching between order by terms and result +** columns, and iOrderByCol will not be set if the result column +** and ORDER BY collations differ. +** +** (3) The subquery and outer ORDER BY can be in opposite directions as +** long as the subquery is materialized. If the subquery is +** implemented as a co-routine, the sort orders must be in the same +** direction because there is no way to run a co-routine backwards. */ static SQLITE_NOINLINE int wherePathMatchSubqueryOB( + WhereInfo *pWInfo, /* The WHERE clause */ WhereLoop *pLoop, /* The nested loop term that is a subquery */ int iLoop, /* Which level of the nested loop. 0==outermost */ int iCur, /* Cursor used by the this loop */ - int wctrlFlags, /* Flags determining sort behavior */ ExprList *pOrderBy, /* The ORDER BY clause on the whole query */ Bitmask *pRevMask, /* When loops need to go in reverse order */ Bitmask *pOBSat /* Which terms of pOrderBy are satisfied so far */ ){ - int i, j; - u8 rev = 0; - u8 revIdx = 0; - Expr *pOBExpr; - ExprList *pSubOB = pLoop->u.btree.pOrderBy; + int iOB; /* Index into pOrderBy->a[] */ + int jSub; /* Index into pSubOB->a[] */ + u8 rev = 0; /* True if iOB and jSub sort in opposite directions */ + u8 revIdx = 0; /* Sort direction for jSub */ + Expr *pOBExpr; /* Current term of outer ORDER BY */ + ExprList *pSubOB; /* Complete ORDER BY on the subquery */ + + pSubOB = pLoop->u.btree.pOrderBy; assert( pSubOB!=0 ); - for(i=0; (MASKBIT(i) & *pOBSat)!=0; i++){} - for(j=0; jnExpr && inExpr; j++, i++){ - if( pSubOB->a[j].u.x.iOrderByCol==0 ) break; - pOBExpr = sqlite3ExprSkipCollateAndLikely(pOrderBy->a[j].pExpr); + for(iOB=0; (MASKBIT(iOB) & *pOBSat)!=0; iOB++){} + for(jSub=0; jSubnExpr && iOBnExpr; jSub++, iOB++){ + if( pSubOB->a[jSub].u.x.iOrderByCol==0 ) break; + pOBExpr = pOrderBy->a[iOB].pExpr; if( pOBExpr->op!=TK_COLUMN && pOBExpr->op!=TK_AGG_COLUMN ) break; if( pOBExpr->iTable!=iCur ) break; - if( pOBExpr->iColumn!=pSubOB->a[j].u.x.iOrderByCol-1 ) break; - if( (wctrlFlags & WHERE_GROUPBY)==0 ){ - if( (pSubOB->a[j].fg.sortFlags & KEYINFO_ORDER_BIGNULL) - != (pOrderBy->a[j].fg.sortFlags & KEYINFO_ORDER_BIGNULL) ){ + if( pOBExpr->iColumn!=pSubOB->a[jSub].u.x.iOrderByCol-1 ) break; + if( (pWInfo->wctrlFlags & WHERE_GROUPBY)==0 ){ + u8 sfOB = pOrderBy->a[iOB].fg.sortFlags; /* sortFlags for iOB */ + u8 sfSub = pSubOB->a[jSub].fg.sortFlags; /* sortFlags for jSub */ + if( (sfSub & KEYINFO_ORDER_BIGNULL) != (sfOB & KEYINFO_ORDER_BIGNULL) ){ break; } - revIdx = pSubOB->a[j].fg.sortFlags & KEYINFO_ORDER_DESC; - if( j>0 ){ - if( (rev ^ revIdx)!=(pOrderBy->a[i].fg.sortFlags&KEYINFO_ORDER_DESC) ){ + revIdx = sfSub & KEYINFO_ORDER_DESC; + if( jSub>0 ){ + if( (rev^revIdx)!=(sfOB & KEYINFO_ORDER_DESC) ){ break; } }else{ - rev = revIdx ^ (pOrderBy->a[i].fg.sortFlags & KEYINFO_ORDER_DESC); + rev = revIdx ^ (sfOB & KEYINFO_ORDER_DESC); if( rev ){ if( (pLoop->wsFlags & WHERE_COROUTINE)!=0 ){ /* Cannot run a co-routine in reverse order */ @@ -4908,9 +4933,9 @@ static SQLITE_NOINLINE int wherePathMatchSubqueryOB( } } } - *pOBSat |= MASKBIT(i); + *pOBSat |= MASKBIT(iOB); } - return j>0; + return jSub>0; } /* @@ -5060,7 +5085,7 @@ static i8 wherePathSatisfiesOrderBy( if( pLoop->wsFlags & WHERE_IPK ){ if( pLoop->u.btree.pOrderBy && OptimizationEnabled(db, SQLITE_OrderBySubq) - && wherePathMatchSubqueryOB(pLoop,iLoop,iCur,wctrlFlags, + && wherePathMatchSubqueryOB(pWInfo,pLoop,iLoop,iCur, pOrderBy,pRevMask, &obSat) ){ nColumn = 0; diff --git a/test/orderbyB.test b/test/orderbyB.test new file mode 100644 index 0000000000..42d2de7982 --- /dev/null +++ b/test/orderbyB.test @@ -0,0 +1,94 @@ +# 2024-08-15 +# +# 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. +# +# Specifically, it tests cases with order-by-subquery optimization in which +# an ORDER BY in a subquery is used to help resolve an ORDER BY in the +# outer query without having to do an extra sort. +# + +set testdir [file dirname $argv0] +source $testdir/tester.tcl +set ::testprefix orderbyb + +db null NULL +do_execsql_test 1.0 { + CREATE TABLE t1(a TEXT, b TEXT, c INT); + INSERT INTO t1 VALUES(NULL,NULL,NULL); + WITH RECURSIVE c(n) AS (VALUES(1) UNION ALL SELECT n+1 FROM c WHERE n<7) + INSERT INTO t1(a,b,c) SELECT char(p,p), char(q,q), n FROM + (SELECT ((n-1)%4)+0x61 AS p, abs(n*2-9+(n>=5))+0x60 AS q, n FROM c); + UPDATE t1 SET b=upper(b) WHERE c=1; + CREATE TABLE t2(k TEXT PRIMARY KEY, v INT) WITHOUT ROWID; + WITH RECURSIVE c(n) AS (VALUES(1) UNION ALL SELECT n+1 FROM c WHERE n<7) + INSERT INTO t2(k,v) SELECT char(0x60+n,0x60+n), n FROM c; + WITH RECURSIVE c(n) AS (VALUES(1) UNION ALL SELECT n+1 FROM c WHERE n<7) + INSERT INTO t2(k,v) SELECT char(0x40+n,0x40+n), n FROM c; + SELECT a,b,c,tx.v AS 'v-a', ty.v AS 'v-b' + FROM t1 LEFT JOIN t2 AS tx ON tx.k=a + LEFT JOIN t2 AS ty ON ty.k=b + ORDER BY c; +} { + NULL NULL NULL NULL NULL + aa GG 1 1 7 + bb ee 2 2 5 + cc cc 3 3 3 + dd aa 4 4 1 + aa bb 5 1 2 + bb dd 6 2 4 + cc ff 7 3 6 +} + +do_eqp_execsql_test 1.1 { + WITH t3(x,y) AS (SELECT a, b FROM t1 ORDER BY a, b LIMIT 8) + SELECT x, y, v FROM t3 LEFT JOIN t2 ON k=t3.y ORDER BY x, y COLLATE nocase; +} { + QUERY PLAN + |--CO-ROUTINE t3 + | |--SCAN t1 + | `--USE TEMP B-TREE FOR ORDER BY + |--SCAN t3 + |--SEARCH t2 USING PRIMARY KEY (k=?) LEFT-JOIN + `--USE TEMP B-TREE FOR LAST TERM OF ORDER BY +} { + NULL NULL NULL + aa bb 2 + aa GG 7 + bb dd 4 + bb ee 5 + cc cc 3 + cc ff 6 + dd aa 1 +} + +do_eqp_execsql_test 1.2 { + WITH t3(x,y) AS MATERIALIZED (SELECT a, b COLLATE nocase FROM t1 ORDER BY 1,2) + SELECT x, y, v FROM t3 LEFT JOIN t2 ON k=t3.y ORDER BY x,y; +} { + QUERY PLAN + |--MATERIALIZE t3 + | |--SCAN t1 + | `--USE TEMP B-TREE FOR ORDER BY + |--SCAN t3 + `--SEARCH t2 USING PRIMARY KEY (k=?) LEFT-JOIN +} { + NULL NULL NULL + aa bb 2 + aa GG 7 + bb dd 4 + bb ee 5 + cc cc 3 + cc ff 6 + dd aa 1 +} + + +finish_test diff --git a/test/tester.tcl b/test/tester.tcl index 63c83187aa..43f7c7a047 100644 --- a/test/tester.tcl +++ b/test/tester.tcl @@ -1056,6 +1056,29 @@ proc do_eqp_test {name sql res} { } } +# Do both an eqp_test and an execsql_test on the same SQL. +# +proc do_eqp_execsql_test {name sql res1 res2} { + if {[regexp {^\s+QUERY PLAN\n} $res1]} { + + set query_plan [query_plan_graph $sql] + + if {[list {*}$query_plan]==[list {*}$res1]} { + uplevel [list do_test ${name}a [list set {} ok] ok] + } else { + uplevel [list \ + do_test ${name}a [list query_plan_graph $sql] $res1 + ] + } + } else { + if {[string index $res 0]!="/"} { + set res1 "/*$res1*/" + } + uplevel do_execsql_test ${name}a [list "EXPLAIN QUERY PLAN $sql"] [list $res1] + } + uplevel do_execsql_test ${name}b [list $sql] [list $res2] +} + #------------------------------------------------------------------------- # Usage: do_select_tests PREFIX ?SWITCHES? TESTLIST