Index: src/expr.c ================================================================== --- src/expr.c +++ src/expr.c @@ -3121,13 +3121,16 @@ if( ExprHasAnyProperty(pExpr, EP_TokenOnly) ){ pFarg = 0; }else{ pFarg = pExpr->x.pList; } - sqlite3ExplainPrintf(pOut, "%sFUNCTION:%s(", - op==TK_AGG_FUNCTION ? "AGG_" : "", - pExpr->u.zToken); + if( op==TK_AGG_FUNCTION ){ + sqlite3ExplainPrintf(pOut, "AGG_FUNCTION%d:%s(", + pExpr->op2, pExpr->u.zToken); + }else{ + sqlite3ExplainPrintf(pOut, "FUNCTION:%s(", pExpr->u.zToken); + } if( pFarg ){ sqlite3ExplainExprList(pOut, pFarg); } sqlite3ExplainPrintf(pOut, ")"); break; @@ -3814,42 +3817,59 @@ } return 0; } /* -** This is the expression callback for sqlite3FunctionUsesOtherSrc(). -** -** Determine if an expression references any table other than one of the -** tables in pWalker->u.pSrcList and abort if it does. +** An instance of the following structure is used by the tree walker +** to count references to table columns in the arguments of an +** aggregate function, in order to implement the +** sqlite3FunctionThisSrc() routine. +*/ +struct SrcCount { + SrcList *pSrc; /* One particular FROM clause in a nested query */ + int nThis; /* Number of references to columns in pSrcList */ + int nOther; /* Number of references to columns in other FROM clauses */ +}; + +/* +** Count the number of references to columns. */ -static int exprUsesOtherSrc(Walker *pWalker, Expr *pExpr){ +static int exprSrcCount(Walker *pWalker, Expr *pExpr){ if( pExpr->op==TK_COLUMN || pExpr->op==TK_AGG_COLUMN ){ int i; - SrcList *pSrc = pWalker->u.pSrcList; + struct SrcCount *p = pWalker->u.pSrcCount; + SrcList *pSrc = p->pSrc; for(i=0; inSrc; i++){ - if( pExpr->iTable==pSrc->a[i].iCursor ) return WRC_Continue; - } - return WRC_Abort; - }else{ - return WRC_Continue; - } -} - -/* -** Determine if any of the arguments to the pExpr Function references -** any SrcList other than pSrcList. Return true if they do. Return -** false if pExpr has no argument or has only constant arguments or -** only references tables named in pSrcList. -*/ -static int sqlite3FunctionUsesOtherSrc(Expr *pExpr, SrcList *pSrcList){ + if( pExpr->iTable==pSrc->a[i].iCursor ) break; + } + if( inSrc ){ + p->nThis++; + }else{ + p->nOther++; + } + } + return WRC_Continue; +} + +/* +** Determine if any of the arguments to the pExpr Function reference +** pSrcList. Return true if they do. Also return true if the function +** has no arguments or has only constant arguments. Return false if pExpr +** references columns but not columns of tables found in pSrcList. +*/ +int sqlite3FunctionUsesThisSrc(Expr *pExpr, SrcList *pSrcList){ Walker w; + struct SrcCount cnt; assert( pExpr->op==TK_AGG_FUNCTION ); memset(&w, 0, sizeof(w)); - w.xExprCallback = exprUsesOtherSrc; - w.u.pSrcList = pSrcList; - if( sqlite3WalkExprList(&w, pExpr->x.pList)!=WRC_Continue ) return 1; - return 0; + w.xExprCallback = exprSrcCount; + w.u.pSrcCount = &cnt; + cnt.pSrc = pSrcList; + cnt.nThis = 0; + cnt.nOther = 0; + sqlite3WalkExprList(&w, pExpr->x.pList); + return cnt.nThis>0 || cnt.nOther==0; } /* ** Add a new element to the pAggInfo->aCol[] array. Return the index of ** the new element. Return a negative number if malloc fails. @@ -3964,11 +3984,11 @@ } return WRC_Prune; } case TK_AGG_FUNCTION: { if( (pNC->ncFlags & NC_InAggFunc)==0 - && !sqlite3FunctionUsesOtherSrc(pExpr, pSrcList) + && pWalker->walkerDepth==pExpr->op2 ){ /* Check to see if pExpr is a duplicate of another aggregate ** function that is already in the pAggInfo structure */ struct AggInfo_func *pItem = pAggInfo->aFunc; Index: src/resolve.c ================================================================== --- src/resolve.c +++ src/resolve.c @@ -15,10 +15,33 @@ ** table and column. */ #include "sqliteInt.h" #include #include + +/* +** Walk the expression tree pExpr and increase the aggregate function +** depth (the Expr.op2 field) by N on every TK_AGG_FUNCTION node. +** This needs to occur when copying a TK_AGG_FUNCTION node from an +** outer query into an inner subquery. +** +** incrAggFunctionDepth(pExpr,n) is the main routine. incrAggDepth(..) +** is a helper function - a callback for the tree walker. +*/ +static int incrAggDepth(Walker *pWalker, Expr *pExpr){ + if( pExpr->op==TK_AGG_FUNCTION ) pExpr->op2 += pWalker->u.i; + return WRC_Continue; +} +static void incrAggFunctionDepth(Expr *pExpr, int N){ + if( N>0 ){ + Walker w; + memset(&w, 0, sizeof(w)); + w.xExprCallback = incrAggDepth; + w.u.i = N; + sqlite3WalkExpr(&w, pExpr); + } +} /* ** Turn the pExpr expression into an alias for the iCol-th column of the ** result set in pEList. ** @@ -42,17 +65,24 @@ ** SELECT random()%5 AS x, count(*) FROM tab GROUP BY random()%5 ** ** The result of random()%5 in the GROUP BY clause is probably different ** from the result in the result-set. We might fix this someday. Or ** then again, we might not... +** +** The nSubquery parameter specifies how many levels of subquery the +** alias is removed from the original expression. The usually value is +** zero but it might be more if the alias is contained within a subquery +** of the original expression. The Expr.op2 field of TK_AGG_FUNCTION +** structures must be increased by the nSubquery amount. */ static void resolveAlias( Parse *pParse, /* Parsing context */ ExprList *pEList, /* A result set */ int iCol, /* A column in the result set. 0..pEList->nExpr-1 */ Expr *pExpr, /* Transform this into an alias to the result set */ - const char *zType /* "GROUP" or "ORDER" or "" */ + const char *zType, /* "GROUP" or "ORDER" or "" */ + int nSubquery /* Number of subqueries that the label is moving */ ){ Expr *pOrig; /* The iCol-th column of the result set */ Expr *pDup; /* Copy of pOrig */ sqlite3 *db; /* The database connection */ @@ -61,10 +91,11 @@ assert( pOrig!=0 ); assert( pOrig->flags & EP_Resolved ); db = pParse->db; if( pOrig->op!=TK_COLUMN && zType[0]!='G' ){ pDup = sqlite3ExprDup(db, pOrig, 0); + incrAggFunctionDepth(pDup, nSubquery); pDup = sqlite3PExpr(pParse, TK_AS, pDup, 0, 0); if( pDup==0 ) return; if( pEList->a[iCol].iAlias==0 ){ pEList->a[iCol].iAlias = (u16)(++pParse->nAlias); } @@ -149,13 +180,14 @@ const char *zTab, /* Name of table containing column, or NULL */ const char *zCol, /* Name of the column. */ NameContext *pNC, /* The name context used to resolve the name */ Expr *pExpr /* Make this EXPR node point to the selected column */ ){ - int i, j; /* Loop counters */ + int i, j; /* Loop counters */ int cnt = 0; /* Number of matching column names */ int cntTab = 0; /* Number of matching table names */ + int nSubquery = 0; /* How many levels of subquery */ sqlite3 *db = pParse->db; /* The database connection */ struct SrcList_item *pItem; /* Use for looping over pSrcList items */ struct SrcList_item *pMatch = 0; /* The matching pSrcList item */ NameContext *pTopNC = pNC; /* First namecontext in the list */ Schema *pSchema = 0; /* Schema of the expression */ @@ -313,11 +345,11 @@ pOrig = pEList->a[j].pExpr; if( (pNC->ncFlags&NC_AllowAgg)==0 && ExprHasProperty(pOrig, EP_Agg) ){ sqlite3ErrorMsg(pParse, "misuse of aliased aggregate %s", zAs); return WRC_Abort; } - resolveAlias(pParse, pEList, j, pExpr, ""); + resolveAlias(pParse, pEList, j, pExpr, "", nSubquery); cnt = 1; pMatch = 0; assert( zTab==0 && zDb==0 ); goto lookupname_end; } @@ -327,10 +359,11 @@ /* Advance to the next name context. The loop will exit when either ** we have a match (cnt>0) or when we run out of name contexts. */ if( cnt==0 ){ pNC = pNC->pNext; + nSubquery++; } } /* ** If X and Y are NULL (in other words if only the column name Z is @@ -566,17 +599,23 @@ }else if( wrong_num_args ){ sqlite3ErrorMsg(pParse,"wrong number of arguments to function %.*s()", nId, zId); pNC->nErr++; } - if( is_agg ){ - pExpr->op = TK_AGG_FUNCTION; - pNC->ncFlags |= NC_HasAgg; - } if( is_agg ) pNC->ncFlags &= ~NC_AllowAgg; sqlite3WalkExprList(pWalker, pList); - if( is_agg ) pNC->ncFlags |= NC_AllowAgg; + if( is_agg ){ + NameContext *pNC2 = pNC; + pExpr->op = TK_AGG_FUNCTION; + pExpr->op2 = 0; + while( pNC2 && !sqlite3FunctionUsesThisSrc(pExpr, pNC2->pSrcList) ){ + pExpr->op2++; + pNC2 = pNC2->pNext; + } + if( pNC2 ) pNC2->ncFlags |= NC_HasAgg; + pNC->ncFlags |= NC_AllowAgg; + } /* FIX ME: Compute pExpr->affinity based on the expected return ** type of the function */ return WRC_Prune; } @@ -851,11 +890,11 @@ if( pItem->iOrderByCol ){ if( pItem->iOrderByCol>pEList->nExpr ){ resolveOutOfRangeError(pParse, zType, i+1, pEList->nExpr); return 1; } - resolveAlias(pParse, pEList, pItem->iOrderByCol-1, pItem->pExpr, zType); + resolveAlias(pParse, pEList, pItem->iOrderByCol-1, pItem->pExpr, zType,0); } } return 0; } Index: src/select.c ================================================================== --- src/select.c +++ src/select.c @@ -3519,11 +3519,11 @@ #endif } /* -** This routine sets of a SELECT statement for processing. The +** This routine sets up a SELECT statement for processing. The ** following is accomplished: ** ** * VDBE Cursor numbers are assigned to all FROM-clause terms. ** * Ephemeral Table objects are created for all FROM-clause subqueries. ** * ON and USING clauses are shifted into WHERE statements @@ -3551,11 +3551,12 @@ /* ** Reset the aggregate accumulator. ** ** The aggregate accumulator is a set of memory cells that hold ** intermediate results while calculating an aggregate. This -** routine simply stores NULLs in all of those memory cells. +** routine generates code that stores NULLs in all of those memory +** cells. */ static void resetAccumulator(Parse *pParse, AggInfo *pAggInfo){ Vdbe *v = pParse->pVdbe; int i; struct AggInfo_func *pFunc; Index: src/sqliteInt.h ================================================================== --- src/sqliteInt.h +++ src/sqliteInt.h @@ -1688,12 +1688,13 @@ ynVar iColumn; /* TK_COLUMN: column index. -1 for rowid. ** TK_VARIABLE: variable number (always >= 1). */ i16 iAgg; /* Which entry in pAggInfo->aCol[] or ->aFunc[] */ i16 iRightJoinTable; /* If EP_FromJoin, the right table of the join */ u8 flags2; /* Second set of flags. EP2_... */ - u8 op2; /* If a TK_REGISTER, the original value of Expr.op */ - /* If TK_COLUMN, the value of p5 for OP_Column */ + u8 op2; /* TK_REGISTER: original value of Expr.op + ** TK_COLUMN: the value of p5 for OP_Column + ** TK_AGG_FUNCTION: nesting depth */ AggInfo *pAggInfo; /* Used by TK_AGG_COLUMN and TK_AGG_FUNCTION */ Table *pTab; /* Table for TK_COLUMN expressions. */ #if SQLITE_MAX_EXPR_DEPTH>0 int nHeight; /* Height of the tree headed by this node */ #endif @@ -2496,14 +2497,16 @@ */ struct Walker { int (*xExprCallback)(Walker*, Expr*); /* Callback for expressions */ int (*xSelectCallback)(Walker*,Select*); /* Callback for SELECTs */ Parse *pParse; /* Parser context. */ + int walkerDepth; /* Number of subqueries */ union { /* Extra data for callback */ NameContext *pNC; /* Naming context */ int i; /* Integer value */ SrcList *pSrcList; /* FROM clause */ + struct SrcCount *pSrcCount; /* Counting column references */ } u; }; /* Forward declarations */ int sqlite3WalkExpr(Walker*, Expr*); @@ -2833,10 +2836,11 @@ char *sqlite3NameFromToken(sqlite3*, Token*); int sqlite3ExprCompare(Expr*, Expr*); int sqlite3ExprListCompare(ExprList*, ExprList*); void sqlite3ExprAnalyzeAggregates(NameContext*, Expr*); void sqlite3ExprAnalyzeAggList(NameContext*,ExprList*); +int sqlite3FunctionUsesThisSrc(Expr*, SrcList*); Vdbe *sqlite3GetVdbe(Parse*); void sqlite3PrngSaveState(void); void sqlite3PrngRestoreState(void); void sqlite3PrngResetState(void); void sqlite3RollbackAll(sqlite3*,int); Index: src/walker.c ================================================================== --- src/walker.c +++ src/walker.c @@ -123,14 +123,20 @@ */ int sqlite3WalkSelect(Walker *pWalker, Select *p){ int rc; if( p==0 || pWalker->xSelectCallback==0 ) return WRC_Continue; rc = WRC_Continue; - while( p ){ + pWalker->walkerDepth++; + while( p ){ rc = pWalker->xSelectCallback(pWalker, p); if( rc ) break; - if( sqlite3WalkSelectExpr(pWalker, p) ) return WRC_Abort; - if( sqlite3WalkSelectFrom(pWalker, p) ) return WRC_Abort; + if( sqlite3WalkSelectExpr(pWalker, p) + || sqlite3WalkSelectFrom(pWalker, p) + ){ + pWalker->walkerDepth--; + return WRC_Abort; + } p = p->pPrior; } + pWalker->walkerDepth--; return rc & WRC_Abort; } ADDED test/aggnested.test Index: test/aggnested.test ================================================================== --- /dev/null +++ test/aggnested.test @@ -0,0 +1,71 @@ +# 2012 August 23 +# +# 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 implements tests for processing aggregate queries with +# subqueries in which the subqueries hold the aggregate functions +# or in which the subqueries are themselves aggregate queries +# + +set testdir [file dirname $argv0] +source $testdir/tester.tcl + +do_test aggnested-1.1 { + db eval { + CREATE TABLE t1(a1 INTEGER); + INSERT INTO t1 VALUES(1), (2), (3); + CREATE TABLE t2(b1 INTEGER); + INSERT INTO t2 VALUES(4), (5); + SELECT (SELECT group_concat(a1,'x') FROM t2) FROM t1; + } +} {1x2x3} +do_test aggnested-1.2 { + db eval { + SELECT + (SELECT group_concat(a1,'x') || '-' || group_concat(b1,'y') FROM t2) + FROM t1; + } +} {1x2x3-4y5} +do_test aggnested-1.3 { + db eval { + SELECT (SELECT group_concat(b1,a1) FROM t2) FROM t1; + } +} {415 425 435} +do_test aggnested-1.4 { + db eval { + SELECT (SELECT group_concat(a1,b1) FROM t2) FROM t1; + } +} {151 252 353} + + +# This test case is a copy of the one in +# http://www.mail-archive.com/sqlite-users@sqlite.org/msg70787.html +# +do_test aggnested-2.0 { + sqlite3 db2 :memory: + db2 eval { + CREATE TABLE t1 (A1 INTEGER NOT NULL,A2 INTEGER NOT NULL,A3 INTEGER NOT + NULL,A4 INTEGER NOT NULL,PRIMARY KEY(A1)); + REPLACE INTO t1 VALUES(1,11,111,1111); + REPLACE INTO t1 VALUES(2,22,222,2222); + REPLACE INTO t1 VALUES(3,33,333,3333); + CREATE TABLE t2 (B1 INTEGER NOT NULL,B2 INTEGER NOT NULL,B3 INTEGER NOT + NULL,B4 INTEGER NOT NULL,PRIMARY KEY(B1)); + REPLACE INTO t2 VALUES(1,88,888,8888); + REPLACE INTO t2 VALUES(2,99,999,9999); + SELECT (SELECT GROUP_CONCAT(CASE WHEN a1=1 THEN'A' ELSE 'B' END) FROM t2), + t1.* + FROM t1; + } +} {A,B,B 3 33 333 3333} +db2 close + +finish_test