From: dan <dan@noemail.net>
Date: Mon, 21 Apr 2014 13:21:56 +0000 (+0000)
Subject: Avoid discarding an ORDER BY clause in the case where an identical GROUP BY clauses... 
X-Git-Tag: version-3.8.5~74
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=374cd78c1db5b7b085cf2c289cfc3b04afaa2716;p=thirdparty%2Fsqlite.git

Avoid discarding an ORDER BY clause in the case where an identical GROUP BY clauses uses an index to group, but not sort, the rows. Fix for [b75a9ca6b0].

FossilOrigin-Name: de9a490f594183f337a2ec9e0f87792eac83548b
---

diff --git a/manifest b/manifest
index ad0f59f21e..caaf603930 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Clean\sup\sthe\sproper-subset\scost\sadjustment\slogic\sto\smake\sit\smore\scompact\nand\seasier\sto\sread\sand\sso\sthat\sfull\sbranch\stest\scoverage\sis\smore\seasily\nobtained.
-D 2014-04-18T22:20:31.054
+C Avoid\sdiscarding\san\sORDER\sBY\sclause\sin\sthe\scase\swhere\san\sidentical\sGROUP\sBY\sclauses\suses\san\sindex\sto\sgroup,\sbut\snot\ssort,\sthe\srows.\sFix\sfor\s[b75a9ca6b0].
+D 2014-04-21T13:21:56.964
 F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f
 F Makefile.in 2ef13430cd359f7b361bb863504e227b25cc7f81
 F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23
@@ -217,12 +217,12 @@ F src/printf.c e5a0005f8b3de21f85da6a709d2fbee76775bf4b
 F src/random.c d10c1f85b6709ca97278428fd5db5bbb9c74eece
 F src/resolve.c 273d5f47c4e2c05b2d3d2bffeda939551ab59e66
 F src/rowset.c a9c9aae3234b44a6d7c6f5a3cadf90dce1e627be
-F src/select.c 269c3e31a450fce642a10569221a49180348c88e
+F src/select.c bc7feff0fb4c4a1b9d655b717bef166846b48e33
 F src/shell.c 2afe7a7154e97be0c74c5feacf09626bda8493be
 F src/sqlite.h.in bde98816e1ba0c9ffef50afe7b32f4e5a8f54fe0
 F src/sqlite3.rc 11094cc6a157a028b301a9f06b3d03089ea37c3e
 F src/sqlite3ext.h 886f5a34de171002ad46fae8c36a7d8051c190fc
-F src/sqliteInt.h d3ca0bdd6743c9af2f333cb372b4bf95b19cd1fd
+F src/sqliteInt.h 03e2f60ccb0745fa2d3a072cb4f75fa29251d2ee
 F src/sqliteLimit.h 164b0e6749d31e0daa1a4589a169d31c0dec7b3d
 F src/status.c 7ac05a5c7017d0b9f0b4bcd701228b784f987158
 F src/table.c 2cd62736f845d82200acfa1287e33feb3c15d62e
@@ -291,8 +291,8 @@ F src/vtab.c 21b932841e51ebd7d075e2d0ad1415dce8d2d5fd
 F src/wal.c 76e7fc6de229bea8b30bb2539110f03a494dc3a8
 F src/wal.h df01efe09c5cb8c8e391ff1715cca294f89668a4
 F src/walker.c 11edb74d587bc87b33ca96a5173e3ec1b8389e45
-F src/where.c 114b480d3c2eefd9fbc395ee96e03fbd883f418d
-F src/whereInt.h 2564055b440e44ebec8b47f237bbccae6719b7af
+F src/where.c 3b127bdc24b7aa84ffa69729170be11555cd7733
+F src/whereInt.h 929c1349b5355fd44f22cee5c14d72b3329c58a6
 F test/8_3_names.test ebbb5cd36741350040fd28b432ceadf495be25b2
 F test/aggerror.test a867e273ef9e3d7919f03ef4f0e8c0d2767944f2
 F test/aggnested.test 45c0201e28045ad38a530b5a144b73cd4aa2cfd6
@@ -896,6 +896,7 @@ F test/tkt-a8a0d2996a.test eb597379dbcefa24765763d7f682c00cb5924fa9
 F test/tkt-b1d3a2e531.test 8f7576e41ca179289ee1a8fee28386fd8e4b0550
 F test/tkt-b351d95f9.test d14a503c414c5c58fdde3e80f9a3cfef986498c0
 F test/tkt-b72787b1.test a95e8cdad0b98af1853ac7f0afd4ab27b77bf5f3
+F test/tkt-b75a9ca6b0.test 3f4f7ca6ab552b6efdb664f821bc798a08929b2c
 F test/tkt-bd484a090c.test 60460bf946f79a79712b71f202eda501ca99b898
 F test/tkt-bdc6bbbb38.test fc38bb09bdd440e3513a1f5f98fc60a075182d7d
 F test/tkt-c48d99d690.test ba61977d62ab612fc515b3c488a6fbd6464a2447
@@ -1160,7 +1161,7 @@ F tool/vdbe_profile.tcl 67746953071a9f8f2f668b73fe899074e2c6d8c1
 F tool/warnings-clang.sh f6aa929dc20ef1f856af04a730772f59283631d4
 F tool/warnings.sh d1a6de74685f360ab718efda6265994b99bbea01
 F tool/win/sqlite.vsix 030f3eeaf2cb811a3692ab9c14d021a75ce41fff
-P 2c5363873a6f990a0abaacac6303acd46b48befc
-R b712e75910761b4771536f725cdc6409
-U drh
-Z e77c4b2dd81e4b608cc5f9bf1e51b6ae
+P 9a5d38c79d2482a23bcfbc3ff35ca4fa269c768d
+R ee944819c64836615905c8a0073df726
+U dan
+Z 6ac990d90beb1121a87e97a89a4f2118
diff --git a/manifest.uuid b/manifest.uuid
index 3f9af7e6d6..6776a57944 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-9a5d38c79d2482a23bcfbc3ff35ca4fa269c768d
\ No newline at end of file
+de9a490f594183f337a2ec9e0f87792eac83548b
\ No newline at end of file
diff --git a/src/select.c b/src/select.c
index c7f0b24a4a..6efdde4b72 100644
--- a/src/select.c
+++ b/src/select.c
@@ -4719,18 +4719,6 @@ int sqlite3Select(
   }
 #endif
 
-  /* If there is both a GROUP BY and an ORDER BY clause and they are
-  ** identical, then disable the ORDER BY clause since the GROUP BY
-  ** will cause elements to come out in the correct order.  This is
-  ** an optimization - the correct answer should result regardless.
-  ** Use the SQLITE_GroupByOrder flag with SQLITE_TESTCTRL_OPTIMIZER
-  ** to disable this optimization for testing purposes.
-  */
-  if( sqlite3ExprListCompare(p->pGroupBy, sSort.pOrderBy, -1)==0
-         && OptimizationEnabled(db, SQLITE_GroupByOrder) ){
-    sSort.pOrderBy = 0;
-  }
-
   /* If the query is DISTINCT with an ORDER BY but is not an aggregate, and 
   ** if the select-list is the same as the ORDER BY list, then this query
   ** can be rewritten as a GROUP BY. In other words, this:
@@ -4859,6 +4847,7 @@ int sqlite3Select(
     int addrEnd;        /* End of processing for this SELECT */
     int sortPTab = 0;   /* Pseudotable used to decode sorting results */
     int sortOut = 0;    /* Output register from the sorter */
+    int orderByGrp = 0; /* True if the GROUP BY and ORDER BY are the same */
 
     /* Remove any and all aliases between the result set and the
     ** GROUP BY clause.
@@ -4878,6 +4867,18 @@ int sqlite3Select(
       p->nSelectRow = 1;
     }
 
+
+    /* If there is both a GROUP BY and an ORDER BY clause and they are
+    ** identical, then it may be possible to disable the ORDER BY clause 
+    ** on the grounds that the GROUP BY will cause elements to come out 
+    ** in the correct order. It also may not - the GROUP BY may use a
+    ** database index that causes rows to be grouped together as required
+    ** but not actually sorted. Either way, record the fact that the
+    ** ORDER BY and GROUP BY clauses are the same by setting the orderByGrp
+    ** variable.  */
+    if( sqlite3ExprListCompare(pGroupBy, sSort.pOrderBy, -1)==0 ){
+      orderByGrp = 1;
+    }
  
     /* Create a label to jump to when we want to abort the query */
     addrEnd = sqlite3VdbeMakeLabel(v);
@@ -4958,7 +4959,8 @@ int sqlite3Select(
       */
       sqlite3VdbeAddOp2(v, OP_Gosub, regReset, addrReset);
       pWInfo = sqlite3WhereBegin(pParse, pTabList, pWhere, pGroupBy, 0,
-                                 WHERE_GROUPBY, 0);
+          WHERE_GROUPBY | (orderByGrp ? WHERE_SORTBYGROUP : 0), 0
+      );
       if( pWInfo==0 ) goto select_end;
       if( sqlite3WhereIsOrdered(pWInfo)==pGroupBy->nExpr ){
         /* The optimizer is able to deliver rows in group by order so
@@ -5023,6 +5025,21 @@ int sqlite3Select(
         VdbeComment((v, "GROUP BY sort")); VdbeCoverage(v);
         sAggInfo.useSortingIdx = 1;
         sqlite3ExprCacheClear(pParse);
+
+      }
+
+      /* If the index or temporary table used by the GROUP BY sort
+      ** will naturally deliver rows in the order required by the ORDER BY
+      ** clause, cancel the ephemeral table open coded earlier.
+      **
+      ** This is an optimization - the correct answer should result regardless.
+      ** Use the SQLITE_GroupByOrder flag with SQLITE_TESTCTRL_OPTIMIZER to 
+      ** disable this optimization for testing purposes.  */
+      if( orderByGrp && OptimizationEnabled(db, SQLITE_GroupByOrder) 
+       && (groupBySort || sqlite3WhereIsSorted(pWInfo))
+      ){
+        sSort.pOrderBy = 0;
+        sqlite3VdbeChangeToNoop(v, sSort.addrSortIndex);
       }
 
       /* Evaluate the current GROUP BY terms and store in b0, b1, b2...
diff --git a/src/sqliteInt.h b/src/sqliteInt.h
index 8197a61eb5..8e81b7a970 100644
--- a/src/sqliteInt.h
+++ b/src/sqliteInt.h
@@ -2125,6 +2125,7 @@ struct SrcList {
 #define WHERE_GROUPBY          0x0100 /* pOrderBy is really a GROUP BY */
 #define WHERE_DISTINCTBY       0x0200 /* pOrderby is really a DISTINCT clause */
 #define WHERE_WANT_DISTINCT    0x0400 /* All output needs to be distinct */
+#define WHERE_SORTBYGROUP      0x0800 /* Support sqlite3WhereIsSorted() */
 
 /* Allowed return values from sqlite3WhereIsDistinct()
 */
@@ -3092,6 +3093,7 @@ void sqlite3WhereEnd(WhereInfo*);
 u64 sqlite3WhereOutputRowCount(WhereInfo*);
 int sqlite3WhereIsDistinct(WhereInfo*);
 int sqlite3WhereIsOrdered(WhereInfo*);
+int sqlite3WhereIsSorted(WhereInfo*);
 int sqlite3WhereContinueLabel(WhereInfo*);
 int sqlite3WhereBreakLabel(WhereInfo*);
 int sqlite3WhereOkOnePass(WhereInfo*, int*);
diff --git a/src/where.c b/src/where.c
index 31a0dc5452..9bde27e52f 100644
--- a/src/where.c
+++ b/src/where.c
@@ -4796,7 +4796,7 @@ static int whereLoopAddAll(WhereLoopBuilder *pBuilder){
 ** Note that processing for WHERE_GROUPBY and WHERE_DISTINCTBY is not as
 ** strict.  With GROUP BY and DISTINCT the only requirement is that
 ** equivalent rows appear immediately adjacent to one another.  GROUP BY
-** and DISTINT do not require rows to appear in any particular order as long
+** and DISTINCT do not require rows to appear in any particular order as long
 ** as equivelent rows are grouped together.  Thus for GROUP BY and DISTINCT
 ** the pOrderBy terms can be matched in any order.  With ORDER BY, the 
 ** pOrderBy terms must be matched in strict left-to-right order.
@@ -4965,7 +4965,7 @@ static i8 wherePathSatisfiesOrderBy(
         }
 
         /* Find the ORDER BY term that corresponds to the j-th column
-        ** of the index and and mark that ORDER BY term off 
+        ** of the index and mark that ORDER BY term off 
         */
         bOnce = 1;
         isMatch = 0;
@@ -5045,6 +5045,36 @@ static i8 wherePathSatisfiesOrderBy(
   return -1;
 }
 
+
+/*
+** If the WHERE_GROUPBY flag is set in the mask passed to sqlite3WhereBegin(),
+** the planner assumes that the specified pOrderBy list is actually a GROUP
+** BY clause - and so any order that groups rows as required satisfies the
+** request.
+**
+** Normally, in this case it is not possible for the caller to determine
+** whether or not the rows are really being delivered in sorted order, or
+** just in some other order that provides the required grouping. However,
+** if the WHERE_SORTBYGROUP flag is also passed to sqlite3WhereBegin(), then
+** this function may be called on the returned WhereInfo object. It returns
+** true if the rows really will be sorted in the specified order, or false
+** otherwise.
+**
+** For example, assuming:
+**
+**   CREATE INDEX i1 ON t1(x, Y);
+**
+** then
+**
+**   SELECT * FROM t1 GROUP BY x,y ORDER BY x,y;   -- IsSorted()==1
+**   SELECT * FROM t1 GROUP BY y,x ORDER BY y,x;   -- IsSorted()==0
+*/
+int sqlite3WhereIsSorted(WhereInfo *pWInfo){
+  assert( pWInfo->wctrlFlags & WHERE_GROUPBY );
+  assert( pWInfo->wctrlFlags & WHERE_SORTBYGROUP );
+  return pWInfo->sorted;
+}
+
 #ifdef WHERETRACE_ENABLED
 /* For debugging use only: */
 static const char *wherePathName(WherePath *pPath, int nLoop, WhereLoop *pLast){
@@ -5057,7 +5087,6 @@ static const char *wherePathName(WherePath *pPath, int nLoop, WhereLoop *pLast){
 }
 #endif
 
-
 /*
 ** Given the list of WhereLoop objects at pWInfo->pLoops, this routine
 ** attempts to find the lowest cost path that visits each WhereLoop
@@ -5338,7 +5367,19 @@ static int wherePathSolver(WhereInfo *pWInfo, LogEst nRowEst){
       if( pWInfo->nOBSat<0 ) pWInfo->nOBSat = 0;
       pWInfo->revMask = pFrom->revLoop;
     }
+    if( (pWInfo->wctrlFlags & WHERE_SORTBYGROUP)
+        && pWInfo->nOBSat==pWInfo->pOrderBy->nExpr
+    ){
+      Bitmask notUsed = 0;
+      int nOrder = wherePathSatisfiesOrderBy(pWInfo, pWInfo->pOrderBy, 
+          pFrom, 0, nLoop-1, pFrom->aLoop[nLoop-1], &notUsed
+      );
+      assert( pWInfo->sorted==0 );
+      pWInfo->sorted = (nOrder==pWInfo->pOrderBy->nExpr);
+    }
   }
+
+
   pWInfo->nRowOut = pFrom->nRow;
 
   /* Free temporary memory and return success */
diff --git a/src/whereInt.h b/src/whereInt.h
index 419bb81c0e..72e7530db9 100644
--- a/src/whereInt.h
+++ b/src/whereInt.h
@@ -398,6 +398,7 @@ struct WhereInfo {
   LogEst nRowOut;           /* Estimated number of output rows */
   u16 wctrlFlags;           /* Flags originally passed to sqlite3WhereBegin() */
   i8 nOBSat;                /* Number of ORDER BY terms satisfied by indices */
+  u8 sorted;                /* True if really sorted (not just grouped) */
   u8 okOnePass;             /* Ok to use one-pass algorithm for UPDATE/DELETE */
   u8 untestedTerms;         /* Not all WHERE terms resolved by outer loop */
   u8 eDistinct;             /* One of the WHERE_DISTINCT_* values below */
diff --git a/test/tkt-b75a9ca6b0.test b/test/tkt-b75a9ca6b0.test
new file mode 100644
index 0000000000..38918bedaa
--- /dev/null
+++ b/test/tkt-b75a9ca6b0.test
@@ -0,0 +1,75 @@
+# 2014 April 21
+#
+# 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.
+#
+#*************************************************************************
+#
+# Test that ticket [b75a9ca6b0] has been fixed.
+#
+
+set testdir [file dirname $argv0]
+source $testdir/tester.tcl
+set testprefix tkt-b75a9ca6b0
+
+do_execsql_test 1 {
+  CREATE TABLE t1 (x, y);
+  INSERT INTO t1 VALUES (1, 3); 
+  INSERT INTO t1 VALUES (2, 2);
+  INSERT INTO t1 VALUES (3, 1);
+}
+
+do_execsql_test 1.1 {
+  CREATE INDEX i1 ON t1(x, y);
+} 
+
+set idxscan {0 0 0 {SCAN TABLE t1 USING COVERING INDEX i1}}
+set tblscan {0 0 0 {SCAN TABLE t1}}
+set grpsort {0 0 0 {USE TEMP B-TREE FOR GROUP BY}}
+set sort    {0 0 0 {USE TEMP B-TREE FOR ORDER BY}}
+
+foreach {tn q res eqp} [subst -nocommands {
+  1 "SELECT * FROM t1 GROUP BY x, y ORDER BY x,y"
+  {1 3  2 2  3 1} {$idxscan}
+
+  2 "SELECT * FROM t1 GROUP BY x, y ORDER BY x"
+  {1 3  2 2  3 1} {$idxscan $sort}
+
+  3 "SELECT * FROM t1 GROUP BY y, x ORDER BY y, x"
+  {3 1  2 2  1 3} {$idxscan $sort}
+  
+  4 "SELECT * FROM t1 GROUP BY x ORDER BY x"
+  {1 3  2 2  3 1} {$idxscan}
+
+  5 "SELECT * FROM t1 GROUP BY y ORDER BY y"
+  {3 1  2 2  1 3} {$tblscan $grpsort}
+
+  6 "SELECT * FROM t1 GROUP BY y ORDER BY x"
+  {1 3  2 2  3 1} {$tblscan $grpsort $sort}
+
+  7 "SELECT * FROM t1 GROUP BY x, y ORDER BY x, y DESC"
+  {1 3  2 2  3 1} {$idxscan $sort}
+
+  8 "SELECT * FROM t1 GROUP BY x, y ORDER BY x DESC, y DESC"
+  {3 1  2 2  1 3} {$idxscan $sort}
+
+  9 "SELECT * FROM t1 GROUP BY x, y ORDER BY x ASC, y ASC"
+  {1 3  2 2  3 1} {$idxscan}
+
+  10 "SELECT * FROM t1 GROUP BY x, y ORDER BY x COLLATE nocase, y"
+  {1 3  2 2  3 1} {$idxscan $sort}
+
+}] {
+  do_execsql_test 1.$tn.1 $q $res
+  do_eqp_test     1.$tn.2 $q $eqp
+}
+
+
+finish_test
+
+
+