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 2F6D22D29F for ; Tue, 13 Nov 2018 17:37:47 -0500 (EST) 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 ZCakhrNoRKIj for ; Tue, 13 Nov 2018 17:37:47 -0500 (EST) Received: from smtp37.i.mail.ru (smtp37.i.mail.ru [94.100.177.97]) (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 763CC2D234 for ; Tue, 13 Nov 2018 17:37:46 -0500 (EST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.0 \(3445.100.39\)) Subject: [tarantool-patches] Re: [PATCH v2 3/3] sql: change collation compatibility rules From: "n.pettik" In-Reply-To: <105845b9-019b-9b37-4812-898c0fa4d4be@tarantool.org> Date: Wed, 14 Nov 2018 01:37:40 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <1280E0A9-D06C-47DD-8425-CB734DEDA781@tarantool.org> References: <105845b9-019b-9b37-4812-898c0fa4d4be@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: Vladislav Shpilevoy >> diff --git a/src/box/sql/select.c b/src/box/sql/select.c >> index cea453f08..ab0376a1d 100644 >> --- a/src/box/sql/select.c >> +++ b/src/box/sql/select.c >> @@ -2150,47 +2151,51 @@ computeLimitRegisters(Parse * pParse, Select = * p, int iBreak) >> } >> #ifndef SQLITE_OMIT_COMPOUND_SELECT >> -static struct coll * >> -multi_select_coll_seq_r(Parse *parser, Select *p, int n, bool = *is_found, >> - uint32_t *coll_id) >> +/** >> + * This function determines resulting collation sequence for >> + * @n-th column of the result set for the compound SELECT >> + * statement. Since compound SELECT performs implicit comparisons >> + * between values, all parts of compound queries must use >> + * the same collation. Otherwise, an error is raised. >> + * >> + * @param parser Parse context. >> + * @param p Select meta-information. >> + * @param n Column number of the result set. >> + * @param is_forced_coll Used if we fall into recursion. >> + * For most-outer call it is unused. Used to indicate that >> + * explicit COLLATE clause is used. >> + * @retval Id of collation to be used during string comparison. >> + */ >> +static uint32_t >> +multi_select_coll_seq(struct Parse *parser, struct Select *p, int n, >> + bool *is_forced_coll) >=20 > 1. I renamed this function into multi_select_coll_seq_r and > created a wrapper without is_forced_coll to remove some of > 'bool unused' things. Apply or drop the fix, up to you. Ok, I don=E2=80=99t mind your fixes. Applied. >> diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c >> index 9fa6ce15d..e05967554 100644 >> --- a/src/box/sql/whereexpr.c >> +++ b/src/box/sql/whereexpr.c >> @@ -168,7 +168,7 @@ exprCommute(Parse * pParse, Expr * pExpr) >> bool is_found; >> uint32_t id; >> sql_expr_coll(pParse, pExpr->pLeft, &is_found, = &id); >> - if (is_found) { >> + if (id !=3D 0) { >=20 > 2. If you fix a comment about COLL_NONE constant return, > do not forget to pick this hunk up as well, please. Maybe > this patch contains more, I am not sure. Ok, I=E2=80=99ve returned COLL_NONE usages: diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 09c248154..b67b22c23 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -360,11 +360,11 @@ collations_check_compatibility(uint32_t lhs_id, = bool is_lhs_forced, return 0; } if (lhs_id !=3D rhs_id) { - if (lhs_id =3D=3D 0) { + if (lhs_id =3D=3D COLL_NONE) { *res_id =3D rhs_id; return 0; } - if (rhs_id =3D=3D 0) { + if (rhs_id =3D=3D COLL_NONE) { *res_id =3D lhs_id; return 0; } diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 2db8e1c7f..efdac9dba 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -1963,7 +1963,8 @@ sqlite3SelectAddColumnTypeAndCollation(Parse * = pParse, /* Parsing contexts */ uint32_t coll_id; =20 if (pTab->def->fields[i].coll_id =3D=3D COLL_NONE && - sql_expr_coll(pParse, p, &is_found, &coll_id) && = coll_id !=3D 0) + sql_expr_coll(pParse, p, &is_found, &coll_id) && + coll_id !=3D COLL_NONE) pTab->def->fields[i].coll_id =3D coll_id; } } @@ -2172,7 +2173,7 @@ multi_select_coll_seq_r(struct Parse *parser, = struct Select *p, int n, { bool is_prior_forced =3D false; bool is_current_forced; - uint32_t prior_coll_id =3D 0; + uint32_t prior_coll_id =3D COLL_NONE; uint32_t current_coll_id; if (p->pPrior !=3D NULL) { prior_coll_id =3D multi_select_coll_seq_r(parser, = p->pPrior, n, diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c index e05967554..971f428b7 100644 --- a/src/box/sql/whereexpr.c +++ b/src/box/sql/whereexpr.c @@ -168,7 +168,7 @@ exprCommute(Parse * pParse, Expr * pExpr) bool is_found; uint32_t id; sql_expr_coll(pParse, pExpr->pLeft, &is_found, = &id); - if (id !=3D 0) { + if (id !=3D COLL_NONE) { /* * Neither X nor Y have COLLATE * operators, but X has ag