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 A19CF229AF for ; Thu, 25 Jul 2019 18:10:38 -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 AQ_jHmRp3QDW for ; Thu, 25 Jul 2019 18:10:38 -0400 (EDT) Received: from smtp46.i.mail.ru (smtp46.i.mail.ru [94.100.177.106]) (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 5777C227D1 for ; Thu, 25 Jul 2019 18:10:38 -0400 (EDT) Received: by smtp46.i.mail.ru with esmtpa (envelope-from ) id 1hqlwa-0005M0-I1 for tarantool-patches@freelists.org; Fri, 26 Jul 2019 01:10:36 +0300 Subject: [tarantool-patches] Re: [PATCH 2/5] sql: fix resulting type calculation for CASE-WHEN stmt References: From: Vladislav Shpilevoy Message-ID: Date: Fri, 26 Jul 2019 00:12:34 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 Hi! Thanks for the patch! On 24/07/2019 13:42, Nikita Pettik wrote: > Before this patch, resulting type for CASE-WHEN statement was assumed to > be the same as type of argument of first THEN clause. Obviously, it is > wrong and could lead to sad consequence (e.g. creating ephemeral table > with inconsistent format). To deal with this, we check all THEN > arguments: if all of them have the same type, then such type will be > resulting of the whole statement; if at least two types are different, > we can't determine actual resulting type during compilation stage and > assign SCALAR as a most general type in SQL now. > > Need for #4206 > --- > src/box/sql/expr.c | 27 +++++++++++++++++++++------ > test/sql/types.result | 35 +++++++++++++++++++++++++++++++++++ > test/sql/types.test.lua | 8 ++++++++ > 3 files changed, 64 insertions(+), 6 deletions(-) > > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > index d7104d8a0..97f5bd180 100644 > --- a/src/box/sql/expr.c > +++ b/src/box/sql/expr.c > @@ -85,18 +85,33 @@ sql_expr_type(struct Expr *pExpr) > return sql_type_result(rhs_type, lhs_type); > case TK_CONCAT: > return FIELD_TYPE_STRING; > - case TK_CASE: > - assert(pExpr->x.pList->nExpr >= 2); > + case TK_CASE: { > + struct ExprList *cs = pExpr->x.pList; > + assert(cs->nExpr >= 2); > /* > * CASE expression comes at least with one > * WHEN and one THEN clauses. So, first > * expression always represents WHEN > * argument, and the second one - THEN. > - * > - * TODO: We must ensure that all THEN clauses > - * have arguments of the same type. > + * In case at least one type of THEN argument > + * is different from others then we can't > + * determine type of returning value at compiling > + * stage and set SCALAR (i.e. most general) type. > */ > - return sql_expr_type(pExpr->x.pList->a[1].pExpr); > + enum field_type ref_type = sql_expr_type(cs->a[1].pExpr); > + for (int i = 1; i <= cs->nExpr / 2; i = i * 2) { > + if (ref_type != sql_expr_type(cs->a[i + 1].pExpr)) > + return FIELD_TYPE_SCALAR; > + } Something is wrong with that cycle. box.execute("SELECT CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4 WHEN 'e' THEN 5 WHEN 'f' THEN 6 END") --- - metadata: - name: CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4 WHEN 'e' THEN 5 WHEN 'f' THEN 6 END type: scalar rows: - [1] ... But it is not scalar, it is integer - all 'then's here return an integer. Perhaps this is because in the cycle you do i*=2 instead of i+=2, so you compare not 1 vs 3, 1 vs 5, 1 vs 7, etc, (all 'then' are by odd indexes), but 1 vs 2, 1 vs 3, 1 vs 5, 1 vs 9, etc.