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 374DC25B47 for ; Wed, 24 Apr 2019 19:03:11 -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 hj3q-GRd6x3s for ; Wed, 24 Apr 2019 19:03:11 -0400 (EDT) Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (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 8C26425B26 for ; Wed, 24 Apr 2019 19:03:10 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.8\)) Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: invalid integer type in arithmetic operations From: "n.pettik" In-Reply-To: <0929e575-2d28-7cd0-36bc-0a1bf1af76fe@tarantool.org> Date: Thu, 25 Apr 2019 02:03:05 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <04E0C86F-84E5-400F-ACE0-80FD6799E4E3@tarantool.org> References: <72941eba60647c98f4559cbd0c862bade990d761.1556120090.git.kshcherbatov@tarantool.org> <0929e575-2d28-7cd0-36bc-0a1bf1af76fe@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 , Kirill Shcherbatov > On 24 Apr 2019, at 22:56, Vladislav Shpilevoy = wrote: >> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c >> index b3613d3ea..9b52e90f3 100644 >> --- a/src/box/sql/expr.c >> +++ b/src/box/sql/expr.c >> @@ -342,8 +342,19 @@ sql_expr_coll(Parse *parse, Expr *p, bool = *is_explicit_coll, uint32_t *coll_id, >> enum field_type >> sql_type_result(enum field_type lhs, enum field_type rhs) >> { >> - if (sql_type_is_numeric(lhs) || sql_type_is_numeric(rhs)) >> + if (sql_type_is_numeric(lhs) || sql_type_is_numeric(rhs)) { >> + if (lhs =3D=3D FIELD_TYPE_NUMBER || rhs =3D=3D = FIELD_TYPE_NUMBER) >> + return FIELD_TYPE_NUMBER; >> + if (lhs =3D=3D FIELD_TYPE_INTEGER || rhs =3D=3D = FIELD_TYPE_INTEGER) >> + return FIELD_TYPE_INTEGER; >> + /* >> + * FIXME: FIELD_TYPE_UNSIGNED static type is not >> + * allowed yet. >> + */ >> + assert(lhs =3D=3D FIELD_TYPE_UNSIGNED || >> + rhs =3D=3D FIELD_TYPE_UNSIGNED); >=20 > How does it work? If it is not allowed, then lhs and rhs should not > be equal to FIELD_TYPE_UNSIGNED, and this assertion should fail, it is = not? >=20 > (I did not test, just looked at the diff in the mailing list) Execution flaw hits this branch if both operands are numeric. We consider only _INTEGER, _NUMBER and _UNSIGNED be numeric types. If we hit this assertion, then both operands should be _UNSIGNED (if I=E2=80=99m not mistaken, condition should be logical AND not OR). Surely, we still can create space and set format with unsigned types from Lua, so strictly speaking UNSIGNED is allowed even now. But we can=E2=80=99t set UNSIGNED as a type of column in SQL, and we = don=E2=80=99t set this type in meta. So in some sense it is not allowed. Mb it is worth fixing comment. Or return _UNSIGNED instead of _NUMBER in this case. I guess there will be no severe consequences.