diff --git a/manifest b/manifest index 3dc920122d..4f79804d82 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Improvements\sto\sa\scomment\sin\slempar.c.\s\sNo\scode\schanges. -D 2017-10-02T02:32:12.306 +C The\squery\splanner\snow\salways\sprefers\sto\simplement\sa\sFROM-clause\ssubquery\susing\na\sco-routine\srather\sthan\sflattening\sthe\ssubquery\sinto\sthe\souter\squery. +D 2017-10-02T02:52:54.153 F Makefile.in 4bc36d913c2e3e2d326d588d72f618ac9788b2fd4b7efda61102611a6495c3ff F Makefile.linux-gcc 7bc79876b875010e8c8f9502eb935ca92aa3c434 F Makefile.msc 6033b51b6aea702ea059f6ab2d47b1d3cef648695f787247dd4fb395fe60673f @@ -411,7 +411,7 @@ F src/ctime.c ff1be3eed7bdd75aaca61ca8dc848f7c9f850ef2fb9cb56f2734e922a098f9c0 F src/date.c 48f743d88bbe88f848532d333cca84f26e52a4f217e86f86be7fc1b919c33d74 F src/dbstat.c 7a4ba8518b6369ef3600c49cf9c918ad979acba610b2aebef1b656d649b96720 F src/delete.c 21a5f1812fdb599e9f7afb9f650bdabab60a3afd51d7e94e539c982f647b0023 -F src/expr.c 628395aea8bbf4e2851565f9dab76565e3100ca0836a337fb748834c9cec9f04 +F src/expr.c 4d2d0aafd945424f638ee03e11330f03288ccf616e025498f3c8602d01609a0a F src/fault.c 460f3e55994363812d9d60844b2a6de88826e007 F src/fkey.c 5ff2c895fe087756d8085dc1a9bc229b5670e2a65c3929dd87c71e43649af333 F src/func.c b4d259af627e3cd9510cf08db37f0bcc88b1887c735169c74490c3739d5cf5c6 @@ -458,18 +458,18 @@ F src/printf.c 40aee47ae9be4bd3dbdc8968bd07fddc027be8edec8daddf24d3391d36698a1c F src/random.c 80f5d666f23feb3e6665a6ce04c7197212a88384 F src/resolve.c 4324a94573b1e29286f8121e4881db59eaedc014afeb274c8d3e07ed282e0e20 F src/rowset.c 7b7e7e479212e65b723bf40128c7b36dc5afdfac -F src/select.c 733e7f727d4f83234983fcbcb71670229f6d9ae78ee724fbf40b4aa6904474e2 +F src/select.c 19ebf10e33de8e1f327cbc844aff12af74bdf0513b3eb1b712b7bcdca4c9bdb5 F src/shell.c c1206a23d9239f8f51751d3be9b8c3b02fa4103546bea1add7f864d84a8276ab F src/shell.c.in bb9720a8c5c98d3984b16ab7540e7142bcae959666ecf248bfc523a1d44220ee F src/sqlite.h.in ab4f8a29d1580dfaeb6891fa1b83cff8229ba0daa56994707ceaca71495d9ab7 F src/sqlite3.rc 5121c9e10c3964d5755191c80dd1180c122fc3a8 F src/sqlite3ext.h a1fd3aa82f967da436164e0728a7d6841651fd0c6e27b9044e0eb9f6c8462e47 -F src/sqliteInt.h 954fed875c59d283870f13010bbbb96d8b25cf3db0683843403c36e9dfcf1bdd +F src/sqliteInt.h 99e4beebd466495434ca6ed94bd7966a7f8c5879cd1f01768c588a5a2acd84c2 F src/sqliteLimit.h 1513bfb7b20378aa0041e7022d04acb73525de35b80b252f1b83fedb4de6a76b F src/status.c 9737ed017279a9e0c5da748701c3c7bf1e8ae0dae459aad20dd64fcff97a7e35 F src/table.c b46ad567748f24a326d9de40e5b9659f96ffff34 F src/tclsqlite.c 487951d81f9704800fd9f0ffdaa2f935a83ccb6be3575c2c4ef83e4789b4c828 -F src/test1.c a947b2554fa77d0ef2dd21d1ef08e37e5d91b17af83de923a4e3c7f10957a2eb +F src/test1.c 8ef15f7a357f85dfc41c6c748ce9c947b4f676e01bb5ae6a45bee4923dff8b51 F src/test2.c 3efb99ab7f1fc8d154933e02ae1378bac9637da5 F src/test3.c b8434949dfb8aff8dfa082c8b592109e77844c2135ed3c492113839b6956255b F src/test4.c 18ec393bb4d0ad1de729f0b94da7267270f3d8e6 @@ -911,7 +911,7 @@ F test/fuzzer2.test a85ef814ce071293bce1ad8dffa217cbbaad4c14 F test/fuzzerfault.test 8792cd77fd5bce765b05d0c8e01b9edcf8af8536 F test/gcfault.test dd28c228a38976d6336a3fc42d7e5f1ad060cb8c F test/genesis.tcl 1e2e2e8e5cc4058549a154ff1892fe5c9de19f98 -F test/having.test b3d6b17cc9601b6b373b2d0f08c075ccf30e2d307249c3c8a236e3c36907b1a5 +F test/having.test e4098a4b8962f9596035c3b87a8928a10648acc509f1bb8d6f96413bbf79a1b3 F test/hexlit.test 4a6a5f46e3c65c4bf1fa06f5dd5a9507a5627751 F test/hidden.test 23c1393a79e846d68fd902d72c85d5e5dcf98711 F test/hook.test dbc0b87756e1e20e7497b56889c9e9cd2f8cc2b5 @@ -1655,7 +1655,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 0840f9f824c16212ce3fd6c859e501176eb0a58924ea1728a54d5bdfd0c25c86 -R 7f732cb0e00eb0e6e46dbdc3b44db828 +P 8000d230171fe53599fc6a916666f60f88f789e6c9c6c2869514c02d837d6fc8 928486231ff7cc0df17a5aa23a080a1b8720e168db5227a7dbd34861283ba689 +R d80162b36b4fccf5799907b67fd02d3c +T +closed 928486231ff7cc0df17a5aa23a080a1b8720e168db5227a7dbd34861283ba689 U drh -Z bc9e28c24d7165521312a05b1d8d5118 +Z dc6d533f9eb613034e3c381c85c3f10b diff --git a/manifest.uuid b/manifest.uuid index 254a13150d..9d275448b7 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -8000d230171fe53599fc6a916666f60f88f789e6c9c6c2869514c02d837d6fc8 \ No newline at end of file +c9104b59c7ed360291f7f6fc8caae938e9840c77620d598e4096f78183bf807a \ No newline at end of file diff --git a/src/expr.c b/src/expr.c index 8282c99a4a..0b922cd0fd 100644 --- a/src/expr.c +++ b/src/expr.c @@ -1712,12 +1712,11 @@ void sqlite3ExprListDelete(sqlite3 *db, ExprList *pList){ u32 sqlite3ExprListFlags(const ExprList *pList){ int i; u32 m = 0; - if( pList ){ - for(i=0; inExpr; i++){ - Expr *pExpr = pList->a[i].pExpr; - assert( pExpr!=0 ); - m |= pExpr->flags; - } + assert( pList!=0 ); + for(i=0; inExpr; i++){ + Expr *pExpr = pList->a[i].pExpr; + assert( pExpr!=0 ); + m |= pExpr->flags; } return m; } diff --git a/src/select.c b/src/select.c index edf55a5c7b..b7c459011c 100644 --- a/src/select.c +++ b/src/select.c @@ -3298,9 +3298,11 @@ static void substSelect( ** ** Flattening is subject to the following constraints: ** -** (1) The subquery and the outer query cannot both be aggregates. +** (**) We no longer attempt to flatten aggregate subqueries. Was: +** The subquery and the outer query cannot both be aggregates. ** -** (2) If the subquery is an aggregate then +** (**) We no longer attempt to flatten aggregate subqueries. Was: +** (2) If the subquery is an aggregate then ** (2a) the outer query must not be a join and ** (2b) the outer query must not use subqueries ** other than the one FROM-clause subquery that is a candidate @@ -3319,7 +3321,8 @@ static void substSelect( ** sub-queries that were excluded from this optimization. Restriction ** (4) has since been expanded to exclude all DISTINCT subqueries. ** -** (6) If the subquery is aggregate, the outer query may not be DISTINCT. +** (**) We no longer attempt to flatten aggregate subqueries. Was: +** If the subquery is aggregate, the outer query may not be DISTINCT. ** ** (7) The subquery must have a FROM clause. TODO: For subqueries without ** A FROM clause, consider adding a FROM clause with the special @@ -3398,7 +3401,8 @@ static void substSelect( ** parent to a compound query confuses the code that handles ** recursive queries in multiSelect(). ** -** (24) The subquery may not be an aggregate that uses the built-in min() or +** (**) We no longer attempt to flatten aggregate subqueries. Was: +** The subquery may not be an aggregate that uses the built-in min() or ** or max() functions. (Without this restriction, a query like: ** "SELECT x FROM (SELECT max(y), x FROM t1)" would not necessarily ** return the value X for which Y was maximal.) @@ -3406,7 +3410,7 @@ static void substSelect( ** ** In this routine, the "p" parameter is a pointer to the outer query. ** The subquery is p->pSrc->a[iFrom]. isAgg is true if the outer query -** uses aggregates and subqueryIsAgg is true if the subquery uses aggregates. +** uses aggregates. ** ** If flattening is not attempted, this routine is a no-op and returns 0. ** If flattening is attempted this routine returns 1. @@ -3418,8 +3422,7 @@ static int flattenSubquery( Parse *pParse, /* Parsing context */ Select *p, /* The parent or outer SELECT statement */ int iFrom, /* Index in p->pSrc->a[] of the inner subquery */ - int isAgg, /* True if outer SELECT uses aggregate functions */ - int subqueryIsAgg /* True if the subquery uses aggregate functions */ + int isAgg /* True if outer SELECT uses aggregate functions */ ){ const char *zSavedAuthContext = pParse->zAuthContext; Select *pParent; /* Current UNION ALL term of the other query */ @@ -3446,16 +3449,6 @@ static int flattenSubquery( iParent = pSubitem->iCursor; pSub = pSubitem->pSelect; assert( pSub!=0 ); - if( subqueryIsAgg ){ - if( isAgg ) return 0; /* Restriction (1) */ - if( pSrc->nSrc>1 ) return 0; /* Restriction (2a) */ - if( (p->pWhere && ExprHasProperty(p->pWhere,EP_Subquery)) - || (sqlite3ExprListFlags(p->pEList) & EP_Subquery)!=0 - || (sqlite3ExprListFlags(p->pOrderBy) & EP_Subquery)!=0 - ){ - return 0; /* Restriction (2b) */ - } - } pSubSrc = pSub->pSrc; assert( pSubSrc ); @@ -3474,9 +3467,6 @@ static int flattenSubquery( if( pSub->pLimit && (pSrc->nSrc>1 || isAgg) ){ return 0; /* Restrictions (8)(9) */ } - if( (p->selFlags & SF_Distinct)!=0 && subqueryIsAgg ){ - return 0; /* Restriction (6) */ - } if( p->pOrderBy && pSub->pOrderBy ){ return 0; /* Restriction (11) */ } @@ -3485,10 +3475,8 @@ static int flattenSubquery( if( pSub->pLimit && (p->selFlags & SF_Distinct)!=0 ){ return 0; /* Restriction (21) */ } - testcase( pSub->selFlags & SF_Recursive ); - testcase( pSub->selFlags & SF_MinMaxAgg ); - if( pSub->selFlags & (SF_Recursive|SF_MinMaxAgg) ){ - return 0; /* Restrictions (22) and (24) */ + if( pSub->selFlags & (SF_Recursive) ){ + return 0; /* Restrictions (22) */ } if( (p->selFlags & SF_Recursive) && pSub->pPrior ){ return 0; /* Restriction (23) */ @@ -3778,18 +3766,7 @@ static int flattenSubquery( if( isLeftJoin>0 ){ setJoinExpr(pWhere, iNewParent); } - if( subqueryIsAgg ){ - assert( pParent->pHaving==0 ); - pParent->pHaving = pParent->pWhere; - pParent->pWhere = pWhere; - pParent->pHaving = sqlite3ExprAnd(db, - sqlite3ExprDup(db, pSub->pHaving, 0), pParent->pHaving - ); - assert( pParent->pGroupBy==0 ); - pParent->pGroupBy = sqlite3ExprListDup(db, pSub->pGroupBy, 0); - }else{ - pParent->pWhere = sqlite3ExprAnd(db, pWhere, pParent->pWhere); - } + pParent->pWhere = sqlite3ExprAnd(db, pWhere, pParent->pWhere); if( db->mallocFailed==0 ){ SubstContext x; x.pParse = pParse; @@ -3852,9 +3829,13 @@ static int flattenSubquery( ** ** Do not attempt this optimization if: ** -** (1) The inner query is an aggregate. (In that case, we'd really want -** to copy the outer WHERE-clause terms onto the HAVING clause of the -** inner query. But they probably won't help there so do not bother.) +** (1) (** This restriction was removed on 2017-09-29. We used to +** disallow this optimization for aggregate subqueries, but now +** it is allowed by putting the extra terms on the HAVING clause. +** The added HAVING clause is pointless if the subquery lacks +** a GROUP BY clause. But such a HAVING clause is also harmless +** so there does not appear to be any reason to add extra logic +** to suppress it. **) ** ** (2) The inner query is the recursive part of a common table expression. ** @@ -3879,16 +3860,22 @@ static int pushDownWhereTerms( ){ Expr *pNew; int nChng = 0; - Select *pX; /* For looping over compound SELECTs in pSubq */ if( pWhere==0 ) return 0; - for(pX=pSubq; pX; pX=pX->pPrior){ - if( (pX->selFlags & (SF_Aggregate|SF_Recursive))!=0 ){ - testcase( pX->selFlags & SF_Aggregate ); - testcase( pX->selFlags & SF_Recursive ); - testcase( pX!=pSubq ); - return 0; /* restrictions (1) and (2) */ + if( pSubq->selFlags & SF_Recursive ) return 0; /* restriction (2) */ + +#ifdef SQLITE_DEBUG + /* Only the first term of a compound can have a WITH clause. But make + ** sure no other terms are marked SF_Recursive in case something changes + ** in the future. + */ + { + Select *pX; + for(pX=pSubq; pX; pX=pX->pPrior){ + assert( (pX->selFlags & (SF_Recursive))==0 ); } } +#endif + if( pSubq->pLimit!=0 ){ return 0; /* restriction (3) */ } @@ -3896,7 +3883,7 @@ static int pushDownWhereTerms( nChng += pushDownWhereTerms(pParse, pSubq, pWhere->pRight, iCursor); pWhere = pWhere->pLeft; } - if( ExprHasProperty(pWhere,EP_FromJoin) ) return 0; /* restriction 5 */ + if( ExprHasProperty(pWhere,EP_FromJoin) ) return 0; /* restriction (5) */ if( sqlite3ExprIsTableConstant(pWhere, iCursor) ){ nChng++; while( pSubq ){ @@ -3908,7 +3895,11 @@ static int pushDownWhereTerms( x.isLeftJoin = 0; x.pEList = pSubq->pEList; pNew = substExpr(&x, pNew); - pSubq->pWhere = sqlite3ExprAnd(pParse->db, pSubq->pWhere, pNew); + if( pSubq->selFlags & SF_Aggregate ){ + pSubq->pHaving = sqlite3ExprAnd(pParse->db, pSubq->pHaving, pNew); + }else{ + pSubq->pWhere = sqlite3ExprAnd(pParse->db, pSubq->pWhere, pNew); + } pSubq = pSubq->pPrior; } } @@ -5202,7 +5193,6 @@ int sqlite3Select( for(i=0; !p->pPrior && inSrc; i++){ struct SrcList_item *pItem = &pTabList->a[i]; Select *pSub = pItem->pSelect; - int isAggSub; Table *pTab = pItem->pTab; if( pSub==0 ) continue; @@ -5214,13 +5204,36 @@ int sqlite3Select( goto select_end; } - isAggSub = (pSub->selFlags & SF_Aggregate)!=0; - if( flattenSubquery(pParse, p, i, isAgg, isAggSub) ){ + /* Do not try to flatten an aggregate subquery. + ** + ** Flattening an aggregate subquery is only possible if the outer query + ** is not a join. But if the outer query is not a join, then the subquery + ** will be implemented as a co-routine and there is no advantage to + ** flattening in that case. + */ + if( (pSub->selFlags & SF_Aggregate)!=0 ) continue; + assert( pSub->pGroupBy==0 ); + + /* If the subquery contains an ORDER BY clause and if + ** it will be implemented as a co-routine, then do not flatten. This + ** restriction allows SQL constructs like this: + ** + ** SELECT expensive_function(x) + ** FROM (SELECT x FROM tab ORDER BY y LIMIT 10); + ** + ** The expensive_function() is only computed on the 10 rows that + ** are output, rather than every row of the table. + */ + if( pSub->pOrderBy!=0 + && i==0 + && (pTabList->nSrc==1 + || (pTabList->a[1].fg.jointype&(JT_LEFT|JT_CROSS))!=0) + ){ + continue; + } + + if( flattenSubquery(pParse, p, i, isAgg) ){ /* This subquery can be absorbed into its parent. */ - if( isAggSub ){ - isAgg = 1; - p->selFlags |= SF_Aggregate; - } i = -1; } pTabList = p->pSrc; @@ -5329,12 +5342,9 @@ int sqlite3Select( /* Generate code to implement the subquery ** - ** The subquery is implemented as a co-routine if all of these are true: - ** (1) The subquery is guaranteed to be the outer loop (so that it - ** does not need to be computed more than once) - ** (2) REMOVED (2017-09-28): The ALL keyword after SELECT is omitted. - ** (3) Co-routines are not disabled using sqlite3_test_control() - ** with SQLITE_TESTCTRL_OPTIMIZATIONS. + ** The subquery is implemented as a co-routine if the subquery is + ** guaranteed to be the outer loop (so that it does not need to be + ** computed more than once) ** ** TODO: Are there other reasons beside (1) to use a co-routine ** implementation? @@ -5342,8 +5352,6 @@ int sqlite3Select( if( i==0 && (pTabList->nSrc==1 || (pTabList->a[1].fg.jointype&(JT_LEFT|JT_CROSS))!=0) /* (1) */ - /*** constraint removed: && (p->selFlags & SF_All)==0 (2) */ - && OptimizationEnabled(db, SQLITE_SubqCoroutine) /* (3) */ ){ /* Implement a co-routine that will return a single row of the result ** set on each invocation. diff --git a/src/sqliteInt.h b/src/sqliteInt.h index 3259df894f..99c8b73748 100644 --- a/src/sqliteInt.h +++ b/src/sqliteInt.h @@ -1506,16 +1506,15 @@ struct sqlite3 { #define SQLITE_ColumnCache 0x0002 /* Column cache */ #define SQLITE_GroupByOrder 0x0004 /* GROUPBY cover of ORDERBY */ #define SQLITE_FactorOutConst 0x0008 /* Constant factoring */ -/* not used 0x0010 // Was: SQLITE_IdxRealAsInt */ -#define SQLITE_DistinctOpt 0x0020 /* DISTINCT using indexes */ -#define SQLITE_CoverIdxScan 0x0040 /* Covering index scans */ -#define SQLITE_OrderByIdxJoin 0x0080 /* ORDER BY of joins via index */ -#define SQLITE_SubqCoroutine 0x0100 /* Evaluate subqueries as coroutines */ -#define SQLITE_Transitive 0x0200 /* Transitive constraints */ -#define SQLITE_OmitNoopJoin 0x0400 /* Omit unused tables in joins */ +#define SQLITE_DistinctOpt 0x0010 /* DISTINCT using indexes */ +#define SQLITE_CoverIdxScan 0x0020 /* Covering index scans */ +#define SQLITE_OrderByIdxJoin 0x0040 /* ORDER BY of joins via index */ +#define SQLITE_Transitive 0x0080 /* Transitive constraints */ +#define SQLITE_OmitNoopJoin 0x0100 /* Omit unused tables in joins */ +#define SQLITE_CountOfView 0x0200 /* The count-of-view optimization */ +#define SQLITE_CursorHints 0x0400 /* Add OP_CursorHint opcodes */ #define SQLITE_Stat34 0x0800 /* Use STAT3 or STAT4 data */ -#define SQLITE_CountOfView 0x1000 /* The count-of-view optimization */ -#define SQLITE_CursorHints 0x2000 /* Add OP_CursorHint opcodes */ + /* TH3 expects the Stat34 ^^^^^^ value to be 0x0800. Don't change it */ #define SQLITE_AllOpts 0xffff /* All optimizations */ /* diff --git a/src/test1.c b/src/test1.c index d70ce77a7a..446317d803 100644 --- a/src/test1.c +++ b/src/test1.c @@ -6901,7 +6901,6 @@ static int SQLITE_TCLAPI optimization_control( { "cover-idx-scan", SQLITE_CoverIdxScan }, { "order-by-idx-join", SQLITE_OrderByIdxJoin }, { "transitive", SQLITE_Transitive }, - { "subquery-coroutine", SQLITE_SubqCoroutine }, { "omit-noop-join", SQLITE_OmitNoopJoin }, { "stat3", SQLITE_Stat34 }, { "stat4", SQLITE_Stat34 }, diff --git a/test/having.test b/test/having.test index aea12319d7..a3882552d3 100644 --- a/test/having.test +++ b/test/having.test @@ -65,19 +65,6 @@ foreach {tn sql1 sql2} { 3 "SELECT a, sum(b) FROM t1 GROUP BY a COLLATE binary HAVING a=2" "SELECT a, sum(b) FROM t1 WHERE a=2 GROUP BY a COLLATE binary" - 4 { - SELECT x,y FROM ( - SELECT a AS x, sum(b) AS y FROM t1 - GROUP BY a - ) WHERE x BETWEEN 8888 AND 9999 - } { - SELECT x,y FROM ( - SELECT a AS x, sum(b) AS y FROM t1 - WHERE x BETWEEN 8888 AND 9999 - GROUP BY a - ) - } - 5 "SELECT a, sum(b) FROM t1 GROUP BY a COLLATE binary HAVING 0" "SELECT a, sum(b) FROM t1 WHERE 0 GROUP BY a COLLATE binary" @@ -98,6 +85,24 @@ foreach {tn sql1 sql2} { do_compare_vdbe_test 2.$tn $sql1 $sql2 1 } +# The (4) test in the above set used to generate identical bytecode, but +# that is no longer the case. The byte code is equivalent, though. +# +do_execsql_test 2.4a { + SELECT x,y FROM ( + SELECT a AS x, sum(b) AS y FROM t1 + GROUP BY a + ) WHERE x BETWEEN 2 AND 9999 +} {2 12} +do_execsql_test 2.4b { + SELECT x,y FROM ( + SELECT a AS x, sum(b) AS y FROM t1 + WHERE x BETWEEN 2 AND 9999 + GROUP BY a + ) +} {2 12} + + #------------------------------------------------------------------------- # 1: Test that the optimization is only applied if the GROUP BY term # uses BINARY collation.