From: "n.pettik" <korablev@tarantool.org> To: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Subject: [tarantool-patches] Re: [PATCH 2/2] sql: compute resulting collation for concatenation Date: Fri, 22 Feb 2019 16:40:58 +0300 [thread overview] Message-ID: <C95EB7A1-F88D-40C6-9726-207E81B01462@tarantool.org> (raw) In-Reply-To: <57ca444c-96d3-bd85-9137-26d408200d1b@tarantool.org> >>>> + bool is_rhs_forced; >>>> + uint32_t rhs_coll_id; >>>> + if (sql_expr_coll(parse, p->pRight, &is_rhs_forced, >>>> + &rhs_coll_id) != 0) >>>> + return -1; >>>> + if (is_lhs_forced && is_rhs_forced) { >>>> + if (lhs_coll_id != rhs_coll_id) >>>> + return -1; >>> >>> 5. Did you miss diag_set? >> No, I did it on purpose. Firstly, this function is recursive, >> so in case error occurred on bottom levels of recursion, >> diag would be reseted on each level above (and parser’s >> counter of errors would be incremented several times as >> well). No terrible consequences would take place in this case, >> but it looks like a bad pattern. Anyway, fixed it according to >> your suggestion. > > Each ClientError is accounted in a global errors counter. It > means, that the same error should be set multiple times, otherwise > the statistics would be wrong. Instead of resetting diag each time > check if parser->nErr > 0, and if it is, then do nothing. > diag_is_empty() can not be used, because it happens sometimes, that > there are no errors, but diag is not cleared from a previous error. Hmm, Mergen is trying to remove nErr, if I’m not mistaken. So, soon I guess we will be able to use diag_is_empty(). Now I’ve implemented your suggestion. Diff: diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 680075792..0de9a46a5 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -246,10 +246,17 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id, return -1; if (is_lhs_forced && is_rhs_forced) { if (lhs_coll_id != rhs_coll_id) { - diag_set(ClientError, - ER_ILLEGAL_COLLATION_MIX); - parse->nErr++; - parse->rc = SQL_TARANTOOL_ERROR; + /* + * Don't set the same error + * several times: this + * function is recursive. + */ + if (parse->nErr == 0) { + diag_set(ClientError, + ER_ILLEGAL_COLLATION_MIX); + parse->nErr++; + parse->rc = SQL_TARANTOOL_ERROR; + } return -1; } }
next prev parent reply other threads:[~2019-02-22 13:41 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-01-16 13:13 [tarantool-patches] [PATCH 0/2] Compute derived " Nikita Pettik 2019-01-16 13:13 ` [tarantool-patches] [PATCH 1/2] sql: refactor sql_expr_coll and sql_binary_compare_coll_seq functions Nikita Pettik 2019-01-17 13:28 ` [tarantool-patches] " Konstantin Osipov 2019-01-24 18:28 ` Vladislav Shpilevoy 2019-02-14 23:26 ` n.pettik 2019-01-16 13:13 ` [tarantool-patches] [PATCH 2/2] sql: compute resulting collation for concatenation Nikita Pettik 2019-01-17 13:33 ` [tarantool-patches] " Konstantin Osipov 2019-01-17 19:19 ` n.pettik 2019-01-18 9:59 ` Kirill Yukhin 2019-01-24 18:29 ` Vladislav Shpilevoy 2019-02-14 23:26 ` n.pettik 2019-02-22 11:23 ` Vladislav Shpilevoy 2019-02-22 13:40 ` n.pettik [this message] 2019-02-22 13:51 ` Vladislav Shpilevoy 2019-02-25 11:29 ` [tarantool-patches] Re: [PATCH 0/2] Compute derived " 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=C95EB7A1-F88D-40C6-9726-207E81B01462@tarantool.org \ --to=korablev@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=v.shpilevoy@tarantool.org \ --subject='[tarantool-patches] Re: [PATCH 2/2] sql: compute resulting collation for concatenation' \ /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