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