[Tarantool-patches] [PATCH] box: make box.func.call consistent with netbox.call
Maria Khaydich
maria.khaydich at tarantool.org
Mon Jun 15 21:53:19 MSK 2020
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20200615/3faa1199/attachment.html>
More information about the Tarantool-patches
mailing list