From: drh <drh@noemail.net>
Date: Thu, 14 Apr 2016 19:29:59 +0000 (+0000)
Subject: Work toward improving analysis and code generation for DISTINCT and
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fheads%2Fbetter-distinct-agg;p=thirdparty%2Fsqlite.git

Work toward improving analysis and code generation for DISTINCT and
aggregate queries.

FossilOrigin-Name: 129083bd5e9f8688604a7b7169f15907ec6e332f
---

diff --git a/manifest b/manifest
index 420dfe28cf..a657fa6834 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Add\sthe\sSF_HasAgg\sconstant\s(currently\sunused).\s\sAlso\senhance\sthe\scomments\son\nmany\sother\sconstant\sdefinitions\sto\sdetail\sconstraints\son\stheir\svalues.
-D 2016-04-14T16:40:13.322
+C Work\stoward\simproving\sanalysis\sand\scode\sgeneration\sfor\sDISTINCT\sand\naggregate\squeries.
+D 2016-04-14T19:29:59.500
 F Makefile.in eba680121821b8a60940a81454316f47a341487a
 F Makefile.linux-gcc 7bc79876b875010e8c8f9502eb935ca92aa3c434
 F Makefile.msc 71b8b16cf9393f68e2e2035486ca104872558836
@@ -373,9 +373,9 @@ F src/pragma.h 64c78a648751b9f4f297276c4eb7507b14b4628c
 F src/prepare.c 22df6171aec1d86904ed2ad30c2348a5748aa04e
 F src/printf.c 63e6fb12bbe702dd664dc3703776c090383a5a26
 F src/random.c ba2679f80ec82c4190062d756f22d0c358180696
-F src/resolve.c b8f7174e5f8c33c44ded3a25a973d0bb89228c20
+F src/resolve.c d3eaf8c20c9b2292fb1d278293c1e4cfe0a9dc87
 F src/rowset.c 9fe4b3ad7cc00944386bb600233d8f523de07a6e
-F src/select.c 30217121bdf6b587462150b8ee9e1467f7a6036b
+F src/select.c d3ef06f9247f19fd4797f261b489077bd1fde41c
 F src/shell.c ebcdf99f3e7c7409bd463eae443f1bd01e3e2d02
 F src/sqlite.h.in 64eb70a3b309751bebf73a5552a51244f68f0ea5
 F src/sqlite3.rc 5121c9e10c3964d5755191c80dd1180c122fc3a8
@@ -462,7 +462,7 @@ F src/whereexpr.c eacc0e60d029a082b4fc0cc42ea98544add1319e
 F test/8_3_names.test ebbb5cd36741350040fd28b432ceadf495be25b2
 F test/affinity2.test a6d901b436328bd67a79b41bb0ac2663918fe3bd
 F test/aggerror.test a867e273ef9e3d7919f03ef4f0e8c0d2767944f2
-F test/aggnested.test b35b4cd69fc913f90d39a575e171e1116c3a4bb7
+F test/aggnested.test 116ab8359b88553a0cf20b44e655df9942e5ed41
 F test/alias.test 4529fbc152f190268a15f9384a5651bbbabc9d87
 F test/all.test 6ff7b43c2b4b905c74dc4a813d201d0fa64c5783
 F test/alter.test 2facdddf08d0d48e75dc6cc312cd2b030f4835dd
@@ -1021,7 +1021,7 @@ F test/schema4.test e6a66e20cc69f0e306667c08be7fda3d11707dc5
 F test/schema5.test 29699b4421f183c8f0e88bd28ce7d75d13ea653e
 F test/securedel.test 21749c32ccc30f1ea9e4b9f33295a6521ec20fa0
 F test/securedel2.test 2d54c28e46eb1fd6902089958b20b1b056c6f1c5
-F test/select1.test be62204d2bd9a5a8a149e9974cfddce893d8f686
+F test/select1.test e80459748cf40aecffa7cbd28069a73b791c8a2d
 F test/select2.test 352480e0e9c66eda9c3044e412abdf5be0215b56
 F test/select3.test 2ce595f8fb8e2ac10071d3b4e424cadd4634a054
 F test/select4.test d926792a5e4d88fef0ddcddeb45d27ce75f7296c
@@ -1482,7 +1482,10 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P 79147dca87cfd7eb62d57baa3b70fa2a8542232a
-R 6419c618333708e1ee248e43219e551e
+P 7b7a69d098f7581a43b818c251717c2450b797de
+R 13f89f53cffc3ab72b6f782d41fa8915
+T *branch * better-distinct-agg
+T *sym-better-distinct-agg *
+T -sym-trunk *
 U drh
-Z 76d9f185ebd1be8244746d28959cec5d
+Z e9a95e98abf36ab9d11354211f2ff8ae
diff --git a/manifest.uuid b/manifest.uuid
index 75ce1d9040..e8101fe065 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-7b7a69d098f7581a43b818c251717c2450b797de
\ No newline at end of file
+129083bd5e9f8688604a7b7169f15907ec6e332f
\ No newline at end of file
diff --git a/src/resolve.c b/src/resolve.c
index 8e290af123..0fe598b355 100644
--- a/src/resolve.c
+++ b/src/resolve.c
@@ -391,12 +391,12 @@ static int lookupName(
       for(j=0; j<pEList->nExpr; j++){
         char *zAs = pEList->a[j].zName;
         if( zAs!=0 && sqlite3StrICmp(zAs, zCol)==0 ){
-          Expr *pOrig;
           assert( pExpr->pLeft==0 && pExpr->pRight==0 );
           assert( pExpr->x.pList==0 );
           assert( pExpr->x.pSelect==0 );
-          pOrig = pEList->a[j].pExpr;
-          if( (pNC->ncFlags&NC_AllowAgg)==0 && ExprHasProperty(pOrig, EP_Agg) ){
+          if( (pNC->ncFlags&NC_AllowAgg)==0
+           && ExprHasProperty(pEList->a[j].pExpr, EP_Agg)
+          ){
             sqlite3ErrorMsg(pParse, "misuse of aliased aggregate %s", zAs);
             return WRC_Abort;
           }
@@ -714,11 +714,16 @@ static int resolveExprStep(Walker *pWalker, Expr *pExpr){
           notValid(pParse, pNC, "non-deterministic functions", NC_IdxExpr);
         }
       }
+#if 0
+      /* This error condition will be caught later, during code
+      ** generation */
       if( is_agg && (pNC->ncFlags & NC_AllowAgg)==0 ){
         sqlite3ErrorMsg(pParse, "misuse of aggregate function %.*s()", nId,zId);
         pNC->nErr++;
         is_agg = 0;
-      }else if( no_such_func && pParse->db->init.busy==0 ){
+      }else 
+#endif
+      if( no_such_func && pParse->db->init.busy==0 ){
         sqlite3ErrorMsg(pParse, "no such function: %.*s", nId, zId);
         pNC->nErr++;
       }else if( wrong_num_args ){
@@ -1208,20 +1213,9 @@ static int resolveSelectStep(Walker *pWalker, Select *p){
     /* Resolve names in the result set. */
     if( sqlite3ResolveExprListNames(&sNC, p->pEList) ) return WRC_Abort;
   
-    /* If there are no aggregate functions in the result-set, and no GROUP BY 
-    ** expression, do not allow aggregates in any of the other expressions.
-    */
-    assert( (p->selFlags & SF_Aggregate)==0 );
-    pGroupBy = p->pGroupBy;
-    if( pGroupBy || (sNC.ncFlags & NC_HasAgg)!=0 ){
-      assert( NC_MinMaxAgg==SF_MinMaxAgg );
-      p->selFlags |= SF_Aggregate | (sNC.ncFlags&NC_MinMaxAgg);
-    }else{
-      sNC.ncFlags &= ~NC_AllowAgg;
-    }
-  
     /* If a HAVING clause is present, then there must be a GROUP BY clause.
     */
+    pGroupBy = p->pGroupBy;
     if( p->pHaving && !pGroupBy ){
       sqlite3ErrorMsg(pParse, "a GROUP BY clause is required before HAVING");
       return WRC_Abort;
@@ -1236,8 +1230,20 @@ static int resolveSelectStep(Walker *pWalker, Select *p){
     ** re-evaluated for each reference to it.
     */
     sNC.pEList = p->pEList;
-    if( sqlite3ResolveExprNames(&sNC, p->pHaving) ) return WRC_Abort;
     if( sqlite3ResolveExprNames(&sNC, p->pWhere) ) return WRC_Abort;
+    if( sqlite3ResolveExprNames(&sNC, p->pHaving) ) return WRC_Abort;
+
+    /* If there are no aggregate functions in the result-set, and no GROUP BY 
+    ** expression, do not allow aggregates in any of the other expressions.
+    */
+    assert( (p->selFlags & SF_Aggregate)==0 );
+    if( pGroupBy || (sNC.ncFlags & NC_HasAgg)!=0 ){
+      assert( NC_MinMaxAgg==SF_MinMaxAgg );
+      assert( NC_HasAgg==SF_HasAgg );
+      p->selFlags |= SF_Aggregate | (sNC.ncFlags&(NC_MinMaxAgg|NC_HasAgg));
+    }else{
+      sNC.ncFlags &= ~NC_AllowAgg;
+    }
 
     /* Resolve names in table-valued-function arguments */
     for(i=0; i<p->pSrc->nSrc; i++){
diff --git a/src/select.c b/src/select.c
index b86e040f0f..c75e60d3f1 100644
--- a/src/select.c
+++ b/src/select.c
@@ -4899,12 +4899,12 @@ int sqlite3Select(
       goto select_end;
     }
 
-    isAggSub = (pSub->selFlags & SF_Aggregate)!=0;
+    isAggSub = pSub->selFlags & (SF_Aggregate|SF_HasAgg);
     if( flattenSubquery(pParse, p, i, isAgg, isAggSub) ){
       /* This subquery can be absorbed into its parent. */
       if( isAggSub ){
         isAgg = 1;
-        p->selFlags |= SF_Aggregate;
+        p->selFlags |= isAggSub;
       }
       i = -1;
     }
@@ -5062,6 +5062,7 @@ int sqlite3Select(
   pGroupBy = p->pGroupBy;
   pHaving = p->pHaving;
   sDistinct.isTnct = (p->selFlags & SF_Distinct)!=0;
+  assert( (p->selFlags & SF_HasAgg)==0 || (p->selFlags & SF_Aggregate)!=0 );
 
 #if SELECTTRACE_ENABLED
   if( sqlite3SelectTrace & 0x400 ){
@@ -5270,6 +5271,9 @@ int sqlite3Select(
     sAggInfo.mxReg = pParse->nMem;
     if( db->mallocFailed ) goto select_end;
 
+    /* Not true if there are errors in the input SQL!:
+    ** assert( (sAggInfo.nFunc>0)==((p->selFlags&SF_HasAgg)!=0) ); */
+
     /* Processing for aggregates with GROUP BY is very different and
     ** much more complex than aggregates without a GROUP BY.
     */
@@ -5656,6 +5660,7 @@ int sqlite3Select(
   /* The SELECT has been coded. If there is an error in the Parse structure,
   ** set the return code to 1. Otherwise 0. */
   rc = (pParse->nErr>0);
+  assert( (sAggInfo.nFunc>0)==((p->selFlags&SF_HasAgg)!=0) || rc!=SQLITE_OK );
 
   /* Control jumps to here if an error is encountered above, or upon
   ** successful coding of the SELECT.
diff --git a/test/aggnested.test b/test/aggnested.test
index a87c751eda..00d7f94e76 100644
--- a/test/aggnested.test
+++ b/test/aggnested.test
@@ -218,13 +218,12 @@ do_test aggnested-3.14 {
   }
 } {12 2 34 4}
 do_test aggnested-3.15 {
-  # FIXME:  If case 3.16 works, then this case really ought to work too...
   catchsql {
     SELECT max(value1), (SELECT sum(value2=max(value1)) FROM t2)
       FROM t1
      GROUP BY id1;
   }
-} {1 {misuse of aggregate function max()}}
+} {0 {12 2 34 4}}
 do_test aggnested-3.16 {
   db eval {
     SELECT max(value1), (SELECT sum(value2=value1) FROM t2)
diff --git a/test/select1.test b/test/select1.test
index 4d6c07f2d0..7596a01ca7 100644
--- a/test/select1.test
+++ b/test/select1.test
@@ -223,7 +223,7 @@ do_test select1-2.19 {
 do_test select1-2.20 {
   set v [catch {execsql {SELECT SUM(min(f1)) FROM test1}} msg]
   lappend v $msg
-} {1 {misuse of aggregate function min()}}
+} {1 {misuse of aggregate: min()}}
 
 # Ticket #2526
 #