From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id E7A36445320 for ; Thu, 16 Jul 2020 17:45:48 +0300 (MSK) From: imeevma@tarantool.org Date: Thu, 16 Jul 2020 17:45:47 +0300 Message-Id: In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH v6 03/22] sql: check args of abs() List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: korablev@tarantool.org, tsafin@tarantool.org, tarantool-patches@dev.tarantool.org After this patch, the argument types of the abs() function will be checked properly. Part of #4159 --- src/box/sql/func.c | 52 ++++++++++------------------------- test/sql-tap/func.test.lua | 4 +-- test/sql-tap/func5.test.lua | 55 ++++++++++++++++++++++++++++++++++++- test/sql/boolean.result | 2 +- test/sql/types.result | 2 +- 5 files changed, 73 insertions(+), 42 deletions(-) diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 511061b50..329554731 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -504,44 +504,22 @@ absFunc(sql_context * context, int argc, sql_value ** argv) { assert(argc == 1); UNUSED_PARAMETER(argc); - switch (sql_value_type(argv[0])) { - case MP_UINT: { - sql_result_uint(context, sql_value_uint64(argv[0])); - break; - } - case MP_INT: { + enum mp_type mp_type = sql_value_type(argv[0]); + if (mp_type == MP_NIL) + return sql_result_null(context); + if (mp_type == MP_UINT) + return sql_result_uint(context, sql_value_uint64(argv[0])); + if (mp_type == MP_INT) { int64_t value = sql_value_int64(argv[0]); assert(value < 0); - sql_result_uint(context, -value); - break; - } - case MP_NIL:{ - /* IMP: R-37434-19929 Abs(X) returns NULL if X is NULL. */ - sql_result_null(context); - break; - } - case MP_BOOL: - case MP_BIN: - case MP_ARRAY: - case MP_MAP: { - diag_set(ClientError, ER_INCONSISTENT_TYPES, "number", - mem_type_to_str(argv[0])); - context->is_aborted = true; - return; - } - default:{ - /* Because sql_value_double() returns 0.0 if the argument is not - * something that can be converted into a number, we have: - * IMP: R-01992-00519 Abs(X) returns 0.0 if X is a string or blob - * that cannot be converted to a numeric value. - */ - double rVal = sql_value_double(argv[0]); - if (rVal < 0) - rVal = -rVal; - sql_result_double(context, rVal); - break; - } - } + return sql_result_uint(context, -value); + } + assert(mp_type == MP_DOUBLE); + double value = sql_value_double(argv[0]); + if (value < 0) + value = -value; + sql_result_double(context, value); + return; } /** @@ -2242,7 +2220,7 @@ static struct { } sql_builtins[] = { {.name = "ABS", .param_count = 1, - .first_arg = FIELD_TYPE_ANY, + .first_arg = FIELD_TYPE_NUMBER, .args = FIELD_TYPE_ANY, .is_blob_like_str = false, .returns = FIELD_TYPE_NUMBER, diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua index 3c088920f..422c8ce58 100755 --- a/test/sql-tap/func.test.lua +++ b/test/sql-tap/func.test.lua @@ -412,13 +412,13 @@ test:do_execsql_test( -- }) -test:do_execsql_test( +test:do_catchsql_test( "func-4.4.2", [[ SELECT abs(t1) FROM tbl1 ]], { -- - 0.0, 0.0, 0.0, 0.0, 0.0 + 1, "Type mismatch: can not convert this to number" -- }) diff --git a/test/sql-tap/func5.test.lua b/test/sql-tap/func5.test.lua index 8329e1735..d6d5d4df6 100755 --- a/test/sql-tap/func5.test.lua +++ b/test/sql-tap/func5.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(22) +test:plan(29) --!./tcltestrunner.lua -- 2010 August 27 @@ -293,4 +293,57 @@ test:do_catchsql_test( box.func.COUNTER1:drop() box.func.COUNTER2:drop() +-- +-- gh-4159: Make sure function argument types are validated +-- correctly. +-- +test:do_execsql_test( + "func-5-6.1.1", [[ + SELECT abs(NULL); + ]],{ + "" + }) + +test:do_execsql_test( + "func-5-6.1.2", [[ + SELECT abs(123); + ]], { + 123 + }) + +test:do_execsql_test( + "func-5-6.1.3", [[ + SELECT abs(-123); + ]], { + 123 + }) + +test:do_execsql_test( + "func-5-6.1.4", [[ + SELECT abs(-5.5); + ]], { + 5.5 + }) + +test:do_catchsql_test( + "func-5-6.1.5", [[ + SELECT abs('-123'); + ]], { + 1, "Type mismatch: can not convert -123 to number" + }) + +test:do_catchsql_test( + "func-5-6.1.6", [[ + SELECT abs(false); + ]], { + 1, "Type mismatch: can not convert FALSE to number" + }) + +test:do_catchsql_test( + "func-5-6.1.7", [[ + SELECT abs(X'3334'); + ]], { + 1, "Type mismatch: can not convert varbinary to number" + }) + test:finish_test() diff --git a/test/sql/boolean.result b/test/sql/boolean.result index 112e41a12..c470ebd40 100644 --- a/test/sql/boolean.result +++ b/test/sql/boolean.result @@ -276,7 +276,7 @@ SELECT is_boolean('true'); SELECT abs(a) FROM t0; | --- | - null - | - 'Inconsistent types: expected number got boolean' + | - 'Type mismatch: can not convert FALSE to number' | ... SELECT lower(a) FROM t0; | --- diff --git a/test/sql/types.result b/test/sql/types.result index 438ac44ab..9cb0818e6 100644 --- a/test/sql/types.result +++ b/test/sql/types.result @@ -1326,7 +1326,7 @@ box.execute("SELECT upper(v) FROM t;") box.execute("SELECT abs(v) FROM t;") --- - null -- 'Inconsistent types: expected number got varbinary' +- 'Type mismatch: can not convert varbinary to number' ... box.execute("SELECT typeof(v) FROM t;") --- -- 2.25.1