[tarantool-patches] Re: [PATCH v1 1/1] sql: support HAVING without GROUP BY clause

[n.pettik]

Still, other DBs (as usual I checked - Postgres, Oracle and DB2) process this
query as well:

 select 1 from t1 having min(a) > 0;

In other words, not only aggregate can appear in SELECT args,
but also constant literals. Semantically, it seems to be correct.
Moreover, quieres like

select date() from t1 having min(a) > 0;

are valid too. What I mean is SELECT arguments must return
single value for all rows in table (i.e. be single-group).

From the first sight, this problem is likely to be minor, but I guess
we should implement correct behaviour as far as we are dealing
with this issue right now.

> diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
> index 280ecd9..c38374d 100644
> --- a/src/box/sql/resolve.c
> +++ b/src/box/sql/resolve.c
> @@ -600,6 +600,8 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
> 		/* A lone identifier is the name of a column.
> 		 */
> 	case TK_ID:{
> +			if ((pNC->ncFlags & NC_AllowAgg) != 0)
> +				pNC->ncFlags |= NC_HasUnaggregatedId;
> 			return lookupName(pParse, 0, pExpr->u.zToken, pNC,
> 					  pExpr);
> 		}
> @@ -1283,13 +1285,19 @@ resolveSelectStep(Walker * pWalker, Select * p)
> 		/* Set up the local name-context to pass to sqlite3ResolveExprNames() to
> 		 * resolve the result-set expression list.
> 		 */
> +		bool all_select_agg = true;

Nit: use ‘is_’ or ‘if_’ prefix for predicate variable.

> 		sNC.ncFlags = NC_AllowAgg;
> 		sNC.pSrcList = p->pSrc;
> 		sNC.pNext = pOuterNC;
> -
> 		/* Resolve names in the result set. */
> -		if (sqlite3ResolveExprListNames(sNC, p->pEList))
> -			return WRC_Abort;

Leave here explanation comment pls. Without any notion it is
quite hard to understand that this snippet helps to check
HAVING without GROUP BY queries.

> +		for (i = 0; i < p->pEList->nExpr; i++) {
> +			u16 has_agg_flag = sNC.ncFlags & NC_HasAgg;
> +			sNC.ncFlags &= ~NC_HasAgg;
> +			if (sqlite3ResolveExprNames(&sNC, p->pEList->a[i].pExpr))
> +				return WRC_Abort;
> +			all_select_agg &= (sNC.ncFlags & NC_HasAgg) != 0;
> +			sNC.ncFlags |= has_agg_flag;
> +		}