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 0F17724732 for ; Sun, 28 Jul 2019 19:56:15 -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 lLkvZS6ZomfL for ; Sun, 28 Jul 2019 19:56:14 -0400 (EDT) Received: from smtp33.i.mail.ru (smtp33.i.mail.ru [94.100.177.93]) (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 58ABA2470F for ; Sun, 28 Jul 2019 19:56:14 -0400 (EDT) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\)) Subject: [tarantool-patches] Re: [PATCH 2/5] sql: fix resulting type calculation for CASE-WHEN stmt From: "n.pettik" In-Reply-To: Date: Mon, 29 Jul 2019 02:56:10 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: 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 > On 26 Jul 2019, at 01:03, Vladislav Shpilevoy = wrote: >=20 > Hi! Thanks for the patch! >=20 > 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. >>=20 >> 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(-) >>=20 >> 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 >=3D 2); >> + case TK_CASE: { >> + struct ExprList *cs =3D pExpr->x.pList; >> + assert(cs->nExpr >=3D 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 =3D = sql_expr_type(cs->a[1].pExpr); >> + for (int i =3D 1; i <=3D cs->nExpr / 2; i =3D i * 2) { >> + if (ref_type !=3D sql_expr_type(cs->a[i + = 1].pExpr)) >> + return FIELD_TYPE_SCALAR; >> + } >=20 > Something is wrong with that cycle. >=20 > 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] > ... >=20 > 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*=3D2 instead of i+=3D2, 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. Yep, this is obvious bug. Fixed: diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 97f5bd180..7b3a41c46 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -99,8 +99,8 @@ sql_expr_type(struct Expr *pExpr) * stage and set SCALAR (i.e. most general) type. */ enum field_type ref_type =3D = sql_expr_type(cs->a[1].pExpr); - for (int i =3D 1; i <=3D cs->nExpr / 2; i =3D i * 2) { - if (ref_type !=3D sql_expr_type(cs->a[i + = 1].pExpr)) + for (int i =3D 3; i < cs->nExpr; i +=3D 2) { + if (ref_type !=3D sql_expr_type(cs->a[i].pExpr)) return FIELD_TYPE_SCALAR; } /* diff --git a/test/sql/types.result b/test/sql/types.result index 1db4b980d..d05fa9dfa 100644 --- a/test/sql/types.result +++ b/test/sql/types.result @@ -1001,3 +1001,39 @@ box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 = THEN 123 ELSE 'asd' END") rows: - [666] ... +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: integer + rows: + - [1] +... +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 'asd' 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 'asd' END + type: scalar + rows: + - [1] +... +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 ELSE 'asd' 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 ELSE 'asd' END + type: scalar + rows: + - [1] +... +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 ELSE 7 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 ELSE 7 END + type: integer + rows: + - [1] +... diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua index b66a3e068..8fd20f892 100644 --- a/test/sql/types.test.lua +++ b/test/sql/types.test.lua @@ -242,3 +242,7 @@ box.execute("SELECT CASE 1 WHEN 1 THEN x'0000000000' = WHEN 2 THEN 'str' END") box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 END") box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 ELSE 321 = END") box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 ELSE 'asd' = END") +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;") +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 'asd' END;") +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 ELSE 'asd' END;") +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 ELSE 7 END;")