This attempt at modifying AggInfo to make use of indexed expressions does not

work.  It gets an incorrect answer for the test case shown in the ticket.

FossilOrigin-Name: 84c06023f4a1606664fdb9811312603b31f7c94a43d0e443ba7dde7fdba029e3

diff --git a/manifest b/manifest
index 1ffe8b4bb74db3470da6946e229695487832e44d..294477b047ce6dce886b8b17e084be9a9740fd2b 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Further\sfoundation\sprep\swork\sprior\sto\sstarting\sto\sflesh-out\sthe\noptimizeAggregateUseOfIndexedExpr()\sroutine.
-D 2022-11-23T14:13:39.215
+C This\sattempt\sat\smodifying\sAggInfo\sto\smake\suse\sof\sindexed\sexpressions\sdoes\snot\nwork.\s\sIt\sgets\san\sincorrect\sanswer\sfor\sthe\stest\scase\sshown\sin\sthe\sticket.
+D 2022-11-23T17:56:00.136
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -591,7 +591,7 @@ F src/date.c 94ce83b4cd848a387680a5f920c9018c16655db778c4d36525af0a0f34679ac5
 F src/dbpage.c f1a87f4ebcf22284e0aaf0697862f4ccfc120dcd6db3d8dfa3b049b2580c01d8
 F src/dbstat.c 861e08690fcb0f2ee1165eff0060ea8d4f3e2ea10f80dab7d32ad70443a6ff2d
 F src/delete.c 86573edae75e3d3e9a8b590d87db8e47222103029df4f3e11fa56044459b514e
-F src/expr.c 63cce2c219748d8f8e00a30e4dc43d65b686b62489f154eebe892933bfb4a249
+F src/expr.c 527fc56468f75c380a34d38426cc1deece693a3cc6139fb45544923ff461d569
 F src/fault.c 460f3e55994363812d9d60844b2a6de88826e007
 F src/fkey.c 722f20779f5342a787922deded3628d8c74b5249cab04098cf17ee2f2aaff002
 F src/func.c 7e86074afc4dc702691a29b7801f6dcc191db092b52e8bbe69dcd2f7be52194d
@@ -641,12 +641,12 @@ F src/printf.c e99ee9741e79ae3873458146f59644276657340385ade4e76a5f5d1c25793764
 F src/random.c 606b00941a1d7dd09c381d3279a058d771f406c5213c9932bbd93d5587be4b9c
 F src/resolve.c efea4e5fbecfd6d0a9071b0be0d952620991673391b6ffaaf4c277b0bb674633
 F src/rowset.c ba9515a922af32abe1f7d39406b9d35730ed65efab9443dc5702693b60854c92
-F src/select.c 1d3f8a5b510f8e2859c7f800babb5a63802789f93c9c29498312b37655dac1b8
+F src/select.c 8ac7f73d40d5615cdb73660a85e05ee26943a618ec5c7262160994f263fa7178
 F src/shell.c.in 7d1705f139e6762e8c0fe254a8ebf3ab77aec6d8366f033cdd5f5ebadefbbb20
 F src/sqlite.h.in 100fc660c2f19961b8ed8437b9d53d687de2f8eb2b96437ec6da216adcb643ca
 F src/sqlite3.rc 5121c9e10c3964d5755191c80dd1180c122fc3a8
 F src/sqlite3ext.h c4b9fa7a7e2bcdf850cfeb4b8a91d5ec47b7a00033bc996fd2ee96cbf2741f5f
-F src/sqliteInt.h 894f7d98b1104fecb2ca2f457a4c598944250d9462eb433f12bd42b1080d525f
+F src/sqliteInt.h f4917d663170308601d794cdff7b8cf9704c4bafe03d7e926a156be835e29f29
 F src/sqliteLimit.h d7323ffea5208c6af2734574bae933ca8ed2ab728083caa117c9738581a31657
 F src/status.c 160c445d7d28c984a0eae38c144f6419311ed3eace59b44ac6dafc20db4af749
 F src/table.c 0f141b58a16de7e2fbe81c308379e7279f4c6b50eb08efeec5892794a0ba30d1
@@ -2059,8 +2059,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 f8932e04d4d18eb9d71edda15aa08af2eb139ff14d77ca147ea6e9b173e0f5e0
-R e49e6155bccbafba580b14baaea3db86
+P 23145fe999ff74d787a3999baedd4ffe755c5f1f1275082ed0338ba637ecc56e
+R f3df5c5148120a1bf76a131c870fdc09
 U drh
-Z b6500d1e8d8b2720badce4287d0e1594
+Z 97820e8b433aee285f0d7a5d438ada65
 # Remove this line to create a well-formed Fossil manifest.

diff --git a/manifest.uuid b/manifest.uuid
index f88bcea3d3677501e9b8f61767439e30d2ca056b..c7c9044f2ae65975cb19f168da5d0ecd05e710bb 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-23145fe999ff74d787a3999baedd4ffe755c5f1f1275082ed0338ba637ecc56e
\ No newline at end of file
+84c06023f4a1606664fdb9811312603b31f7c94a43d0e443ba7dde7fdba029e3
\ No newline at end of file

diff --git a/src/expr.c b/src/expr.c
index 42099536ed798575b44f531157e5269cc7cb11a3..db81983e4af761b00992ff263e28ab71752dad70 100644 (file)
--- a/src/expr.c
+++ b/src/expr.c
@@ -4135,7 +4135,7 @@ expr_code_doover:
                               pCol->iSorterColumn, target);
         if( pCol->iColumn<0 ){
           VdbeComment((v,"%s.rowid",pTab->zName));
-        }else if( ALWAYS(pTab!=0) ){
+        }else if( pTab!=0 ){
           VdbeComment((v,"%s.%s", 
               pTab->zName, pTab->aCol[pCol->iColumn].zCnName));
           if( pTab->aCol[pCol->iColumn].affinity==SQLITE_AFF_REAL ){
@@ -6241,6 +6241,71 @@ static int addAggInfoFunc(sqlite3 *db, AggInfo *pInfo){
   return i;
 }
 
+/*
+** Search the AggInfo object for an aCol[] entry that has iTable and iColumn.
+** Return the index in aCol[] of the entry that describes that column. 
+**
+** If no prior entry is found, create a new one and return -1.  The
+** new column will have an idex of pAggInfo->nColumn-1.
+*/
+static void findOrCreateAggInfoColumn(
+  Parse *pParse,       /* Parsing context */
+  AggInfo *pAggInfo,   /* The AggInfo object to search and/or modify */
+  Expr *pExpr          /* Expr describing the column to find or insert */
+){
+  struct AggInfo_col *pCol;
+  int k;
+
+  pCol = pAggInfo->aCol;
+  for(k=0; k<pAggInfo->nColumn; k++, pCol++){
+    if( pCol->iTable==pExpr->iTable
+     && pCol->iColumn==pExpr->iColumn
+     && pExpr->op!=TK_IF_NULL_ROW
+    ){
+      goto fix_up_expr;
+    }
+  }
+  k = addAggInfoColumn(pParse->db, pAggInfo);
+  if( k<0 ){
+    /* OOM on resize */
+    assert( pParse->db->mallocFailed );
+    return;
+  }
+  pCol = &pAggInfo->aCol[k];
+  assert( ExprUseYTab(pExpr) );
+  pCol->pTab = pExpr->y.pTab;
+  pCol->iTable = pExpr->iTable;
+  pCol->iColumn = pExpr->iColumn;
+  pCol->iSorterColumn = -1;
+  pCol->pCExpr = pExpr;
+  if( pAggInfo->pGroupBy && pExpr->op!=TK_IF_NULL_ROW ){
+    int j, n;
+    ExprList *pGB = pAggInfo->pGroupBy;
+    struct ExprList_item *pTerm = pGB->a;
+    n = pGB->nExpr;
+    for(j=0; j<n; j++, pTerm++){
+      Expr *pE = pTerm->pExpr;
+      if( pE->op==TK_COLUMN
+       && pE->iTable==pExpr->iTable
+       && pE->iColumn==pExpr->iColumn
+      ){
+        pCol->iSorterColumn = j;
+        break;
+      }
+    }
+  }
+  if( pCol->iSorterColumn<0 ){
+    pCol->iSorterColumn = pAggInfo->nSortingColumn++;
+  }
+fix_up_expr:
+  ExprSetVVAProperty(pExpr, EP_NoReduce);
+  pExpr->pAggInfo = pAggInfo;
+  if( pExpr->op==TK_COLUMN ){
+    pExpr->op = TK_AGG_COLUMN;
+  }
+  pExpr->iAgg = (i16)k;
+}
+
 /*
 ** This is the xExprCallback for a tree walker.  It is used to
 ** implement sqlite3ExprAnalyzeAggregates().  See sqlite3ExprAnalyzeAggregates
@@ -6255,6 +6320,37 @@ static int analyzeAggregate(Walker *pWalker, Expr *pExpr){
 
   assert( pNC->ncFlags & NC_UAggInfo );
   switch( pExpr->op ){
+    default: {
+      IndexedExpr *pIEpr;
+      Expr tmp;
+      if( (pNC->ncFlags & NC_InAggFunc)==0 ) break;
+      if( pParse->pIdxEpr==0 ) break;
+      for(pIEpr=pParse->pIdxEpr; pIEpr; pIEpr=pIEpr->pIENext){
+        int iDataCur = pIEpr->iDataCur;
+        if( iDataCur<0 ) continue;
+        if( pParse->iSelfTab ){
+          if( pIEpr->iDataCur!=pParse->iSelfTab-1 ) continue;
+          iDataCur = -1;
+        }
+        if( sqlite3ExprCompare(0, pExpr, pIEpr->pExpr, iDataCur)==0 ) break;
+      }
+      if( pIEpr==0 ) break;
+      if( !ExprUseYTab(pExpr) ) break;
+
+      /* If we reach this point, it means that expression pExpr can be
+      ** translated into a reference to an index column as described by
+      ** pIEpr.
+      */
+      memset(&tmp, 0, sizeof(tmp));
+      tmp.op = TK_AGG_COLUMN;
+      tmp.iTable = pIEpr->iIdxCur;
+      tmp.iColumn = pIEpr->iIdxCol;
+      findOrCreateAggInfoColumn(pParse, pAggInfo, &tmp);
+      pAggInfo->aCol[tmp.iAgg].pCExpr = pExpr;
+      pExpr->pAggInfo = pAggInfo;
+      pExpr->iAgg = tmp.iAgg;
+      return WRC_Prune;
+    }
     case TK_IF_NULL_ROW:
     case TK_AGG_COLUMN:
     case TK_COLUMN: {
@@ -6266,66 +6362,9 @@ static int analyzeAggregate(Walker *pWalker, Expr *pExpr){
       if( ALWAYS(pSrcList!=0) ){
         SrcItem *pItem = pSrcList->a;
         for(i=0; i<pSrcList->nSrc; i++, pItem++){
-          struct AggInfo_col *pCol;
           assert( !ExprHasProperty(pExpr, EP_TokenOnly|EP_Reduced) );
           if( pExpr->iTable==pItem->iCursor ){
-            /* If we reach this point, it means that pExpr refers to a table
-            ** that is in the FROM clause of the aggregate query.  
-            **
-            ** Make an entry for the column in pAggInfo->aCol[] if there
-            ** is not an entry there already.
-            */
-            int k;
-            pCol = pAggInfo->aCol;
-            for(k=0; k<pAggInfo->nColumn; k++, pCol++){
-              if( pCol->iTable==pExpr->iTable
-               && pCol->iColumn==pExpr->iColumn
-               && pExpr->op!=TK_IF_NULL_ROW
-              ){
-                break;
-              }
-            }
-            if( (k>=pAggInfo->nColumn)
-             && (k = addAggInfoColumn(pParse->db, pAggInfo))>=0 
-            ){
-              pCol = &pAggInfo->aCol[k];
-              assert( ExprUseYTab(pExpr) );
-              pCol->pTab = pExpr->y.pTab;
-              pCol->iTable = pExpr->iTable;
-              pCol->iColumn = pExpr->iColumn;
-              pCol->iSorterColumn = -1;
-              pCol->pCExpr = pExpr;
-              if( pAggInfo->pGroupBy && pExpr->op!=TK_IF_NULL_ROW ){
-                int j, n;
-                ExprList *pGB = pAggInfo->pGroupBy;
-                struct ExprList_item *pTerm = pGB->a;
-                n = pGB->nExpr;
-                for(j=0; j<n; j++, pTerm++){
-                  Expr *pE = pTerm->pExpr;
-                  if( pE->op==TK_COLUMN
-                   && pE->iTable==pExpr->iTable
-                   && pE->iColumn==pExpr->iColumn
-                  ){
-                    pCol->iSorterColumn = j;
-                    break;
-                  }
-                }
-              }
-              if( pCol->iSorterColumn<0 ){
-                pCol->iSorterColumn = pAggInfo->nSortingColumn++;
-              }
-            }
-            /* There is now an entry for pExpr in pAggInfo->aCol[] (either
-            ** because it was there before or because we just created it).
-            ** Convert the pExpr to be a TK_AGG_COLUMN referring to that
-            ** pAggInfo->aCol[] entry.
-            */
-            ExprSetVVAProperty(pExpr, EP_NoReduce);
-            pExpr->pAggInfo = pAggInfo;
-            if( pExpr->op==TK_COLUMN ){
-              pExpr->op = TK_AGG_COLUMN;
-            }
-            pExpr->iAgg = (i16)k;
+            findOrCreateAggInfoColumn(pParse, pAggInfo, pExpr);
             break;
           } /* endif pExpr->iTable==pItem->iCursor */
         } /* end loop over pSrcList */

diff --git a/src/select.c b/src/select.c
index 8f2517ad4f4a9b895be6d65afd8a3c987a69da43..49bef86bab253e7ac057370a20e54ac0e4c28b90 100644 (file)
--- a/src/select.c
+++ b/src/select.c
@@ -6281,11 +6281,22 @@ static void optimizeAggregateUseOfIndexedExpr(
   AggInfo *pAggInfo,      /* The aggregate info */
   NameContext *pNC        /* Name context used to resolve agg-func args */
 ){
+  pAggInfo->nColumn = pAggInfo->nAccumulator;
+  if( pAggInfo->nSortingColumn>0 ){
+    if( pAggInfo->nColumn==0 ){
+      pAggInfo->nSortingColumn = 0;
+    }else{
+      pAggInfo->nSortingColumn =
+        pAggInfo->aCol[pAggInfo->nColumn-1].iSorterColumn+1;
+    }
+  }
+  analyzeAggFuncArgs(pParse, pAggInfo, pNC);
 #if TREETRACE_ENABLED
-  if( sqlite3TreeTrace & 0x80000 ){
+  if( sqlite3TreeTrace & 0x20 ){
     IndexedExpr *pIEpr;
-    TREETRACE(0x80000, pParse, pSelect,
-        ("Attempting to optimize AggInfo for Indexed Exprs\n"));
+    TREETRACE(0x20, pParse, pSelect,
+        ("AggInfo (possibly) adjusted for Indexed Exprs\n"));
+    sqlite3TreeViewSelect(0, pSelect, 0);
     for(pIEpr=pParse->pIdxEpr; pIEpr; pIEpr=pIEpr->pIENext){
       printf("data-cursor=%d index={%d,%d}\n",
           pIEpr->iDataCur, pIEpr->iIdxCur, pIEpr->iIdxCol);
@@ -6296,8 +6307,6 @@ static void optimizeAggregateUseOfIndexedExpr(
 #else
   (void)pSelect;  /* Suppress unused-parameter warnings */
 #endif
-  pAggInfo->nColumn = pAggInfo->nAccumulator;
-  analyzeAggFuncArgs(pParse, pAggInfo, pNC);
 }
 
 /*
@@ -7959,7 +7968,7 @@ select_end:
   if( pAggInfo && !db->mallocFailed ){
     for(i=0; i<pAggInfo->nColumn; i++){
       Expr *pExpr = pAggInfo->aCol[i].pCExpr;
-      assert( pExpr!=0 );
+      if( pExpr==0 ) continue;
       assert( pExpr->pAggInfo==pAggInfo );
       assert( pExpr->iAgg==i );
     }

diff --git a/src/sqliteInt.h b/src/sqliteInt.h
index 74c997c6dd0e83e9cd9784dad356ab54f74f5471..3ecdb35feeb38136b45b08c3d59b229e5d42e454 100644 (file)
--- a/src/sqliteInt.h
+++ b/src/sqliteInt.h
@@ -1059,7 +1059,6 @@ extern u32 sqlite3TreeTrace;
 **   0x00010000     Beginning of DELETE/INSERT/UPDATE processing
 **   0x00020000     Transform DISTINCT into GROUP BY
 **   0x00040000     SELECT tree dump after all code has been generated
-**   0x00080000     Optimize Aggregates using indexed expressions
 */
 
 /*