From 439a48f26c03647feda0c1ee73f7f06eb99e71bb Mon Sep 17 00:00:00 2001
From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Date: Fri, 13 Sep 2019 10:52:05 +0300
Subject: [PATCH] sql: check returned value type for UDF
All user-defined functions feature type of returned value (if it
is not specified during function creation, it is assumed to be
ANY), but currently it is ignored. This patch introduces check
which verifies that returned value is of specified in function's
definition type. There's no attempt at implicit conversion to
specified (target) type - returned value must be literally of
the specified type.
Closes #4387
---
src/box/errcode.h | 1 +
src/box/field_def.h | 17 ++++-
src/box/sql/vdbe.c | 11 +++
test/box/misc.result | 1 +
test/sql-tap/func.test.lua | 123 +++++++++++++++++++++++++++++++-
test/sql/func-recreate.result | 8 +--
test/sql/func-recreate.test.lua | 6 +-
7 files changed, 157 insertions(+), 10 deletions(-)
diff --git a/src/box/errcode.h b/src/box/errcode.h
index 05bc46e9b2..c660b1c704 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -257,6 +257,7 @@ struct errcode_record {
/*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_SQL_FUNC_WRONG_RET_COUNT, "SQL expects exactly one argument returned from %s, got %d")\
+ /*205 */_(ER_FUNC_INVALID_RETURN_TYPE, "Function '%s' returned value of invalid type: 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 fdedc9622d..49c2998f57 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 9495ada37f..f881a732e3 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1843,6 +1843,11 @@ case OP_FunctionByName: {
diag_set(ClientError, ER_NO_SUCH_FUNCTION, pOp->p4.z);
goto abort_due_to_error;
}
+ /*
+ * Function call may yield so pointer to the function may
+ * turn out to be invalid after call.
+ */
+ enum field_type returns = func->def->returns;
int argc = pOp->p5;
struct Mem *argv = &aMem[pOp->p2];
struct port args, ret;
@@ -1862,6 +1867,12 @@ case OP_FunctionByName: {
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, false)) {
+ diag_set(ClientError, ER_FUNC_INVALID_RETURN_TYPE, 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/box/misc.result b/test/box/misc.result
index 784ff4686a..b2930515b5 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -534,6 +534,7 @@ t;
202: box.error.FUNC_WRONG_ARG_COUNT
203: box.error.BOOTSTRAP_READONLY
204: box.error.SQL_FUNC_WRONG_RET_COUNT
+ 205: box.error.FUNC_INVALID_RETURN_TYPE
...
test_run:cmd("setopt delimiter ''");
---
diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua
index 39d9ccf2d9..e65fba1509 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(14610)
+test:plan(14694)
--!./tcltestrunner.lua
-- 2001 September 15
@@ -2952,4 +2952,125 @@ test:do_catchsql_test(
-- </func-76.4>
})
+ffi = require('ffi')
+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.1", "SELECT TOSTRING('1');", {'1'})
+test:do_execsql_test("func-77.2", "SELECT TOSTRING('a');", {'a'})
+test:do_catchsql_test("func-77.3", "SELECT TOSTRING(1);", {1, "Function 'TOSTRING' returned value of invalid type: expected string got unsigned"})
+test:do_catchsql_test("func-77.4", "SELECT TOSTRING(-1);", {1, "Function 'TOSTRING' returned value of invalid type: expected string got integer"})
+test:do_catchsql_test("func-77.5", "SELECT TOSTRING(-1.1);", {1, "Function 'TOSTRING' returned value of invalid type: expected string got double"})
+test:do_catchsql_test("func-77.6", "SELECT TOSTRING(TRUE);", {1, "Function 'TOSTRING' returned value of invalid type: expected string got boolean"})
+test:do_catchsql_test("func-77.7", "SELECT TOSTRING(NULL);", {1, "Function 'TOSTRING' returned value of invalid type: expected string got nil"})
+test:do_catchsql_test("func-77.8", "SELECT TOSTRING(LUA('return nil'));", {1, "Function 'TOSTRING' returned value of invalid type: expected string got nil"})
+test:do_catchsql_test("func-77.9", "SELECT TOSTRING(LUA('return box.NULL'));", {1, "Function 'TOSTRING' returned value of invalid type: expected string got nil"})
+test:do_catchsql_test("func-77.10", "SELECT TOSTRING(LUA('return ffi.new(\"unsigned\", 666)'));", {1, "Function 'TOSTRING' returned value of invalid type: expected string got unsigned"})
+test:do_catchsql_test("func-77.11", "SELECT TOSTRING(LUA('return ffi.new(\"int\", -666)'));", {1, "Function 'TOSTRING' returned value of invalid type: expected string got integer"})
+test:do_catchsql_test("func-77.12", "SELECT TOSTRING(LUA('return ffi.new(\"double\", -666.666)'));", {1, "Function 'TOSTRING' returned value of invalid type: expected string got double"})
+test:do_catchsql_test("func-77.14", "SELECT TOSTRING(LUA('return ffi.new(\"bool\", true)'));", {1, "Function 'TOSTRING' returned value of invalid type: expected string got boolean"})
+test:do_execsql_test("func-77.13", "SELECT TOSTRING(LUA('return ffi.string(\"hello\", 5)'));", {'hello'})
+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-78.1", "SELECT TOUNSIGNED('1');", {1, "Function 'TOUNSIGNED' returned value of invalid type: expected unsigned got string"})
+test:do_catchsql_test("func-78.2", "SELECT TOUNSIGNED('a');", {1, "Function 'TOUNSIGNED' returned value of invalid type: expected unsigned got string"})
+test:do_execsql_test("func-78.3", "SELECT TOUNSIGNED(1);", {1})
+test:do_catchsql_test("func-78.4", "SELECT TOUNSIGNED(-1);", {1, "Function 'TOUNSIGNED' returned value of invalid type: expected unsigned got integer"})
+test:do_catchsql_test("func-78.5", "SELECT TOUNSIGNED(-1.1);", {1, "Function 'TOUNSIGNED' returned value of invalid type: expected unsigned got double"})
+test:do_catchsql_test("func-78.6", "SELECT TOUNSIGNED(TRUE);", {1, "Function 'TOUNSIGNED' returned value of invalid type: expected unsigned got boolean"})
+test:do_catchsql_test("func-78.7", "SELECT TOUNSIGNED(NULL);", {1, "Function 'TOUNSIGNED' returned value of invalid type: expected unsigned got nil"})
+test:do_catchsql_test("func-78.8", "SELECT TOUNSIGNED(LUA('return nil'));", {1, "Function 'TOUNSIGNED' returned value of invalid type: expected unsigned got nil"})
+test:do_catchsql_test("func-78.9", "SELECT TOUNSIGNED(LUA('return box.NULL'));", {1, "Function 'TOUNSIGNED' returned value of invalid type: expected unsigned got nil"})
+test:do_execsql_test("func-78.10", "SELECT TOUNSIGNED(LUA('return ffi.new(\"unsigned\", 666)'));", {666})
+test:do_catchsql_test("func-78.11", "SELECT TOUNSIGNED(LUA('return ffi.new(\"int\", -666)'));", {1, "Function 'TOUNSIGNED' returned value of invalid type: expected unsigned got integer"})
+test:do_catchsql_test("func-78.12", "SELECT TOUNSIGNED(LUA('return ffi.new(\"double\", -666.666)'));", {1, "Function 'TOUNSIGNED' returned value of invalid type: expected unsigned got double"})
+test:do_catchsql_test("func-78.14", "SELECT TOUNSIGNED(LUA('return ffi.new(\"bool\", true)'));", {1, "Function 'TOUNSIGNED' returned value of invalid type: expected unsigned got boolean"})
+test:do_catchsql_test("func-78.13", "SELECT TOUNSIGNED(LUA('return ffi.string(\"hello\", 5)'));", {1, "Function 'TOUNSIGNED' returned value of invalid type: expected unsigned got string"})
+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-79.1", "SELECT TOINTEGER('1');", {1, "Function 'TOINTEGER' returned value of invalid type: expected integer got string"})
+test:do_catchsql_test("func-79.2", "SELECT TOINTEGER('a');", {1, "Function 'TOINTEGER' returned value of invalid type: expected integer got string"})
+test:do_execsql_test("func-79.3", "SELECT TOINTEGER(1);", {1})
+test:do_execsql_test("func-79.4", "SELECT TOINTEGER(-1);", {-1})
+test:do_catchsql_test("func-79.5", "SELECT TOINTEGER(-1.1);", {1, "Function 'TOINTEGER' returned value of invalid type: expected integer got double"})
+test:do_catchsql_test("func-79.6", "SELECT TOINTEGER(TRUE);", {1, "Function 'TOINTEGER' returned value of invalid type: expected integer got boolean"})
+test:do_catchsql_test("func-79.7", "SELECT TOINTEGER(NULL);", {1, "Function 'TOINTEGER' returned value of invalid type: expected integer got nil"})
+test:do_catchsql_test("func-79.8", "SELECT TOINTEGER(LUA('return nil'));", {1, "Function 'TOINTEGER' returned value of invalid type: expected integer got nil"})
+test:do_catchsql_test("func-79.9", "SELECT TOINTEGER(LUA('return box.NULL'));", {1, "Function 'TOINTEGER' returned value of invalid type: expected integer got nil"})
+test:do_execsql_test("func-79.10", "SELECT TOINTEGER(LUA('return ffi.new(\"unsigned\", 666)'));", {666})
+test:do_execsql_test("func-79.11", "SELECT TOINTEGER(LUA('return ffi.new(\"int\", -666)'));", {-666})
+test:do_catchsql_test("func-79.12", "SELECT TOINTEGER(LUA('return ffi.new(\"double\", -666.666)'));", {1, "Function 'TOINTEGER' returned value of invalid type: expected integer got double"})
+test:do_catchsql_test("func-79.14", "SELECT TOINTEGER(LUA('return ffi.new(\"bool\", true)'));", {1, "Function 'TOINTEGER' returned value of invalid type: expected integer got boolean"})
+test:do_catchsql_test("func-79.13", "SELECT TOINTEGER(LUA('return ffi.string(\"hello\", 5)'));", {1, "Function 'TOINTEGER' returned value of invalid type: expected integer got string"})
+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-80.1", "SELECT TONUMBER('1');", {1, "Function 'TONUMBER' returned value of invalid type: expected number got string"})
+test:do_catchsql_test("func-80.2", "SELECT TONUMBER('a');", {1, "Function 'TONUMBER' returned value of invalid type: expected number got string"})
+test:do_execsql_test("func-80.3", "SELECT TONUMBER(1);", {1})
+test:do_execsql_test("func-80.4", "SELECT TONUMBER(-1);", {-1})
+test:do_execsql_test("func-80.5", "SELECT TONUMBER(-1.1);", {-1.1})
+test:do_catchsql_test("func-80.6", "SELECT TONUMBER(TRUE);", {1, "Function 'TONUMBER' returned value of invalid type: expected number got boolean"})
+test:do_catchsql_test("func-80.7", "SELECT TONUMBER(NULL);", {1, "Function 'TONUMBER' returned value of invalid type: expected number got nil"})
+test:do_catchsql_test("func-80.8", "SELECT TONUMBER(LUA('return nil'));", {1, "Function 'TONUMBER' returned value of invalid type: expected number got nil"})
+test:do_catchsql_test("func-80.9", "SELECT TONUMBER(LUA('return box.NULL'));", {1, "Function 'TONUMBER' returned value of invalid type: expected number got nil"})
+test:do_execsql_test("func-80.10", "SELECT TONUMBER(LUA('return ffi.new(\"unsigned\", 666)'));", {666})
+test:do_execsql_test("func-80.11", "SELECT TONUMBER(LUA('return ffi.new(\"int\", -666)'));", {-666})
+test:do_execsql_test("func-80.12", "SELECT TONUMBER(LUA('return ffi.new(\"double\", -666.666)'));", {-666.666})
+test:do_catchsql_test("func-80.14", "SELECT TONUMBER(LUA('return ffi.new(\"bool\", true)'));", {1, "Function 'TONUMBER' returned value of invalid type: expected number got boolean"})
+test:do_catchsql_test("func-80.13", "SELECT TONUMBER(LUA('return ffi.string(\"hello\", 5)'));", {1, "Function 'TONUMBER' returned value of invalid type: expected number got string"})
+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-81.1", "SELECT TOBOOLEAN('1');", {1, "Function 'TOBOOLEAN' returned value of invalid type: expected boolean got string"})
+test:do_catchsql_test("func-81.2", "SELECT TOBOOLEAN('a');", {1, "Function 'TOBOOLEAN' returned value of invalid type: expected boolean got string"})
+test:do_catchsql_test("func-81.3", "SELECT TOBOOLEAN(1);", {1, "Function 'TOBOOLEAN' returned value of invalid type: expected boolean got unsigned"})
+test:do_catchsql_test("func-81.4", "SELECT TOBOOLEAN(-1);", {1, "Function 'TOBOOLEAN' returned value of invalid type: expected boolean got integer"})
+test:do_catchsql_test("func-81.5", "SELECT TOBOOLEAN(-1.1);", {1, "Function 'TOBOOLEAN' returned value of invalid type: expected boolean got double"})
+test:do_execsql_test("func-81.6", "SELECT TOBOOLEAN(TRUE);", {true})
+test:do_catchsql_test("func-81.7", "SELECT TOBOOLEAN(NULL);", {1, "Function 'TOBOOLEAN' returned value of invalid type: expected boolean got nil"})
+test:do_catchsql_test("func-81.8", "SELECT TOBOOLEAN(LUA('return nil'));", {1, "Function 'TOBOOLEAN' returned value of invalid type: expected boolean got nil"})
+test:do_catchsql_test("func-81.9", "SELECT TOBOOLEAN(LUA('return box.NULL'));", {1, "Function 'TOBOOLEAN' returned value of invalid type: expected boolean got nil"})
+test:do_catchsql_test("func-81.10", "SELECT TOBOOLEAN(LUA('return ffi.new(\"unsigned\", 666)'));", {1, "Function 'TOBOOLEAN' returned value of invalid type: expected boolean got unsigned"})
+test:do_catchsql_test("func-81.11", "SELECT TOBOOLEAN(LUA('return ffi.new(\"int\", -666)'));", {1, "Function 'TOBOOLEAN' returned value of invalid type: expected boolean got integer"})
+test:do_catchsql_test("func-81.12", "SELECT TOBOOLEAN(LUA('return ffi.new(\"double\", -666.666)'));", {1, "Function 'TOBOOLEAN' returned value of invalid type: expected boolean got double"})
+test:do_execsql_test("func-81.14", "SELECT TOBOOLEAN(LUA('return ffi.new(\"bool\", true)'));", {true})
+test:do_catchsql_test("func-81.13", "SELECT TOBOOLEAN(LUA('return ffi.string(\"hello\", 5)'));", {1, "Function 'TOBOOLEAN' returned value of invalid type: expected boolean got string"})
+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-82.1", "SELECT TOSCALAR('1');", {'1'})
+test:do_execsql_test("func-82.2", "SELECT TOSCALAR('a');", {'a'})
+test:do_execsql_test("func-82.3", "SELECT TOSCALAR(1);", {1})
+test:do_execsql_test("func-82.4", "SELECT TOSCALAR(-1);", {-1})
+test:do_execsql_test("func-82.5", "SELECT TOSCALAR(-1.1);", {-1.1})
+test:do_execsql_test("func-82.6", "SELECT TOSCALAR(TRUE);", {true})
+test:do_catchsql_test("func-82.7", "SELECT TOSCALAR(NULL);", {1, "Function 'TOSCALAR' returned value of invalid type: expected scalar got nil"})
+test:do_catchsql_test("func-82.8", "SELECT TOSCALAR(LUA('return nil'));", {1, "Function 'TOSCALAR' returned value of invalid type: expected scalar got nil"})
+test:do_catchsql_test("func-82.9", "SELECT TOSCALAR(LUA('return box.NULL'));", {1, "Function 'TOSCALAR' returned value of invalid type: expected scalar got nil"})
+test:do_execsql_test("func-82.10", "SELECT TOSCALAR(LUA('return ffi.new(\"unsigned\", 666)'));", {666})
+test:do_execsql_test("func-82.11", "SELECT TOSCALAR(LUA('return ffi.new(\"int\", -666)'));", {-666})
+test:do_execsql_test("func-82.12", "SELECT TOSCALAR(LUA('return ffi.new(\"double\", -666.666)'));", {-666.666})
+test:do_execsql_test("func-82.14", "SELECT TOSCALAR(LUA('return ffi.new(\"bool\", true)'));", {true})
+test:do_execsql_test("func-82.13", "SELECT TOSCALAR(LUA('return ffi.string(\"hello\", 5)'));", {"hello"})
+box.func.TOSCALAR:drop()
+
test:finish_test()
diff --git a/test/sql/func-recreate.result b/test/sql/func-recreate.result
index 1f69664316..a0a67a106d 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 5496baf6e3..0b32ea9aec 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 ''");
--
GitLab