[Tarantool-patches] [PATCH] box: make box.func.call consistent with netbox.call

["Maria Khaydich" <maria.khaydich@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 7690 bytes --]


C stored function always returns an array. Local call introduced
with box.func used to convert resulting array to multireturn in
contrast to netbox call that serializes it to a tuple.
This patch makes resulting behavior consistent by making the local
call always return a tuple as well.
 
Closes #4799
---
Issue:
https://github.com/tarantool/tarantool/issues/4799  
Branch:
https://github.com/tarantool/tarantool/commit/7745bc83a44a28cbbe5ad55378caaa15bd7bb210  
 
 src/box/func.c                              |  2 +-
 src/box/lua/misc.cc                         | 13 +++++++
 src/box/port.c                              | 23 ++++++++++++
 src/box/port.h                              |  4 +++
 test/box/function1.result                   | 16 ++++-----
 test/box/gh-4648-func-load-unload.result    |  2 ++
 test/box/gh-4799-c-stored-function.result   | 40 +++++++++++++++++++++
 test/box/gh-4799-c-stored-function.test.lua | 18 ++++++++++
 8 files changed, 109 insertions(+), 9 deletions(-)
 create mode 100644 test/box/gh-4799-c-stored-function.result
 create mode 100644 test/box/gh-4799-c-stored-function.test.lua
diff --git a/src/box/func.c b/src/box/func.c
index 8087c953f..be02c4b4d 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -567,7 +567,7 @@ func_c_call(struct func *base, struct port *args, struct port *ret)
     if (data == NULL)
         return -1;
 
-    port_c_create(ret);
+    port_c_not_flat_create(ret);
     box_function_ctx_t ctx = { ret };
 
     /* Module can be changed after function reload. */
diff --git a/src/box/lua/misc.cc b/src/box/lua/misc.cc
index 5da84b35a..16c0e3ab9 100644
--- a/src/box/lua/misc.cc
+++ b/src/box/lua/misc.cc
@@ -80,6 +80,19 @@ port_c_dump_lua(struct port *base, struct lua_State *L, bool is_flat)
     }
 }
 
+/**
+ * Third parameter (is_flat) should always be considered
+ * false because there is no way to detect whether dumped
+ * result is tuple or multireturn. That ensures behavior
+ * that is consistent with msgpack_dump which encodes
+ * returned tuple as an array.
+ */
+extern "C" void
+port_c_dump_lua_not_flat(struct port *base, struct lua_State *L, bool)
+{
+    port_c_dump_lua(base, L, false);
+}
+
 extern "C" void
 port_msgpack_dump_lua(struct port *base, struct lua_State *L, bool is_flat)
 {
diff --git a/src/box/port.c b/src/box/port.c
index e9e1bda7a..845061f29 100644
--- a/src/box/port.c
+++ b/src/box/port.c
@@ -203,6 +203,9 @@ port_c_dump_msgpack(struct port *base, struct obuf *out)
 extern void
 port_c_dump_lua(struct port *port, struct lua_State *L, bool is_flat);
 
+extern void
+port_c_dump_lua_not_flat(struct port *port, struct lua_State *L, bool is_flat);
+
 extern struct sql_value *
 port_c_get_vdbemem(struct port *base, uint32_t *size);
 
@@ -216,6 +219,26 @@ const struct port_vtab port_c_vtab = {
     .destroy = port_c_destroy,
 };
 
+const struct port_vtab port_c_not_flat_vtab = {
+    .dump_msgpack = port_c_dump_msgpack,
+    .dump_msgpack_16 = port_c_dump_msgpack_16,
+    .dump_lua = port_c_dump_lua_not_flat,
+    .dump_plain = NULL,
+    .get_msgpack = NULL,
+    .get_vdbemem = port_c_get_vdbemem,
+    .destroy = port_c_destroy,
+};
+
+void
+port_c_not_flat_create(struct port *base)
+{
+    struct port_c *port = (struct port_c *)base;
+    port->vtab = &port_c_not_flat_vtab;
+    port->first = NULL;
+    port->last = NULL;
+    port->size = 0;
+}
+
 void
 port_c_create(struct port *base)
 {
diff --git a/src/box/port.h b/src/box/port.h
index 43d0f9deb..9873742b5 100644
--- a/src/box/port.h
+++ b/src/box/port.h
@@ -140,6 +140,10 @@ static_assert(sizeof(struct port_c) <= sizeof(struct port),
 void
 port_c_create(struct port *base);
 
+/** Create a C port object. (always not flat) */
+void
+port_c_not_flat_create(struct port *base);
+
 /** Append a tuple to the port. Tuple is referenced. */
 int
 port_c_add_tuple(struct port *port, struct tuple *tuple);
diff --git a/test/box/function1.result b/test/box/function1.result
index 905a4cdab..a8e33e601 100644
--- a/test/box/function1.result
+++ b/test/box/function1.result
@@ -114,7 +114,7 @@ box.func["function1.args"]:call({ "xx" })
 ...
 box.func["function1.args"]:call({ 15 })
 ---
-- [15, 'hello']
+- - [15, 'hello']
 ...
 box.schema.func.drop("function1.args")
 ---
@@ -645,11 +645,11 @@ func:call({4})
 ...
 func:call({4, 2})
 ---
-- [2]
+- - [2]
 ...
 func:call({4, 2, 1})
 ---
-- [2]
+- - [2]
 ...
 func:drop()
 ---
@@ -802,11 +802,11 @@ box.schema.func.create(name, {language = "C", exports = {'LUA'}})
 ...
 box.func[name]:call()
 ---
-- 1
-- -1
-- 18446744073709551615
-- '123456789101112131415'
-- [2]
+- - 1
+  - -1
+  - 18446744073709551615
+  - '123456789101112131415'
+  - [2]
 ...
 box.schema.user.grant('guest', 'super')
 ---
diff --git a/test/box/gh-4648-func-load-unload.result b/test/box/gh-4648-func-load-unload.result
index e695dd365..3fa7eb9dc 100644
--- a/test/box/gh-4648-func-load-unload.result
+++ b/test/box/gh-4648-func-load-unload.result
@@ -59,6 +59,7 @@ check_module_count_diff(0)
  | ...
 box.func.function1:call()
  | ---
+ | - []
  | ...
 check_module_count_diff(1)
  | ---
@@ -100,6 +101,7 @@ end)
  | ...
 box.func.function1:call()
  | ---
+ | - []
  | ...
 check_module_count_diff(1)
  | ---
diff --git a/test/box/gh-4799-c-stored-function.result b/test/box/gh-4799-c-stored-function.result
new file mode 100644
index 000000000..b6df3d17a
--- /dev/null
+++ b/test/box/gh-4799-c-stored-function.result
@@ -0,0 +1,40 @@
+-- test-run result file version 2
+--
+-- gh-4799: C stored function's output used to be inconsistent:
+-- multireturn if called locally, a tuple if called via netbox.
+-- The latter was chosen as preferred behavior because it existed
+-- long before local call was introduced in box.func.
+--
+build_path = os.getenv("BUILDDIR")
+ | ---
+ | ...
+package.cpath = build_path..'/test/box/?.so;'..build_path..'/test/box/?.dylib;'..package.cpath
+ | ---
+ | ...
+box.schema.func.create('function1.multireturn', {language = "C", exports = {'LUA'}})
+ | ---
+ | ...
+box.schema.user.grant('guest', 'super')
+ | ---
+ | ...
+c = require('net.box').connect(box.cfg.listen)
+ | ---
+ | ...
+
+-- Both calls should return a tuple now.
+c:call('function1.multireturn')
+ | ---
+ | - [[1], [1]]
+ | ...
+box.func['function1.multireturn']:call()
+ | ---
+ | - - [1]
+ |   - [1]
+ | ...
+
+box.schema.user.revoke('guest', 'super')
+ | ---
+ | ...
+box.schema.func.drop('function1.multireturn')
+ | ---
+ | ...
diff --git a/test/box/gh-4799-c-stored-function.test.lua b/test/box/gh-4799-c-stored-function.test.lua
new file mode 100644
index 000000000..d0b3b942f
--- /dev/null
+++ b/test/box/gh-4799-c-stored-function.test.lua
@@ -0,0 +1,18 @@
+--
+-- gh-4799: C stored function's output used to be inconsistent:
+-- multireturn if called locally, a tuple if called via netbox.
+-- The latter was chosen as preferred behavior because it existed
+-- long before local call was introduced in box.func.
+--
+build_path = os.getenv("BUILDDIR")
+package.cpath = build_path..'/test/box/?.so;'..build_path..'/test/box/?.dylib;'..package.cpath
+box.schema.func.create('function1.multireturn', {language = "C", exports = {'LUA'}})
+box.schema.user.grant('guest', 'super')
+c = require('net.box').connect(box.cfg.listen)
+
+-- Both calls should return a tuple now.
+c:call('function1.multireturn')
+box.func['function1.multireturn']:call()
+
+box.schema.user.revoke('guest', 'super')
+box.schema.func.drop('function1.multireturn')
-- 
2.24.0
 
 
--
Maria Khaydich

[-- Attachment #2: Type: text/html, Size: 9723 bytes --]