From 50efe95e88f6a32cc4ea8f6e928deedb73bfb490 Mon Sep 17 00:00:00 2001
From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Date: Tue, 8 Oct 2019 11:26:17 +0300
Subject: [PATCH] sql: errors for UDFs returning too many values

This patch introduces handling the situation when UDF returns
too many values. Previously Tarantool used to silently use
the first value returned. Now an error is raised.

Moreover a test coverage is improved also for the situation when
no value is returned.

Needed for #4387
---
 src/box/errcode.h           |  1 +
 src/box/sql/func.c          | 20 +++++++++++---------
 test/box/function1.c        | 19 +++++++++++++++++++
 test/box/function1.result   | 35 ++++++++++++++++++++++++++++++++++-
 test/box/function1.test.lua | 12 ++++++++++++
 test/box/misc.result        |  1 +
 6 files changed, 78 insertions(+), 10 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index d5d396d878..05bc46e9b2 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_SQL_FUNC_WRONG_RET_COUNT,	"SQL expects exactly one argument returned from %s, got %d")\
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 60efd0d9aa..a710c368b3 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -242,12 +242,13 @@ port_lua_get_vdbemem(struct port *base, uint32_t *size)
 	struct port_lua *port = (struct port_lua *) base;
 	struct lua_State *L = port->L;
 	int argc = lua_gettop(L);
-	if (argc == 0) {
-		diag_set(ClientError, ER_SQL_EXECUTE,
-			 "No value was passed from Lua");
+	if (argc == 0 || argc > 1) {
+		diag_set(ClientError, ER_SQL_FUNC_WRONG_RET_COUNT, "Lua", argc);
 		return NULL;
 	}
 	*size = argc;
+	/** FIXME: Implement an ability to return a vector. */
+	assert(*size == 1);
 	struct region *region = &fiber()->gc;
 	size_t region_svp = region_used(region);
 	struct Mem *val = vdbemem_alloc_on_region(argc);
@@ -288,11 +289,12 @@ port_tuple_get_vdbemem(struct port *base, uint32_t *size)
 {
 	struct port_tuple *port = (struct port_tuple *)base;
 	*size = port->size;
-	if (*size == 0) {
-		diag_set(ClientError, ER_SQL_EXECUTE,
-			 "No value was passed from C");
+	if (*size == 0 || *size > 1) {
+		diag_set(ClientError, ER_SQL_FUNC_WRONG_RET_COUNT, "C", *size);
 		return NULL;
 	}
+	/** FIXME: Implement an ability to return a vector. */
+	assert(*size == 1);
 	struct region *region = &fiber()->gc;
 	size_t region_svp = region_used(region);
 	struct Mem *val = vdbemem_alloc_on_region(port->size);
@@ -321,10 +323,10 @@ port_tuple_get_vdbemem(struct port *base, uint32_t *size)
 			sqlVdbeMemSetDouble(&val[i], mp_decode_double(&data));
 			break;
 		case MP_INT:
-			mem_set_i64(val, mp_decode_int(&data));
+			mem_set_i64(&val[i], mp_decode_int(&data));
 			break;
 		case MP_UINT:
-			mem_set_u64(val, mp_decode_uint(&data));
+			mem_set_u64(&val[i], mp_decode_uint(&data));
 			break;
 		case MP_STR:
 			str = mp_decode_str(&data, &len);
@@ -333,7 +335,7 @@ port_tuple_get_vdbemem(struct port *base, uint32_t *size)
 				goto error;
 			break;
 		case MP_NIL:
-			sqlVdbeMemSetNull(val);
+			sqlVdbeMemSetNull(&val[i]);
 			break;
 		default:
 			diag_set(ClientError, ER_SQL_EXECUTE,
diff --git a/test/box/function1.c b/test/box/function1.c
index ee5a422b51..b0d983e2b5 100644
--- a/test/box/function1.c
+++ b/test/box/function1.c
@@ -12,6 +12,25 @@ function1(box_function_ctx_t *ctx, const char *args, const char *args_end)
 	return 0;
 }
 
+int
+multireturn(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+	char tuple_buf[512];
+	char *d = tuple_buf;
+	d = mp_encode_array(d, 1);
+	d = mp_encode_uint(d, 1);
+	assert(d <= tuple_buf + sizeof(tuple_buf));
+
+	box_tuple_format_t *fmt = box_tuple_format_default();
+	box_tuple_t *tuple_a = box_tuple_new(fmt, tuple_buf, d);
+	if (tuple_a == NULL)
+		return -1;
+	int rc = box_return_tuple(ctx, tuple_a);
+	if (rc != 0)
+		return rc;
+	return box_return_tuple(ctx, tuple_a);
+}
+
 int
 args(box_function_ctx_t *ctx, const char *args, const char *args_end)
 {
diff --git a/test/box/function1.result b/test/box/function1.result
index a41ca4e3cd..b91d63c515 100644
--- a/test/box/function1.result
+++ b/test/box/function1.result
@@ -532,7 +532,7 @@ box.execute('SELECT lua(\'return box.cfg\')')
 box.execute('SELECT lua(\'return box.cfg()\')')
 ---
 - null
-- 'Failed to execute SQL statement: No value was passed from Lua'
+- SQL expects exactly one argument returned from Lua, got 0
 ...
 box.execute('SELECT lua(\'return box.cfg.memtx_memory\')')
 ---
@@ -757,6 +757,39 @@ box.schema.func.drop('secret_leak')
 box.schema.func.drop('secret')
 ---
 ...
+-- UDF corner cases for SQL: no value returned, too many values returned
+box.execute("SELECT LUA('a = 1 + 1')")
+---
+- null
+- SQL expects exactly one argument returned from Lua, got 0
+...
+box.execute("SELECT LUA('return 1, 2')")
+---
+- null
+- SQL expects exactly one argument returned from Lua, got 2
+...
+box.schema.func.create('function1', {language = "C", exports = {'LUA', 'SQL'}})
+---
+...
+box.execute("SELECT \"function1\"()")
+---
+- null
+- SQL expects exactly one argument returned from C, got 0
+...
+box.schema.func.drop("function1")
+---
+...
+box.schema.func.create('function1.multireturn', {language = "C", exports = {'LUA', 'SQL'}})
+---
+...
+box.execute("SELECT \"function1.multireturn\"()")
+---
+- null
+- SQL expects exactly one argument returned from C, got 2
+...
+box.schema.func.drop("function1.multireturn")
+---
+...
 --
 -- gh-4182: Introduce persistent Lua functions.
 --
diff --git a/test/box/function1.test.lua b/test/box/function1.test.lua
index e576cbb6f6..b1841f3adb 100644
--- a/test/box/function1.test.lua
+++ b/test/box/function1.test.lua
@@ -256,6 +256,18 @@ box.schema.user.revoke('guest', 'execute', 'function', 'secret_leak')
 box.schema.func.drop('secret_leak')
 box.schema.func.drop('secret')
 
+-- UDF corner cases for SQL: no value returned, too many values returned
+box.execute("SELECT LUA('a = 1 + 1')")
+box.execute("SELECT LUA('return 1, 2')")
+
+box.schema.func.create('function1', {language = "C", exports = {'LUA', 'SQL'}})
+box.execute("SELECT \"function1\"()")
+box.schema.func.drop("function1")
+
+box.schema.func.create('function1.multireturn', {language = "C", exports = {'LUA', 'SQL'}})
+box.execute("SELECT \"function1.multireturn\"()")
+box.schema.func.drop("function1.multireturn")
+
 --
 -- gh-4182: Introduce persistent Lua functions.
 --
diff --git a/test/box/misc.result b/test/box/misc.result
index 33e5ce8868..784ff4686a 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -533,6 +533,7 @@ t;
   201: box.error.NO_SUCH_FIELD_NAME
   202: box.error.FUNC_WRONG_ARG_COUNT
   203: box.error.BOOTSTRAP_READONLY
+  204: box.error.SQL_FUNC_WRONG_RET_COUNT
 ...
 test_run:cmd("setopt delimiter ''");
 ---
-- 
GitLab