From 5579d59fb3ccb9e9db9913a5bd50871d8e893d95 Mon Sep 17 00:00:00 2001 From: drh Date: Wed, 26 Aug 2015 14:01:41 +0000 Subject: [PATCH] Evaluate expressions only once when the same expression is used in both the result set and in the ORDER BY clause. FossilOrigin-Name: c2f3bbad778504681b39ab9399a1eb3c1a35ab3f --- manifest | 21 ++++++++++--------- manifest.uuid | 2 +- src/expr.c | 11 ++++++---- src/resolve.c | 4 ++-- src/select.c | 20 +++++++++++------- src/sqliteInt.h | 3 ++- src/treeview.c | 6 ++++++ test/orderby9.test | 52 ++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 93 insertions(+), 26 deletions(-) create mode 100644 test/orderby9.test diff --git a/manifest b/manifest index 6f175329a1..5a48ac0a18 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Refactor\sWith.a.zErr\sinto\sWith.a.zCteErr.\s\sNo\slogic\schanges. -D 2015-08-26T11:40:11.832 +C Evaluate\sexpressions\sonly\sonce\swhen\sthe\ssame\sexpression\sis\sused\sin\sboth\sthe\nresult\sset\sand\sin\sthe\sORDER\sBY\sclause. +D 2015-08-26T14:01:41.658 F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f F Makefile.in e2218eb228374422969de7b1680eda6864affcef F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23 @@ -289,7 +289,7 @@ F src/ctime.c 5a0b735dc95604766f5dac73973658eef782ee8b F src/date.c 8ec787fed4929d8ccdf6b1bc360fccc3e1d2ca58 F src/dbstat.c f402e77e25089c6003d0c60b3233b9b3947d599a F src/delete.c 813be7b5659d7658c8a71b5ae194b45c8f739c8b -F src/expr.c 650ac7c4f659980a3315e2aaa02a0d71e87f14a5 +F src/expr.c 5944e529891416f482250e16c598d8c26e149eb0 F src/fault.c 160a0c015b6c2629d3899ed2daf63d75754a32bb F src/fkey.c 3ce33dd49f12c72376cec9adc7a4d8e7111cedcc F src/func.c 824bea430d3a2b7dbc62806ad54da8fdb8ed9e3f @@ -335,14 +335,14 @@ F src/pragma.h 631a91c8b0e6ca8f051a1d8a4a0da4150e04620a F src/prepare.c 82e5db1013846a819f198336fed72c44c974e7b1 F src/printf.c 2bc439ff20a4aad0e0ad50a37a67b5eae7d20edc F src/random.c ba2679f80ec82c4190062d756f22d0c358180696 -F src/resolve.c 66b2740075fdb8baf90155180d33d9850cbcc976 +F src/resolve.c f2ef256786a6435efddd64a632fea89c8be62215 F src/rowset.c eccf6af6d620aaa4579bd3b72c1b6395d9e9fa1e -F src/select.c 50b0f02ec4482f92749e6630de9cd0d175a040e1 +F src/select.c b52c80f2b1bdb62491f9ce40eea0c5f80c78d105 F src/shell.c b1f91e60918df3a68efad1e3a11696b9a7e23d23 F src/sqlite.h.in 378bebc8fe6a88bade25e5f23b7e6123fdc64b00 F src/sqlite3.rc 992c9f5fb8285ae285d6be28240a7e8d3a7f2bad F src/sqlite3ext.h f700e6a9dd1fdcccc9951ab022b366fb66b9e413 -F src/sqliteInt.h d76e7c90775efeec72ea254a5da0a9f1ddcff765 +F src/sqliteInt.h cac6c31a0c7e6aa5572cc97b68d2630034d6d212 F src/sqliteLimit.h 216557999cb45f2e3578ed53ebefe228d779cb46 F src/status.c f266ad8a2892d659b74f0f50cb6a88b6e7c12179 F src/table.c 51b46b2a62d1b3a959633d593b89bab5e2c9155e @@ -394,7 +394,7 @@ F src/test_vfstrace.c bab9594adc976cbe696ff3970728830b4c5ed698 F src/test_wsd.c 41cadfd9d97fe8e3e4e44f61a4a8ccd6f7ca8fe9 F src/threads.c 6bbcc9fe50c917864d48287b4792d46d6e873481 F src/tokenize.c 57cb3720f53f84d811def2069c2b169b6be539a5 -F src/treeview.c 24950c6a79583016c83c43c8b741b3b79a096ce8 +F src/treeview.c 46036cbbceada0836833531b2d963edbca3d9cfa F src/trigger.c 322f23aad694e8f31d384dcfa386d52a48d3c52f F src/update.c adc8b4b2b6fd2cca2e0d2b803e0cf6956aa3a030 F src/utf.c fc6b889ba0779b7722634cdeaa25f1930d93820c @@ -899,6 +899,7 @@ F test/orderby5.test 8f08a54836d21fb7c70245360751aedd1c2286fb F test/orderby6.test 8b38138ab0972588240b3fca0985d2e400432859 F test/orderby7.test 3d1383d52ade5b9eb3a173b3147fdd296f0202da F test/orderby8.test 23ef1a5d72bd3adcc2f65561c654295d1b8047bd +F test/orderby9.test 88a330ea5fc7bed7e407b28beb0d2b79485ae2cc F test/oserror.test 14fec2796c2b6fe431c7823750e8a18a761176d7 F test/ovfl.test 4f7ca651cba5c059a12d8c67dddd49bec5747799 F test/pager1.test 1acbdb14c5952a72dd43129cabdbf69aaa3ed1fa @@ -1379,7 +1380,7 @@ F tool/vdbe_profile.tcl 67746953071a9f8f2f668b73fe899074e2c6d8c1 F tool/warnings-clang.sh f6aa929dc20ef1f856af04a730772f59283631d4 F tool/warnings.sh 48bd54594752d5be3337f12c72f28d2080cb630b F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f -P 65a8918776aa395009a690fa86bfc7d99eb973f9 -R a6b3114ece5e49154c10e2c8118e9866 +P 58ba73630ecc4bc58b03a7962dd45b305ef605ef +R 657299463d01da646731fb6b8ab7d9fe U drh -Z b1a98f3cfbd2154f3cffafb179efca91 +Z 83e3936f8767c9500521d2082aa870a1 diff --git a/manifest.uuid b/manifest.uuid index 867315d21b..1045acaa92 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -58ba73630ecc4bc58b03a7962dd45b305ef605ef \ No newline at end of file +c2f3bbad778504681b39ab9399a1eb3c1a35ab3f \ No newline at end of file diff --git a/src/expr.c b/src/expr.c index 1aebef6b16..71c552c687 100644 --- a/src/expr.c +++ b/src/expr.c @@ -2912,7 +2912,7 @@ int sqlite3ExprCodeTarget(Parse *pParse, Expr *pExpr, int target){ } sqlite3ExprCachePush(pParse); /* Ticket 2ea2425d34be */ - sqlite3ExprCodeExprList(pParse, pFarg, r1, + sqlite3ExprCodeExprList(pParse, pFarg, r1, 0, SQLITE_ECEL_DUP|SQLITE_ECEL_FACTOR); sqlite3ExprCachePop(pParse); /* Ticket 2ea2425d34be */ }else{ @@ -3328,11 +3328,13 @@ int sqlite3ExprCodeExprList( Parse *pParse, /* Parsing context */ ExprList *pList, /* The expression list to be coded */ int target, /* Where to write results */ + int srcReg, /* Source registers if SQLITE_ECEL_REF */ u8 flags /* SQLITE_ECEL_* flags */ ){ struct ExprList_item *pItem; - int i, n; + int i, j, n; u8 copyOp = (flags & SQLITE_ECEL_DUP) ? OP_Copy : OP_SCopy; + Vdbe *v = pParse->pVdbe; assert( pList!=0 ); assert( target>0 ); assert( pParse->pVdbe!=0 ); /* Never gets this far otherwise */ @@ -3340,13 +3342,14 @@ int sqlite3ExprCodeExprList( if( !ConstFactorOk(pParse) ) flags &= ~SQLITE_ECEL_FACTOR; for(pItem=pList->a, i=0; ipExpr; - if( (flags & SQLITE_ECEL_FACTOR)!=0 && sqlite3ExprIsConstant(pExpr) ){ + if( (flags & SQLITE_ECEL_REF)!=0 && (j = pList->a[i].u.x.iOrderByCol)>0 ){ + sqlite3VdbeAddOp2(v, copyOp, j+srcReg-1, target+i); + }else if( (flags & SQLITE_ECEL_FACTOR)!=0 && sqlite3ExprIsConstant(pExpr) ){ sqlite3ExprCodeAtInit(pParse, pExpr, target+i, 0); }else{ int inReg = sqlite3ExprCodeTarget(pParse, pExpr, target+i); if( inReg!=target+i ){ VdbeOp *pOp; - Vdbe *v = pParse->pVdbe; if( copyOp==OP_Copy && (pOp=sqlite3VdbeGetOp(v, -1))->opcode==OP_Copy && pOp->p1+pOp->p3+1==inReg diff --git a/src/resolve.c b/src/resolve.c index 4ef8fe051b..04fa8429a8 100644 --- a/src/resolve.c +++ b/src/resolve.c @@ -407,9 +407,9 @@ static int lookupName( ** resolved by the time the WHERE clause is resolved. ** ** The ability to use an output result-set column in the WHERE, GROUP BY, - ** or HAVING clauses, or as part of a larger expression in the ORDRE BY + ** or HAVING clauses, or as part of a larger expression in the ORDER BY ** clause is not standard SQL. This is a (goofy) SQLite extension, that - ** is supported for backwards compatibility only. TO DO: Issue a warning + ** is supported for backwards compatibility only. Hence, we issue a warning ** on sqlite3_log() whenever the capability is used. */ if( (pEList = pNC->pEList)!=0 diff --git a/src/select.c b/src/select.c index dbf06dd9ad..21366b9d11 100644 --- a/src/select.c +++ b/src/select.c @@ -496,6 +496,7 @@ static void pushOntoSorter( SortCtx *pSort, /* Information about the ORDER BY clause */ Select *pSelect, /* The whole SELECT statement */ int regData, /* First register holding data to be sorted */ + int regOrigData, /* First register holding data before packing */ int nData, /* Number of elements in the data array */ int nPrefixReg /* No. of reg prior to regData available for use */ ){ @@ -509,6 +510,7 @@ static void pushOntoSorter( int op; /* Opcode to add sorter record to sorter */ assert( bSeq==0 || bSeq==1 ); + assert( nData==1 || regData==regOrigData ); if( nPrefixReg ){ assert( nPrefixReg==nExpr+bSeq ); regBase = regData - nExpr - bSeq; @@ -516,7 +518,8 @@ static void pushOntoSorter( regBase = pParse->nMem + 1; pParse->nMem += nBase; } - sqlite3ExprCodeExprList(pParse, pSort->pOrderBy, regBase, SQLITE_ECEL_DUP); + sqlite3ExprCodeExprList(pParse, pSort->pOrderBy, regBase, regOrigData, + SQLITE_ECEL_DUP|SQLITE_ECEL_REF); if( bSeq ){ sqlite3VdbeAddOp2(v, OP_Sequence, pSort->iECursor, regBase+nExpr); } @@ -726,7 +729,7 @@ static void selectInnerLoop( }else{ ecelFlags = 0; } - sqlite3ExprCodeExprList(pParse, pEList, regResult, ecelFlags); + sqlite3ExprCodeExprList(pParse, pEList, regResult, 0, ecelFlags); } /* If the DISTINCT keyword was present on the SELECT statement @@ -842,7 +845,7 @@ static void selectInnerLoop( } #endif if( pSort ){ - pushOntoSorter(pParse, pSort, p, r1+nPrefixReg, 1, nPrefixReg); + pushOntoSorter(pParse, pSort, p, r1+nPrefixReg,regResult,1,nPrefixReg); }else{ int r2 = sqlite3GetTempReg(pParse); sqlite3VdbeAddOp2(v, OP_NewRowid, iParm, r2); @@ -868,7 +871,7 @@ static void selectInnerLoop( ** ORDER BY in this case since the order of entries in the set ** does not matter. But there might be a LIMIT clause, in which ** case the order does matter */ - pushOntoSorter(pParse, pSort, p, regResult, 1, nPrefixReg); + pushOntoSorter(pParse, pSort, p, regResult, regResult, 1, nPrefixReg); }else{ int r1 = sqlite3GetTempReg(pParse); sqlite3VdbeAddOp4(v, OP_MakeRecord, regResult,1,r1, &pDest->affSdst, 1); @@ -894,7 +897,7 @@ static void selectInnerLoop( case SRT_Mem: { assert( nResultCol==1 ); if( pSort ){ - pushOntoSorter(pParse, pSort, p, regResult, 1, nPrefixReg); + pushOntoSorter(pParse, pSort, p, regResult, regResult, 1, nPrefixReg); }else{ assert( regResult==iParm ); /* The LIMIT clause will jump out of the loop for us */ @@ -908,7 +911,8 @@ static void selectInnerLoop( testcase( eDest==SRT_Coroutine ); testcase( eDest==SRT_Output ); if( pSort ){ - pushOntoSorter(pParse, pSort, p, regResult, nResultCol, nPrefixReg); + pushOntoSorter(pParse, pSort, p, regResult, regResult, nResultCol, + nPrefixReg); }else if( eDest==SRT_Coroutine ){ sqlite3VdbeAddOp1(v, OP_Yield, pDest->iSDParm); }else{ @@ -4667,7 +4671,7 @@ static void updateAccumulator(Parse *pParse, AggInfo *pAggInfo){ if( pList ){ nArg = pList->nExpr; regAgg = sqlite3GetTempRange(pParse, nArg); - sqlite3ExprCodeExprList(pParse, pList, regAgg, SQLITE_ECEL_DUP); + sqlite3ExprCodeExprList(pParse, pList, regAgg, 0, SQLITE_ECEL_DUP); }else{ nArg = 0; regAgg = 0; @@ -5287,7 +5291,7 @@ int sqlite3Select( } regBase = sqlite3GetTempRange(pParse, nCol); sqlite3ExprCacheClear(pParse); - sqlite3ExprCodeExprList(pParse, pGroupBy, regBase, 0); + sqlite3ExprCodeExprList(pParse, pGroupBy, regBase, 0, 0); j = nGroupBy; for(i=0; inExpr; i++){ + int j = pList->a[i].u.x.iOrderByCol; + if( j ){ + sqlite3TreeViewPush(pView, 0); + sqlite3TreeViewLine(pView, "iOrderByCol=%d", j); + } sqlite3TreeViewExpr(pView, pList->a[i].pExpr, inExpr-1); + if( j ) sqlite3TreeViewPop(pView); } } sqlite3TreeViewPop(pView); diff --git a/test/orderby9.test b/test/orderby9.test new file mode 100644 index 0000000000..c998c5054e --- /dev/null +++ b/test/orderby9.test @@ -0,0 +1,52 @@ +# 2015-08-26 +# +# 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. +# +# This file seeks to verify that expressions (and especially functions) +# that are in both the ORDER BY clause and the result set are only +# evaluated once. +# + +set testdir [file dirname $argv0] +source $testdir/tester.tcl +set ::testprefix orderby9 + + +do_execsql_test setup { + -- create a table with many entries + CREATE TABLE t1(x); + WITH RECURSIVE + c(x) AS (VALUES(1) UNION ALL SELECT x+1 FROM c WHERE x<100) + INSERT INTO t1 SELECT x FROM c; +} +do_test 1.0 { + set l1 {} + # If random() is only evaluated once and then reused for each row, then + # the output should appear in sorted order. If random() is evaluated + # separately for the result set and the ORDER BY clause, then the output + # order will be random. + db eval {SELECT random() AS y FROM t1 ORDER BY 1;} {lappend l1 $y} + expr {$l1==[lsort -int $l1]} +} {1} + +do_test 1.1 { + set l1 {} + db eval {SELECT random() AS y FROM t1 ORDER BY random();} {lappend l1 $y} + expr {$l1==[lsort -int $l1]} +} {1} + +do_test 1.2 { + set l1 {} + db eval {SELECT random() AS y FROM t1 ORDER BY +random();} {lappend l1 $y} + expr {$l1==[lsort -int $l1]} +} {0} + +finish_test