[tarantool-patches] Re: [PATCH v1 1/3] sql: errors for UDFs returning too many values

[Kirill Shcherbatov]

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/sql/func.c          | 16 +++++++++-------
 test/box/function1.c        | 19 +++++++++++++++++++
 test/box/function1.result   | 33 +++++++++++++++++++++++++++++++++
 test/box/function1.test.lua | 12 ++++++++++++
 4 files changed, 73 insertions(+), 7 deletions(-)

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 60efd0d9a..551613908 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -242,9 +242,10 @@ 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) {
+	if (argc == 0 || argc > 1) {
 		diag_set(ClientError, ER_SQL_EXECUTE,
-			 "No value was passed from Lua");
+			 argc == 0 ? "No value was passed from Lua" :
+				     "Too many values were returned from LUA");
 		return NULL;
 	}
 	*size = argc;
@@ -288,9 +289,10 @@ port_tuple_get_vdbemem(struct port *base, uint32_t *size)
 {
 	struct port_tuple *port = (struct port_tuple *)base;
 	*size = port->size;
-	if (*size == 0) {
+	if (*size == 0 || *size > 1) {
 		diag_set(ClientError, ER_SQL_EXECUTE,
-			 "No value was passed from C");
+			 *size == 0 ? "No value was passed from C" :
+				      "Too many values were returned from C");
 		return NULL;
 	}
 	struct region *region = &fiber()->gc;
@@ -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 ee5a422b5..b0d983e2b 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 a41ca4e3c..546f83369 100644
--- a/test/box/function1.result
+++ b/test/box/function1.result
@@ -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
+- 'Failed to execute SQL statement: No value was passed from Lua'
+...
+box.execute("SELECT LUA('return 1, 2')")
+---
+- null
+- 'Failed to execute SQL statement: Too many values were returned from LUA'
+...
+box.schema.func.create('function1', {language = "C", exports = {'LUA', 'SQL'}})
+---
+...
+box.execute("SELECT \"function1\"()")
+---
+- null
+- 'Failed to execute SQL statement: No value was passed from C'
+...
+box.schema.func.drop("function1")
+---
+...
+box.schema.func.create('function1.multireturn', {language = "C", exports = {'LUA', 'SQL'}})
+---
+...
+box.execute("SELECT \"function1.multireturn\"()")
+---
+- null
+- 'Failed to execute SQL statement: Too many values were returned from C'
+...
+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 e576cbb6f..b1841f3ad 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.
 --
-- 
2.23.0