[Tarantool-patches] [PATCH v5 04/17] sql: check args of abs()

[imeevma at 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(
         -- </func-4.4.1>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "func-4.4.2",
     [[
         SELECT abs(t1) FROM tbl1
     ]], {
         -- <func-4.4.2>
-        0.0, 0.0, 0.0, 0.0, 0.0
+        1, "Type mismatch: can not convert this to number"
         -- </func-4.4.2>
     })
 
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 70fbbc5a2..f5c059ef0 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