[Tarantool-patches] [PATCH v4 28/53] sql: refactor mem_set_*_string()

[Vladislav Shpilevoy]

Thanks for the patch!

See 2 comments below.

1. I think it would be better to have mem_set_str_* as a prefix
for these functions. 'string' is too long, and by placing allocation
type in the beginning you complicate grep by mem_set_str_* and
autocompletion when I type 'mem_set_str_' and want to see all the
options.

On 23.03.2021 10:35, Mergen Imeev via Tarantool-patches wrote:
> This patch introduces mem_set_*_string() functions. Functions
> mem_set_*_string() clears MEM and sets it to given string value.
> 
> Part of #5818
> ---
>  src/box/sql/mem.c     | 111 +++++++++++++++++++++++++++++++++---------
>  src/box/sql/mem.h     |  26 +++++++++-
>  src/box/sql/sqlInt.h  |   4 --
>  src/box/sql/vdbe.c    |   5 +-
>  src/box/sql/vdbeapi.c |  47 ++++++++++++++----
>  src/box/sql/vdbeaux.c |  45 +++++++----------
>  6 files changed, 171 insertions(+), 67 deletions(-)
> 
> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
> index 47a71fb30..91ef7f3c8 100644
> --- a/src/box/sql/mem.c
> +++ b/src/box/sql/mem.c
> @@ -2780,13 +2849,9 @@ vdbe_decode_msgpack_into_ephemeral_mem(const char *buf, struct Mem *mem,
>  		break;
>  	}
>  	case MP_STR: {
> -		/* XXX u32->int */
> -		mem->n = (int) mp_decode_strl(&buf);
> -		mem->flags = MEM_Str | MEM_Ephem;
> -		mem->field_type = FIELD_TYPE_STRING;
> -install_blob:
> -		mem->z = (char *)buf;
> -		buf += mem->n;
> +		uint32_t len = mp_decode_strl(&buf);
> +		mem_set_ephemeral_string(mem, (char *)buf, len);

2. It adds clear() call which is not necessary here. I would
propose to keep the old code for the sake of speed and
consistency with the other mp types.