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 653F5290C6 for ; Tue, 26 Mar 2019 09:34:31 -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 tZ-v6qhLsQpW for ; Tue, 26 Mar 2019 09:34:31 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 A7570205F5 for ; Tue, 26 Mar 2019 09:34:30 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: [tarantool-patches] Re: [PATCH v4 8/8] sql: remove sqlErrorMsg() From: "n.pettik" In-Reply-To: <20190325184737.GA18346@tarantool.org> Date: Tue, 26 Mar 2019 16:34:26 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <2E9AC693-B13A-4FFD-8F0D-A3FA7AA442A3@tarantool.org> References: <99f4adc61352899a83a7097b965845bafaa968ef.1552494059.git.imeevma@gmail.com> <0D9B8996-D469-498B-8D6C-AFD90C6DD35E@tarantool.org> <20190325184737.GA18346@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: Imeev Mergen >> @@ -1039,8 +1036,8 @@ resolveCompoundOrderBy(Parse * pParse, /* = Parsing context. Leave error messages >> const char *err =3D "Error at ORDER BY in = place %d: "\ >> "term does not match any = column in "\ >> "the result set"; >> - err =3D tt_sprintf(err, i + 1); >> - diag_set(ClientError, ER_SQL_PARSER_GENERIC, = err); >> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, >> + tt_sprintf(err, i + 1)); >>=20 >>> @@ -1444,10 +1469,37 @@ resolveSelectStep(Walker * pWalker, Select * = p) >>> * number of expressions in the select list. >>> */ >>> if (p->pNext && p->pEList->nExpr !=3D p->pNext->pEList->nExpr) { >>> - sqlSelectWrongNumTermsError(pParse, p->pNext); >>> + if (p->pNext->selFlags & SF_Values) { >>> + diag_set(ClientError, ER_SQL_PARSER_GENERIC, >>> + "all VALUES must have the same "\ >>> + "number of terms"); >>> + } else { >>> + const char *err_msg =3D >>> + "SELECTs to the left and right of %s "\ >>> + "do not have the same number of "\ >>> + "result columns"; >>> + switch (p->pNext->op) { >>> + case TK_ALL: >>> + err_msg =3D tt_sprintf(err_msg, >>> + "UNION ALL"); >>> + break; >>=20 >> Why don=E2=80=99t use selectOpName? >> Like it was in sqlSelectWrongNumTermsError(). >>=20 > Function selectOpName defined in different file and is static. > I think it isn't worth to share it for one error. I believe it is worth doing. For instance, you can return sqlSelectWrongNumTermsError() which was placed in select.c and called that static func. > Diff between versions: >=20 > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > index e26596a..1077285 100644 > --- a/src/box/sql/expr.c > +++ b/src/box/sql/expr.c > @@ -666,13 +666,7 @@ codeVectorCompare(Parse * pParse, /* Code = generator context */ > int regRight =3D 0; > u8 op =3D pExpr->op; > int addrDone =3D sqlVdbeMakeLabel(v); > - > - if (nLeft !=3D sqlExprVectorSize(pRight)) { > - diag_set(ClientError, ER_SQL_PARSER_GENERIC, > - "row value misused"); > - pParse->is_aborted =3D true; > - return; > - } Here the problem is that function is never called in our test suite: I placed assert(0); and no one test failed. Please, add at least simple tests like SELECT (1, 2) =3D=3D (1, 2) to make sure that codeVectorCompare() can be processed. Tests which you=E2=80=99ve added fails before this func is called. Added comment: diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 107728590..397f8209c 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -666,6 +666,11 @@ codeVectorCompare(Parse * pParse, /* Code = generator context */ int regRight =3D 0; u8 op =3D pExpr->op; int addrDone =3D sqlVdbeMakeLabel(v); + /* + * Situation when vectors have different dimensions is + * filtred way before - during expr resolution: + * see resolveExprStep(). + */ > + assert(nLeft =3D=3D sqlExprVectorSize(pRight)); > assert(pExpr->op =3D=3D TK_EQ || pExpr->op =3D=3D TK_NE > || pExpr->op =3D=3D TK_LT || pExpr->op =3D=3D TK_GT > || pExpr->op =3D=3D TK_LE || pExpr->op =3D=3D TK_GE); >=20 >=20 > @@ -4384,10 +4378,8 @@ sqlWhereBegin(Parse * pParse, /* The parser = context */ > * equal to pTabList->nSrc but might be shortened to 1 if the > * WHERE_OR_SUBCLAUSE flag is set. > */ > - for (ii =3D 0; ii < pTabList->nSrc; ii++) { > + for (ii =3D 0; ii < pTabList->nSrc; ii++) > createMask(pMaskSet, pTabList->a[ii].iCursor); > - sqlWhereTabFuncArgs(pParse, &pTabList->a[ii], = &pWInfo->sWC); Move removal of this func to a separate patch pls alongside with = mentions of table-valued funcs. It is not related to errors.