From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 72C8D2B672 for ; Mon, 1 Oct 2018 12:38:02 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 0AAeh2PpUNlH for ; Mon, 1 Oct 2018 12:38:02 -0400 (EDT) Received: from smtp44.i.mail.ru (smtp44.i.mail.ru [94.100.177.104]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 33B762B655 for ; Mon, 1 Oct 2018 12:38:02 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: support HAVING without GROUP BY clause From: "n.pettik" In-Reply-To: <1bbae1dd-7fa1-92e8-9ed5-cf6b1d5cf6fd@tarantool.org> Date: Mon, 1 Oct 2018 19:37:56 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <16D5B1D2-2DBF-4BAE-9C5D-F23CCCE3723D@tarantool.org> <5f602cc5-9617-6717-c13a-4d5595c23422@tarantool.org> <99A1FBB4-3831-4BB6-A1D6-505D09F792AD@tarantool.org> <077750b5-7433-6122-5df9-caa6ff462e70@tarantool.org> <00C60996-AB0B-4157-AF8D-DCD9B1F63D8D@tarantool.org> <1bbae1dd-7fa1-92e8-9ed5-cf6b1d5cf6fd@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Kirill Shcherbatov >> Still, other DBs (as usual I checked - Postgres, Oracle and DB2) = process this >> query as well: >>=20 >> select 1 from t1 having min(a) > 0; >>=20 >> In other words, not only aggregate can appear in SELECT args, >> but also constant literals. Semantically, it seems to be correct. >> Moreover, quieres like >>=20 >> select date() from t1 having min(a) > 0; >>=20 >> are valid too. What I mean is SELECT arguments must return >> single value for all rows in table (i.e. be single-group). >>=20 >> =46rom 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. > This cases are totally useless, not so trivial to implement (need to = calculate > expression in VDBE, e.g. > SELECT (SELECT s1 FROM te40 LIMIT 1) FROM te40 HAVING min(s1) > 0 > is a valid construction for Postgress. > Moreover, current checks are the part of resolveSelectStep that=20 >=20 > Asked Peter, does it really important. I would like to notice that we=E2=80=99ve decided to allow literals and = constant functions also appear in select arg list. =46rom new diff: diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index 9a2d6ff4e..7c0d9b44e 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -602,6 +602,8 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) /* A lone identifier is the name of a column. */ case TK_ID:{ + if ((pNC->ncFlags & NC_AllowAgg) !=3D 0) + pNC->ncFlags |=3D NC_HasUnaggregatedId; return lookupName(pParse, 0, pExpr->u.zToken, = pNC, pExpr); } @@ -1180,6 +1182,44 @@ resolveOrderGroupBy(NameContext * pNC, /* The = name context of the SELECT stateme return sqlite3ResolveOrderGroupBy(pParse, pSelect, pOrderBy, = zType); } =20 +/** + * Test if specified expression is a regular literal. + * @param expr Expression to analyse. + * @retval true If expression is literal. + * @retval false Otherwise. + */ +static bool +sql_expr_is_literal(struct Expr *expr) +{ + return expr->op =3D=3D TK_INTEGER || expr->op =3D=3D TK_FLOAT || + expr->op =3D=3D TK_BLOB || expr->op =3D=3D TK_STRING; +} What about TK_NULL? What about unary minus? This examples now fail: tarantool> SELECT -1 FROM te40 HAVING SUM(s1) > 0; --- - error: 'SQL error: HAVING argument must appear in the GROUP BY clause = or be used in an aggregate function=E2=80=99 =E2=80=A6 tarantool> SELECT NULL FROM te40 HAVING SUM(s1) > 0; --- - error: 'SQL error: HAVING argument must appear in the GROUP BY clause = or be used in an aggregate function' ... + +/** + * Test if specified expression is a constant function. + * @param parser Parsing context. + * @param expr Expression to analyse. + * @retval true If expression is a existent constant function. + * @retval false Otherwise. + */ +static bool +sql_expr_is_constat_func(struct Parse *parser, struct Expr *expr) +{ + if (expr->op !=3D TK_FUNCTION) + return false; + char *func_name =3D expr->u.zToken; + struct ExprList *args_list =3D expr->x.pList; + int args_count =3D args_list !=3D NULL ? args_list->nExpr : 0; + struct FuncDef *func =3D + sqlite3FindFunction(parser->db, func_name, args_count, = 0); + if (func =3D=3D NULL) + func =3D sqlite3FindFunction(parser->db, func_name, -2, = 0); + if (func =3D=3D NULL) + return false; + return func !=3D NULL ? + (func->funcFlags & SQLITE_FUNC_CONSTANT) !=3D 0 : false; +} + Consider refactoring: at last return =E2=80=98func=E2=80=99 can=E2=80=99t = be NULL - this check is performed one line above. @@ -1203,7 +1204,7 @@ sql_expr_is_literal(struct Expr *expr) * @retval false Otherwise. */ static bool -sql_expr_is_constat_func(struct Parse *parser, struct Expr *expr) +sql_expr_is_constant_func(struct Parse *parser, struct Expr *expr) { if (expr->op !=3D TK_FUNCTION) return false; @@ -1212,12 +1213,17 @@ sql_expr_is_constat_func(struct Parse *parser, = struct Expr *expr) int args_count =3D args_list !=3D NULL ? args_list->nExpr : 0; struct FuncDef *func =3D sqlite3FindFunction(parser->db, func_name, args_count, = 0); - if (func =3D=3D NULL) - func =3D sqlite3FindFunction(parser->db, func_name, -2, = 0); - if (func =3D=3D NULL) - return false; - return func !=3D NULL ? - (func->funcFlags & SQLITE_FUNC_CONSTANT) !=3D 0 : false; + if (func =3D=3D NULL) { + /* + * If we fail to find function with exact number + * of arguments, lets try to search similar + * function but with different number of args. + */ + func =3D sqlite3FindFunction(parser->db, func_name, -2, = 0); + if (func =3D=3D NULL) + return false; + } + return (func->funcFlags & SQLITE_FUNC_CONSTANT) !=3D 0; } =20 /* @@ -1338,8 +1344,8 @@ resolveSelectStep(Walker * pWalker, Select * p) return WRC_Abort; is_all_select_agg &=3D (sNC.ncFlags & NC_HasAgg) = !=3D 0 || sql_expr_is_literal(expr) = || - = sql_expr_is_constat_func(pParse, - = expr); + = sql_expr_is_constant_func(pParse, + = expr);