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 D21182B5BB for ; Thu, 25 Apr 2019 03:32: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 752Ahb34L9na for ; Thu, 25 Apr 2019 03:32:38 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 E019F2884B for ; Thu, 25 Apr 2019 03:32:37 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: invalid integer type in arithmetic operations References: <72941eba60647c98f4559cbd0c862bade990d761.1556120090.git.kshcherbatov@tarantool.org> <7C38D671-BF76-4B11-A15D-5BDC6E2358B8@tarantool.org> From: Kirill Shcherbatov Message-ID: <590688a0-5a0d-8c2c-5db5-66be40956be0@tarantool.org> Date: Thu, 25 Apr 2019 10:32:33 +0300 MIME-Version: 1.0 In-Reply-To: <7C38D671-BF76-4B11-A15D-5BDC6E2358B8@tarantool.org> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit 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, "n.pettik" Cc: Vladislav Shpilevoy > -> if resulting value of arithmetic operations is integers, > then make sure its type also integer (not number). Updated. > 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’m 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’t set UNSIGNED as a type of column in SQL, and we don’t > 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. Let's remove this comment and return _UNSIGNED. > The rest is OK. ================================================ Tarantool SQL used to return 'number' type in request metadata for arithmetic operations even when only 'integer's were used. This also fixes query planner optimisation bug: SELECT a FROM t1 WHERE a+0 IN (SELECT a FROM t1); used to open a new ephemeral table with OpenTEphemeral when it is not required (introduced by 2b22b913). Closes #4103 --- src/box/sql/expr.c | 11 +++++++++-- test/sql-tap/in3.test.lua | 2 +- test/sql-tap/tkt-80e031a00f.test.lua | 8 ++++---- test/sql/gh-3199-no-mem-leaks.result | 24 ++++++++++++------------ test/sql/types.result | 28 ++++++++++++++++++++++++++++ test/sql/types.test.lua | 8 ++++++++ 6 files changed, 62 insertions(+), 19 deletions(-) diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index b3613d3ea..ba7eea59d 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -342,8 +342,15 @@ 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)) - return FIELD_TYPE_NUMBER; + if (sql_type_is_numeric(lhs) || sql_type_is_numeric(rhs)) { + if (lhs == FIELD_TYPE_NUMBER || rhs == FIELD_TYPE_NUMBER) + return FIELD_TYPE_NUMBER; + if (lhs == FIELD_TYPE_INTEGER || rhs == FIELD_TYPE_INTEGER) + return FIELD_TYPE_INTEGER; + assert(lhs == FIELD_TYPE_UNSIGNED || + rhs == FIELD_TYPE_UNSIGNED); + return FIELD_TYPE_UNSIGNED; + } return FIELD_TYPE_SCALAR; } diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua index a5e31f8a7..1ca6a6446 100755 --- a/test/sql-tap/in3.test.lua +++ b/test/sql-tap/in3.test.lua @@ -92,7 +92,7 @@ test:do_test( return exec_neph(" SELECT a FROM t1 WHERE a+0 IN (SELECT a FROM t1); ") end, { -- - 1, 1, 3, 5 + 0, 1, 3, 5 -- }) diff --git a/test/sql-tap/tkt-80e031a00f.test.lua b/test/sql-tap/tkt-80e031a00f.test.lua index 7fd348632..3edea11ac 100755 --- a/test/sql-tap/tkt-80e031a00f.test.lua +++ b/test/sql-tap/tkt-80e031a00f.test.lua @@ -346,7 +346,7 @@ test:do_catchsql_test( SELECT 'hello' IN t1 ]], { -- - 1, 'Type mismatch: can not convert hello to number' + 1, 'Type mismatch: can not convert hello to integer' -- }) @@ -356,7 +356,7 @@ test:do_catchsql_test( SELECT 'hello' NOT IN t1 ]], { -- - 1, 'Type mismatch: can not convert hello to number' + 1, 'Type mismatch: can not convert hello to integer' -- }) @@ -386,7 +386,7 @@ test:do_catchsql_test( SELECT x'303132' IN t1 ]], { -- - 1, 'Type mismatch: can not convert 012 to number' + 1, 'Type mismatch: can not convert 012 to integer' -- }) @@ -396,7 +396,7 @@ test:do_catchsql_test( SELECT x'303132' NOT IN t1 ]], { -- - 1, 'Type mismatch: can not convert 012 to number' + 1, 'Type mismatch: can not convert 012 to integer' -- }) diff --git a/test/sql/gh-3199-no-mem-leaks.result b/test/sql/gh-3199-no-mem-leaks.result index 22be46a82..e7ba1d29c 100644 --- a/test/sql/gh-3199-no-mem-leaks.result +++ b/test/sql/gh-3199-no-mem-leaks.result @@ -31,7 +31,7 @@ box.execute('SELECT x, y, x + y FROM test ORDER BY y') - name: Y type: integer - name: x + y - type: number + type: integer rows: - [1, 1, 2] - [2, 2, 4] @@ -48,7 +48,7 @@ box.execute('SELECT x, y, x + y FROM test ORDER BY y') - name: Y type: integer - name: x + y - type: number + type: integer rows: - [1, 1, 2] - [2, 2, 4] @@ -61,7 +61,7 @@ box.execute('SELECT x, y, x + y FROM test ORDER BY y') - name: Y type: integer - name: x + y - type: number + type: integer rows: - [1, 1, 2] - [2, 2, 4] @@ -74,7 +74,7 @@ box.execute('SELECT x, y, x + y FROM test ORDER BY y') - name: Y type: integer - name: x + y - type: number + type: integer rows: - [1, 1, 2] - [2, 2, 4] @@ -87,7 +87,7 @@ box.execute('SELECT x, y, x + y FROM test ORDER BY y') - name: Y type: integer - name: x + y - type: number + type: integer rows: - [1, 1, 2] - [2, 2, 4] @@ -114,7 +114,7 @@ box.execute('SELECT a, id + 2, b FROM test2 WHERE b < id * 2 ORDER BY a ') - name: A type: string - name: id + 2 - type: number + type: integer - name: B type: integer rows: @@ -133,7 +133,7 @@ box.execute('SELECT a, id + 2 * b, a FROM test2 WHERE b < id * 2 ORDER BY a ') - name: A type: string - name: id + 2 * b - type: number + type: integer - name: A type: string rows: @@ -148,7 +148,7 @@ box.execute('SELECT a, id + 2 * b, a FROM test2 WHERE b < id * 2 ORDER BY a ') - name: A type: string - name: id + 2 * b - type: number + type: integer - name: A type: string rows: @@ -163,7 +163,7 @@ box.execute('SELECT a, id + 2 * b, a FROM test2 WHERE b < id * 2 ORDER BY a ') - name: A type: string - name: id + 2 * b - type: number + type: integer - name: A type: string rows: @@ -182,7 +182,7 @@ box.execute('SELECT x, y + 3 * b, b FROM test2, test WHERE b = x') - name: X type: integer - name: y + 3 * b - type: number + type: integer - name: B type: integer rows: @@ -195,7 +195,7 @@ box.execute('SELECT x, y + 3 * b, b FROM test2, test WHERE b = x') - name: X type: integer - name: y + 3 * b - type: number + type: integer - name: B type: integer rows: @@ -208,7 +208,7 @@ box.execute('SELECT x, y + 3 * b, b FROM test2, test WHERE b = x') - name: X type: integer - name: y + 3 * b - type: number + type: integer - name: B type: integer rows: diff --git a/test/sql/types.result b/test/sql/types.result index de17bbb78..8f442dc7d 100644 --- a/test/sql/types.result +++ b/test/sql/types.result @@ -860,3 +860,31 @@ box.space.T:drop() box.space.T1:drop() --- ... +-- +-- gh-4103: If resulting value of arithmetic operations is +-- integers, then make sure its type also integer (not number). +-- +box.execute('SELECT 1 + 1;') +--- +- metadata: + - name: 1 + 1 + type: integer + rows: + - [2] +... +box.execute('SELECT 1 + 1.1;') +--- +- metadata: + - name: 1 + 1.1 + type: number + rows: + - [2.1] +... +box.execute('SELECT \'9223372036854\' + 1;') +--- +- metadata: + - name: '''9223372036854'' + 1' + type: integer + rows: + - [9223372036855] +... diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua index 461635978..48c9bde10 100644 --- a/test/sql/types.test.lua +++ b/test/sql/types.test.lua @@ -205,3 +205,11 @@ box.execute("SELECT s FROM t1 WHERE s IN (true, 1, 'abcd')") box.space.T:drop() box.space.T1:drop() + +-- +-- gh-4103: If resulting value of arithmetic operations is +-- integers, then make sure its type also integer (not number). +-- +box.execute('SELECT 1 + 1;') +box.execute('SELECT 1 + 1.1;') +box.execute('SELECT \'9223372036854\' + 1;') -- 2.21.0