[Tarantool-patches] [PATCH v6 14/22] sql: check args of substr()

[imeevma@tarantool.org]

After this patch, the argument types of the substr() function will be
checked properly.

Part of #4159
---
 src/box/sql/func.c          | 25 ++++++++++--------
 test/sql-tap/func.test.lua  |  8 +++---
 test/sql-tap/func2.test.lua | 18 ++++++-------
 test/sql-tap/func5.test.lua | 51 ++++++++++++++++++++++++++++++++++++-
 test/sql/collation.result   |  2 +-
 5 files changed, 78 insertions(+), 26 deletions(-)

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 120e37522..a22c50c79 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -677,22 +677,21 @@ substrFunc(sql_context * context, int argc, sql_value ** argv)
 	const unsigned char *z;
 	const unsigned char *z2;
 	int len;
-	int p0type;
 	i64 p1, p2;
 	int negP2 = 0;
 
 	if (argc != 2 && argc != 3) {
 		diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, "SUBSTR",
-			 "1 or 2", argc);
+			 "2 or 3", argc);
 		context->is_aborted = true;
 		return;
 	}
-	if (sql_value_is_null(argv[1])
-	    || (argc == 3 && sql_value_is_null(argv[2]))
-	    ) {
-		return;
-	}
-	p0type = sql_value_type(argv[0]);
+	enum mp_type p0type = sql_value_type(argv[0]);
+	enum mp_type mp_type_pos = sql_value_type(argv[1]);
+	if (p0type == MP_NIL || mp_type_pos == MP_NIL)
+		return sql_result_null(context);
+	assert(p0type == MP_STR || p0type == MP_BIN);
+	assert(mp_type_pos == MP_INT || mp_type_pos == MP_UINT);
 	p1 = sql_value_int(argv[1]);
 	if (p0type == MP_BIN) {
 		len = sql_value_bytes(argv[0]);
@@ -709,6 +708,10 @@ substrFunc(sql_context * context, int argc, sql_value ** argv)
 			len = sql_utf8_char_count(z, sql_value_bytes(argv[0]));
 	}
 	if (argc == 3) {
+		enum mp_type mp_type_cnt = sql_value_type(argv[2]);
+		if (mp_type_cnt == MP_NIL)
+			return sql_result_null(context);
+		assert(mp_type_cnt == MP_INT || mp_type_cnt == MP_UINT);
 		p2 = sql_value_int(argv[2]);
 		if (p2 < 0) {
 			p2 = -p2;
@@ -2834,9 +2837,9 @@ static struct {
 	}, {
 	 .name = "SUBSTR",
 	 .param_count = -1,
-	 .first_arg = FIELD_TYPE_ANY,
-	 .args = FIELD_TYPE_ANY,
-	 .is_blob_like_str = false,
+	 .first_arg = FIELD_TYPE_STRING,
+	 .args = FIELD_TYPE_INTEGER,
+	 .is_blob_like_str = true,
 	 .returns = FIELD_TYPE_STRING,
 	 .aggregate = FUNC_AGGREGATE_NONE,
 	 .is_deterministic = true,
diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua
index 35e7be562..c9df2a024 100755
--- a/test/sql-tap/func.test.lua
+++ b/test/sql-tap/func.test.lua
@@ -194,23 +194,23 @@ test:do_execsql_test(
         -- </func-2.8>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "func-2.9",
     [[
         SELECT substr(a,1,1) FROM t2
     ]], {
         -- <func-2.9>
-        "1", "", "3", "", "6"
+        1, "Type mismatch: can not convert 1 to string"
         -- </func-2.9>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "func-2.10",
     [[
         SELECT substr(a,2,2) FROM t2
     ]], {
         -- <func-2.10>
-        "", "", "45", "", "78"
+        1, "Type mismatch: can not convert 1 to string"
         -- </func-2.10>
     })
 
diff --git a/test/sql-tap/func2.test.lua b/test/sql-tap/func2.test.lua
index c9bd3665f..d2c83fcf2 100755
--- a/test/sql-tap/func2.test.lua
+++ b/test/sql-tap/func2.test.lua
@@ -50,7 +50,7 @@ test:do_catchsql_test(
         SELECT SUBSTR()
     ]], {
         -- <func2-1.2.1>
-        1, "Wrong number of arguments is passed to SUBSTR(): expected 1 or 2, got 0"
+        1, "Wrong number of arguments is passed to SUBSTR(): expected 2 or 3, got 0"
         -- </func2-1.2.1>
     })
 
@@ -60,7 +60,7 @@ test:do_catchsql_test(
         SELECT SUBSTR('Supercalifragilisticexpialidocious')
     ]], {
         -- <func2-1.2.2>
-        1, "Wrong number of arguments is passed to SUBSTR(): expected 1 or 2, got 1"
+        1, "Wrong number of arguments is passed to SUBSTR(): expected 2 or 3, got 1"
         -- </func2-1.2.2>
     })
 
@@ -70,7 +70,7 @@ test:do_catchsql_test(
         SELECT SUBSTR('Supercalifragilisticexpialidocious', 1,1,1)
     ]], {
         -- <func2-1.2.3>
-        1, "Wrong number of arguments is passed to SUBSTR(): expected 1 or 2, got 4"
+        1, "Wrong number of arguments is passed to SUBSTR(): expected 2 or 3, got 4"
         -- </func2-1.2.3>
     })
 
@@ -673,7 +673,7 @@ if ("ሴ" ~= "u1234")
             SELECT SUBSTR()
         ]], {
             -- <func2-2.1.2>
-            1, "Wrong number of arguments is passed to SUBSTR(): expected 1 or 2, got 0"
+            1, "Wrong number of arguments is passed to SUBSTR(): expected 2 or 3, got 0"
             -- </func2-2.1.2>
         })
 
@@ -683,7 +683,7 @@ if ("ሴ" ~= "u1234")
             SELECT SUBSTR('hiሴho')
         ]], {
             -- <func2-2.1.3>
-            1, "Wrong number of arguments is passed to SUBSTR(): expected 1 or 2, got 1"
+            1, "Wrong number of arguments is passed to SUBSTR(): expected 2 or 3, got 1"
             -- </func2-2.1.3>
         })
 
@@ -693,7 +693,7 @@ if ("ሴ" ~= "u1234")
             SELECT SUBSTR('hiሴho', 1,1,1)
         ]], {
             -- <func2-2.1.4>
-            1, "Wrong number of arguments is passed to SUBSTR(): expected 1 or 2, got 4"
+            1, "Wrong number of arguments is passed to SUBSTR(): expected 2 or 3, got 4"
             -- </func2-2.1.4>
         })
 
@@ -1038,7 +1038,7 @@ test:do_catchsql_test(
         SELECT SUBSTR()
     ]], {
         -- <func2-3.1.2>
-        1, "Wrong number of arguments is passed to SUBSTR(): expected 1 or 2, got 0"
+        1, "Wrong number of arguments is passed to SUBSTR(): expected 2 or 3, got 0"
         -- </func2-3.1.2>
     })
 
@@ -1048,7 +1048,7 @@ test:do_catchsql_test(
         SELECT SUBSTR(x'1234')
     ]], {
         -- <func2-3.1.3>
-        1, "Wrong number of arguments is passed to SUBSTR(): expected 1 or 2, got 1"
+        1, "Wrong number of arguments is passed to SUBSTR(): expected 2 or 3, got 1"
         -- </func2-3.1.3>
     })
 
@@ -1058,7 +1058,7 @@ test:do_catchsql_test(
         SELECT SUBSTR(x'1234', 1,1,1)
     ]], {
         -- <func2-3.1.4>
-        1, "Wrong number of arguments is passed to SUBSTR(): expected 1 or 2, got 4"
+        1, "Wrong number of arguments is passed to SUBSTR(): expected 2 or 3, got 4"
         -- </func2-3.1.4>
     })
 
diff --git a/test/sql-tap/func5.test.lua b/test/sql-tap/func5.test.lua
index d2cddc56d..78b98f2b1 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(134)
+test:plan(141)
 
 --!./tcltestrunner.lua
 -- 2010 August 27
@@ -1081,4 +1081,53 @@ test:do_catchsql_test(
         1, "Type mismatch: can not convert varbinary to string"
     })
 
+test:do_execsql_test(
+    "func-5-6.17.1", [[
+        SELECT substr(NULL, NULL, NULL);
+    ]],{
+        ""
+    })
+
+test:do_catchsql_test(
+    "func-5-6.17.2", [[
+        SELECT substr(123, 123, 123);
+    ]], {
+        1, "Type mismatch: can not convert 123 to string"
+    })
+
+test:do_catchsql_test(
+    "func-5-6.17.3", [[
+        SELECT substr(-123, -123, -123);
+    ]], {
+        1, "Type mismatch: can not convert -123 to string"
+    })
+
+test:do_catchsql_test(
+    "func-5-6.17.4", [[
+        SELECT substr(-5.5, -5.5, -5.5);
+    ]], {
+        1, "Type mismatch: can not convert -5.5 to string"
+    })
+
+test:do_catchsql_test(
+    "func-5-6.17.5", [[
+        SELECT substr('-123', '-123', '-123');
+    ]], {
+        1, "Type mismatch: can not convert -123 to integer"
+    })
+
+test:do_catchsql_test(
+    "func-5-6.17.6", [[
+        SELECT substr(false, false, false);
+    ]], {
+        1, "Type mismatch: can not convert FALSE to string"
+    })
+
+test:do_catchsql_test(
+    "func-5-6.17.7", [[
+        SELECT substr(X'3334', X'3334', X'3334');
+    ]], {
+        1, "Type mismatch: can not convert varbinary to integer"
+    })
+
 test:finish_test()
diff --git a/test/sql/collation.result b/test/sql/collation.result
index 4e4c27ef0..857564607 100644
--- a/test/sql/collation.result
+++ b/test/sql/collation.result
@@ -298,7 +298,7 @@ box.execute("SELECT * FROM t WHERE a COLLATE \"binary\" = c COLLATE \"unicode\";
 box.execute("SELECT * FROM t WHERE a COLLATE \"binary\" = substr();")
 ---
 - null
-- 'Wrong number of arguments is passed to SUBSTR(): expected 1 or 2, got 0'
+- 'Wrong number of arguments is passed to SUBSTR(): expected 2 or 3, got 0'
 ...
 -- Compound queries perform implicit comparisons between values.
 -- Hence, rules for collations compatibilities are the same.
-- 
2.25.1