From: "n.pettik" <korablev@tarantool.org> To: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Subject: [tarantool-patches] Re: [PATCH v2 3/3] sql: change collation compatibility rules Date: Wed, 14 Nov 2018 01:37:40 +0300 [thread overview] Message-ID: <1280E0A9-D06C-47DD-8425-CB734DEDA781@tarantool.org> (raw) In-Reply-To: <105845b9-019b-9b37-4812-898c0fa4d4be@tarantool.org> >> 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) > > 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’t 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 != 0) { > > 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’ve 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 != rhs_id) { - if (lhs_id == 0) { + if (lhs_id == COLL_NONE) { *res_id = rhs_id; return 0; } - if (rhs_id == 0) { + if (rhs_id == COLL_NONE) { *res_id = 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; if (pTab->def->fields[i].coll_id == COLL_NONE && - sql_expr_coll(pParse, p, &is_found, &coll_id) && coll_id != 0) + sql_expr_coll(pParse, p, &is_found, &coll_id) && + coll_id != COLL_NONE) pTab->def->fields[i].coll_id = coll_id; } } @@ -2172,7 +2173,7 @@ multi_select_coll_seq_r(struct Parse *parser, struct Select *p, int n, { bool is_prior_forced = false; bool is_current_forced; - uint32_t prior_coll_id = 0; + uint32_t prior_coll_id = COLL_NONE; uint32_t current_coll_id; if (p->pPrior != NULL) { prior_coll_id = 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 != 0) { + if (id != COLL_NONE) { /* * Neither X nor Y have COLLATE * operators, but X has ag
next prev parent reply other threads:[~2018-11-13 22:37 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-13 0:07 [tarantool-patches] [PATCH v2 0/3] Change collation compatibility rules according to ANSI SQL Nikita Pettik 2018-11-13 0:07 ` [tarantool-patches] [PATCH v2 1/3] sql: do not add explicit <COLLATE "BINARY"> clause Nikita Pettik 2018-11-13 0:07 ` [tarantool-patches] [PATCH v2 2/3] Introduce "none" and "binary" collations Nikita Pettik 2018-11-13 12:02 ` [tarantool-patches] " Vladislav Shpilevoy 2018-11-13 22:37 ` n.pettik 2018-11-14 11:52 ` Vladislav Shpilevoy 2018-11-14 20:59 ` n.pettik 2018-11-15 11:04 ` Vladislav Shpilevoy 2018-11-13 0:07 ` [tarantool-patches] [PATCH v2 3/3] sql: change collation compatibility rules Nikita Pettik 2018-11-13 12:02 ` [tarantool-patches] " Vladislav Shpilevoy 2018-11-13 22:37 ` n.pettik [this message] 2018-11-15 11:24 ` [tarantool-patches] Re: [PATCH v2 0/3] Change collation compatibility rules according to ANSI SQL Kirill Yukhin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1280E0A9-D06C-47DD-8425-CB734DEDA781@tarantool.org \ --to=korablev@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=v.shpilevoy@tarantool.org \ --subject='[tarantool-patches] Re: [PATCH v2 3/3] sql: change collation compatibility rules' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox