Re: [Tarantool-patches] [PATCH v1 1/1] sql: fix error on copy empty string in mem_copy()

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi! Thanks for the patch!

See 3 comments below.

> diff --git a/test/sql-tap/gh-6157-unnecessary-free-on-string.c b/test/sql-tap/gh-6157-unnecessary-free-on-string.c
> new file mode 100644
> index 000000000..ce928d494
> --- /dev/null
> +++ b/test/sql-tap/gh-6157-unnecessary-free-on-string.c
> @@ -0,0 +1,10 @@
> +#include "msgpuck.h"
> +#include "module.h"
> +
> +int
> +f(box_function_ctx_t* ctx, const char* args, const char* args_end)

1. '*' must be right before variable name, not after the types.

> +{
> +	char res[16];
> +	char *end = mp_encode_str(res, "stub", strlen("stub"));
> +	return box_return_mp(ctx, res, end);
> +}

2. Why do you need a C function? Can't you reproduce it with Lua?

> diff --git a/test/sql-tap/gh-6157-unnecessary-free-on-string.test.lua b/test/sql-tap/gh-6157-unnecessary-free-on-string.test.lua
> new file mode 100755
> index 000000000..326570aea
> --- /dev/null
> +++ b/test/sql-tap/gh-6157-unnecessary-free-on-string.test.lua
> @@ -0,0 +1,38 @@
> +#!/usr/bin/env tarantool
> +local build_path = os.getenv("BUILDDIR")
> +package.cpath = build_path..'/test/sql-tap/?.so;'..build_path..'/test/sql-tap/?.dylib;'..package.cpath
> +
> +local test = require("sqltester")
> +test:plan(1)
> +
> +box.schema.func.create("gh-6157.f", {
> +    language = "C",
> +    param_list = {"string"},
> +    returns = "string",
> +    exports = {"SQL"}
> +})
> +
> +box.execute([[CREATE TABLE ts(s STRING PRIMARY KEY);]])
> +box.execute([[INSERT INTO ts VALUES ('');]])
> +box.execute([[CREATE TABLE ti(i INT PRIMARY KEY);]])
> +for i = 1, 100 do
> +    box.execute([[INSERT INTO ti VALUES(]]..i..[[);]])

3. Why do you need a space to reproduce this bug? It does not seem related.