Re: [Tarantool-patches] [PATCH 3/3] box: introduce box_return_mp() public C function

[Igor Munkin <imun@tarantool.org>]

Vlad,

Thanks for the patch! LGTM except the one nit.

On 08.03.20, Vladislav Shpilevoy wrote:
> Closes #4641
> 
> @TarantoolBot document
> Title: box_return_mp() public C function
> 
> Stored C functions could return a result only via
> `box_return_tuple()` function. That made users create a tuple
> every time they wanted to return something from a C function.
> 
> Now public C API offers another way to return - `box_return_mp()`.
> It allows to return arbitrary MessagePack, not wrapped into a
> tuple object. This is simpler to use for small results like a
> number, boolean, or a short string. Besides, `box_return_mp()` is
> much faster than `box_return_tuple()`, especially for small
> MessagePack.
> 
> Note, that it is faster only if an alternative is to create a
> tuple by yourself. If an already existing tuple was obtained from
> an iterator, and you want to return it, then of course it is
> faster to return via `box_return_tuple()`, than via extraction of
> tuple data, and calling `box_return_mp()`.
> 
> Here is the function declaration from module.h:
> ```C
> /**
>  * Return MessagePack from a stored C procedure. The MessagePack
>  * is copied, so it is safe to free/reuse the passed arguments
>  * after the call.
>  * MessagePack is not validated, for the sake of speed. It is
>  * expected to be a single encoded object. An attempt to encode
>  * and return multiple objects without wrapping them into an
>  * MP_ARRAY or MP_MAP is undefined behaviour.
>  *
>  * \param ctx An opaque structure passed to the stored C procedure
>  *        by Tarantool.
>  * \param mp Begin of MessagePack.
>  * \param mp_end End of MessagePack.
>  * \retval -1 Error.
>  * \retval 0 Success.
>  */
> API_EXPORT int
> box_return_mp(box_function_ctx_t *ctx, const char *mp, const char *mp_end);
> ```
> ---
>  extra/exports               |  1 +
>  src/box/box.cc              |  6 ++++++
>  src/box/box.h               | 19 +++++++++++++++++++
>  test/box/function1.c        | 37 +++++++++++++++++++++++++++++++++++++
>  test/box/function1.result   | 31 +++++++++++++++++++++++++++++++
>  test/box/function1.test.lua | 14 ++++++++++++++
>  6 files changed, 108 insertions(+)
> 

<snipped>

> diff --git a/test/box/function1.result b/test/box/function1.result
> index b91d63c51..301f666ef 100644
> --- a/test/box/function1.result
> +++ b/test/box/function1.result
> @@ -791,6 +791,37 @@ box.schema.func.drop("function1.multireturn")
>  ---
>  ...
>  --
> +-- gh-4641: box_return_mp() C API to return arbitrary MessagePack
> +-- from C functions.
> +--
> +name = 'function1.test_return_mp'
> +---
> +...
> +box.schema.func.create(name, {language = "C", exports = {'LUA'}})
> +---
> +...
> +box.func[name]:call()
> +---
> +- 1
> +- -1
> +- 18446744073709551615
> +- '123456789101112131415'
> +- [2]
> +...
> +box.schema.user.grant('guest', 'super')
> +---
> +...

Minor: I guess it worth to add a comment below referring the #4799 since
I was confused with the output below.

> +net:connect(box.cfg.listen):call(name)
> +---
> +- [1, -1, 18446744073709551615, '123456789101112131415', [2]]
> +...
> +box.schema.user.revoke('guest', 'super')
> +---
> +...
> +box.schema.func.drop(name)
> +---
> +...
> +--
>  -- gh-4182: Introduce persistent Lua functions.
>  --
>  test_run:cmd("setopt delimiter ';'")

<snipped>

> -- 
> 2.21.1 (Apple Git-122.3)
> 

-- 
Best regards,
IM