[tarantool-patches] [PATCH v1 2/2] sql: check returned value type for UDF

Kirill Shcherbatov kshcherbatov at tarantool.org
Mon Sep 16 16:23:13 MSK 2019


box.schema.func create endpoint defines a returned
value type 'returns', but it used to be ignored.
This patch implements required type checks for user
defined functions.

Closes #4387
---
 src/box/errcode.h               |  1 +
 src/box/field_def.h             | 17 +++++++-
 src/box/sql/vdbe.c              | 12 ++++++
 test/sql-tap/func.test.lua      | 74 ++++++++++++++++++++++++++++++++-
 test/sql/func-recreate.result   |  8 ++--
 test/sql/func-recreate.test.lua |  6 +--
 6 files changed, 108 insertions(+), 10 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index dcb90f678..e71737ba1 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -256,6 +256,7 @@ struct errcode_record {
 	/*201 */_(ER_NO_SUCH_FIELD_NAME,	"Field '%s' was not found in the tuple") \
 	/*202 */_(ER_FUNC_WRONG_ARG_COUNT,	"Wrong number of arguments is passed to %s(): expected %s, got %d") \
 	/*203 */_(ER_BOOTSTRAP_READONLY,	"Trying to bootstrap a local read-only instance as master") \
+	/*204 */_(ER_FUNC_INVALID_RETURN,	"Function '%s' returned invalid value: expected %s got %s") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/field_def.h b/src/box/field_def.h
index fdedc9622..49c2998f5 100644
--- a/src/box/field_def.h
+++ b/src/box/field_def.h
@@ -144,6 +144,19 @@ struct field_def {
 	struct Expr *default_value_expr;
 };
 
+/**
+ * Checks if mp_type (except MP_EXT) (MsgPack) is compatible
+ * with given field type.
+ */
+static inline bool
+field_mp_plain_type_is_compatible(enum field_type type, enum mp_type mp_type,
+				  bool is_nullable)
+{
+	assert(mp_type != MP_EXT);
+	uint32_t mask = field_mp_type[type] | (is_nullable * (1U << MP_NIL));
+	return (mask & (1U << mp_type)) != 0;
+}
+
 /** Checks if mp_type (MsgPack) is compatible with field type. */
 static inline bool
 field_mp_type_is_compatible(enum field_type type, const char *data,
@@ -154,8 +167,8 @@ field_mp_type_is_compatible(enum field_type type, const char *data,
 	assert((size_t)mp_type < CHAR_BIT * sizeof(*field_mp_type));
 	uint32_t mask;
 	if (mp_type != MP_EXT) {
-		mask = field_mp_type[type] | (is_nullable * (1U << MP_NIL));
-		return (mask & (1U << mp_type)) != 0;
+		return field_mp_plain_type_is_compatible(type, mp_type,
+							 is_nullable);
 	} else {
 		int8_t ext_type;
 		mp_decode_extl(&data, &ext_type);
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 9a9648c1a..20c1d2291 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -348,6 +348,7 @@ mem_apply_type(struct Mem *record, enum field_type type)
 			return -1;
 		return 0;
 	case FIELD_TYPE_SCALAR:
+	case FIELD_TYPE_ANY:
 		return 0;
 	default:
 		return -1;
@@ -1810,6 +1811,11 @@ case OP_Function: {
 		diag_set(ClientError, ER_NO_SUCH_FUNCTION, pOp->p4.z);
 		goto abort_due_to_error;
 	}
+	/*
+	 * Function call may yield and func pointer may be
+	 * invalid above.
+	 */
+	enum field_type returns = func->def->returns;
 	int argc = pOp->p5;
 	struct Mem *argv = &aMem[pOp->p2];
 	struct port args, ret;
@@ -1829,6 +1835,12 @@ case OP_Function: {
 	region_truncate(region, region_svp);
 	if (mem == NULL)
 		goto abort_due_to_error;
+	enum mp_type type = sql_value_type((sql_value *)pOut);
+	if (!field_mp_plain_type_is_compatible(returns, type, true)) {
+		diag_set(ClientError, ER_FUNC_INVALID_RETURN, pOp->p4.z,
+			 field_type_strs[returns], mp_type_strs[type]);
+		goto abort_due_to_error;
+	}
 
 	/*
 	 * Copy the result of the function invocation into
diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua
index a96fc611e..9d3a16c96 100755
--- a/test/sql-tap/func.test.lua
+++ b/test/sql-tap/func.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(14606)
+test:plan(14642)
 
 --!./tcltestrunner.lua
 -- 2001 September 15
@@ -2909,4 +2909,76 @@ test:do_execsql_test(
     "SELECT POSITION(CHAR(00), CHAR(65,66));",
     {0})
 
+box.schema.func.create('TOSTRING', {language = 'Lua', is_deterministic = true,
+                                    body = 'function(a) return a end',
+                                    param_list = {'any'}, returns = 'string',
+                                    exports = {'LUA', 'SQL'}})
+test:do_execsql_test("func-77", "SELECT TOSTRING('1');", {'1'})
+test:do_execsql_test("func-78", "SELECT TOSTRING('a');", {'a'})
+test:do_catchsql_test("func-79", "SELECT TOSTRING(1);", {1, "Function 'TOSTRING' returned invalid value: expected string got unsigned"})
+test:do_catchsql_test("func-80", "SELECT TOSTRING(-1);", {1, "Function 'TOSTRING' returned invalid value: expected string got integer"})
+test:do_catchsql_test("func-81", "SELECT TOSTRING(-1.1);", {1, "Function 'TOSTRING' returned invalid value: expected string got double"})
+test:do_catchsql_test("func-83", "SELECT TOSTRING(TRUE);", {1, "Function 'TOSTRING' returned invalid value: expected string got boolean"})
+box.func.TOSTRING:drop()
+
+box.schema.func.create('TOUNSIGNED', {language = 'Lua', is_deterministic = true,
+                                      body = 'function(a) return a end',
+                                      param_list = {'any'}, returns = 'unsigned',
+                                      exports = {'LUA', 'SQL'}})
+test:do_catchsql_test("func-84", "SELECT TOUNSIGNED('1');", {1, "Function 'TOUNSIGNED' returned invalid value: expected unsigned got string"})
+test:do_catchsql_test("func-85", "SELECT TOUNSIGNED('a');", {1, "Function 'TOUNSIGNED' returned invalid value: expected unsigned got string"})
+test:do_execsql_test("func-86", "SELECT TOUNSIGNED(1);", {1})
+test:do_catchsql_test("func-87", "SELECT TOUNSIGNED(-1);", {1, "Function 'TOUNSIGNED' returned invalid value: expected unsigned got integer"})
+test:do_catchsql_test("func-88", "SELECT TOUNSIGNED(-1.1);", {1, "Function 'TOUNSIGNED' returned invalid value: expected unsigned got double"})
+test:do_catchsql_test("func-90", "SELECT TOUNSIGNED(TRUE);", {1, "Function 'TOUNSIGNED' returned invalid value: expected unsigned got boolean"})
+box.func.TOUNSIGNED:drop()
+
+box.schema.func.create('TOINTEGER', {language = 'Lua', is_deterministic = true,
+                                     body = 'function(a) return a end',
+                                     param_list = {'any'}, returns = 'integer',
+                                     exports = {'LUA', 'SQL'}})
+test:do_catchsql_test("func-91", "SELECT TOINTEGER('1');", {1, "Function 'TOINTEGER' returned invalid value: expected integer got string"})
+test:do_catchsql_test("func-92", "SELECT TOINTEGER('a');", {1, "Function 'TOINTEGER' returned invalid value: expected integer got string"})
+test:do_execsql_test("func-93", "SELECT TOINTEGER(1);", {1})
+test:do_execsql_test("func-94", "SELECT TOINTEGER(-1);", {-1})
+test:do_catchsql_test("func-95", "SELECT TOINTEGER(-1.1);", {1, "Function 'TOINTEGER' returned invalid value: expected integer got double"})
+test:do_catchsql_test("func-96", "SELECT TOINTEGER(TRUE);", {1, "Function 'TOINTEGER' returned invalid value: expected integer got boolean"})
+box.func.TOINTEGER:drop()
+
+box.schema.func.create('TONUMBER', {language = 'Lua', is_deterministic = true,
+                                    body = 'function(a) return a end',
+                                    param_list = {'any'}, returns = 'number',
+                                    exports = {'LUA', 'SQL'}})
+test:do_catchsql_test("func-97", "SELECT TONUMBER('1');", {1, "Function 'TONUMBER' returned invalid value: expected number got string"})
+test:do_catchsql_test("func-98", "SELECT TONUMBER('a');", {1, "Function 'TONUMBER' returned invalid value: expected number got string"})
+test:do_execsql_test("func-99", "SELECT TONUMBER(1);", {1})
+test:do_execsql_test("func-100", "SELECT TONUMBER(-1);", {-1})
+test:do_execsql_test("func-101", "SELECT TONUMBER(-1.1);", {-1.1})
+test:do_catchsql_test("func-102", "SELECT TONUMBER(TRUE);", {1, "Function 'TONUMBER' returned invalid value: expected number got boolean"})
+box.func.TONUMBER:drop()
+
+box.schema.func.create('TOBOOLEAN', {language = 'Lua', is_deterministic = true,
+                                    body = 'function(a) return a end',
+                                    param_list = {'any'}, returns = 'boolean',
+                                    exports = {'LUA', 'SQL'}})
+test:do_catchsql_test("func-103", "SELECT TOBOOLEAN('1');", {1, "Function 'TOBOOLEAN' returned invalid value: expected boolean got string"})
+test:do_catchsql_test("func-104", "SELECT TOBOOLEAN('a');", {1, "Function 'TOBOOLEAN' returned invalid value: expected boolean got string"})
+test:do_catchsql_test("func-105", "SELECT TOBOOLEAN(1);", {1, "Function 'TOBOOLEAN' returned invalid value: expected boolean got unsigned"})
+test:do_catchsql_test("func-106", "SELECT TOBOOLEAN(-1);", {1, "Function 'TOBOOLEAN' returned invalid value: expected boolean got integer"})
+test:do_catchsql_test("func-107", "SELECT TOBOOLEAN(-1.1);", {1, "Function 'TOBOOLEAN' returned invalid value: expected boolean got double"})
+test:do_execsql_test("func-108", "SELECT TOBOOLEAN(TRUE);", {true})
+box.func.TOBOOLEAN:drop()
+
+box.schema.func.create('TOSCALAR', {language = 'Lua', is_deterministic = true,
+                                    body = 'function(a) return a end',
+                                    param_list = {'any'}, returns = 'scalar',
+                                    exports = {'LUA', 'SQL'}})
+test:do_execsql_test("func-109", "SELECT TOSCALAR('1');", {'1'})
+test:do_execsql_test("func-110", "SELECT TOSCALAR('a');", {'a'})
+test:do_execsql_test("func-111", "SELECT TOSCALAR(1);", {1})
+test:do_execsql_test("func-112", "SELECT TOSCALAR(-1);", {-1})
+test:do_execsql_test("func-113", "SELECT TOSCALAR(-1.1);", {-1.1})
+test:do_execsql_test("func-114", "SELECT TOSCALAR(TRUE);", {true})
+box.func.TOSCALAR:drop()
+
 test:finish_test()
diff --git a/test/sql/func-recreate.result b/test/sql/func-recreate.result
index 1f6966431..a0a67a106 100644
--- a/test/sql/func-recreate.result
+++ b/test/sql/func-recreate.result
@@ -18,7 +18,7 @@ test_run:cmd("setopt delimiter ';'")
 ...
 box.schema.func.create('WAITFOR', {language = 'Lua',
                        body = 'function (n) fiber.sleep(n) return n end',
-                       param_list = {'integer'}, returns = 'integer',
+                       param_list = {'number'}, returns = 'number',
                        exports = {'LUA', 'SQL'}})
 test_run:cmd("setopt delimiter ''");
 ---
@@ -41,7 +41,7 @@ test_run:cmd("setopt delimiter ';'")
 ...
 box.schema.func.create('WAITFOR', {language = 'Lua',
                        body = 'function (n) fiber.sleep(n) return n end',
-                       param_list = {'integer'}, returns = 'integer',
+                       param_list = {'number'}, returns = 'number',
                        exports = {'LUA', 'SQL'}})
 test_run:cmd("setopt delimiter ''");
 ---
@@ -50,7 +50,7 @@ ch:get()
 ---
 - metadata:
   - name: WAITFOR(0.2)
-    type: integer
+    type: number
   rows:
   - [0.2]
 ...
@@ -63,7 +63,7 @@ test_run:cmd("setopt delimiter ';'")
 ...
 box.schema.func.create('WAITFOR', {language = 'Lua',
                    body = 'function (n) fiber.sleep(n) return n end',
-                   param_list = {'integer'}, returns = 'integer',
+                   param_list = {'number'}, returns = 'number',
                    exports = {'LUA', 'SQL'}})
 test_run:cmd("setopt delimiter ''");
 ---
diff --git a/test/sql/func-recreate.test.lua b/test/sql/func-recreate.test.lua
index 5496baf6e..0b32ea9ae 100644
--- a/test/sql/func-recreate.test.lua
+++ b/test/sql/func-recreate.test.lua
@@ -7,7 +7,7 @@ fiber = require('fiber')
 test_run:cmd("setopt delimiter ';'")
 box.schema.func.create('WAITFOR', {language = 'Lua',
                        body = 'function (n) fiber.sleep(n) return n end',
-                       param_list = {'integer'}, returns = 'integer',
+                       param_list = {'number'}, returns = 'number',
                        exports = {'LUA', 'SQL'}})
 test_run:cmd("setopt delimiter ''");
 
@@ -21,7 +21,7 @@ box.func.WAITFOR:drop()
 test_run:cmd("setopt delimiter ';'")
 box.schema.func.create('WAITFOR', {language = 'Lua',
                        body = 'function (n) fiber.sleep(n) return n end',
-                       param_list = {'integer'}, returns = 'integer',
+                       param_list = {'number'}, returns = 'number',
                        exports = {'LUA', 'SQL'}})
 test_run:cmd("setopt delimiter ''");
 ch:get()
@@ -30,7 +30,7 @@ box.func.WAITFOR:drop()
 test_run:cmd("setopt delimiter ';'")
 box.schema.func.create('WAITFOR', {language = 'Lua',
                    body = 'function (n) fiber.sleep(n) return n end',
-                   param_list = {'integer'}, returns = 'integer',
+                   param_list = {'number'}, returns = 'number',
                    exports = {'LUA', 'SQL'}})
 test_run:cmd("setopt delimiter ''");
 
-- 
2.23.0





More information about the Tarantool-patches mailing list