[Tarantool-patches] [PATCH v4 44/53] sql: introduce mem_convert_to_string()

[Vladislav Shpilevoy]

Thanks for the patch!

See 3 comments below.

On 23.03.2021 10:36, imeevma at tarantool.org wrote:
> This patch introduces mem_convert_to_string(). This function is used to
> convert MEM to MEM contains string value.
> 
> Part of #5818
> ---
>  src/box/sql/mem.c | 197 ++++++++++++++++++++++++++++++----------------
>  src/box/sql/mem.h |   7 +-
>  2 files changed, 133 insertions(+), 71 deletions(-)
> 
> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
> index 8600b5c41..262f48aca 100644
> --- a/src/box/sql/mem.c
> +++ b/src/box/sql/mem.c
> @@ -818,6 +818,131 @@ mem_convert_to_number(struct Mem *mem)

<...>

> +
> +static inline int
> +mem_convert_boolean_to_string(struct Mem *mem)
> +{
> +	const char *str = mem->u.b ? "TRUE" : "FALSE";
> +	return mem_copy_string0(mem, str);

1. This can be a static string. Because the string
constants are not going anywhere, can ref them as is.

<...>

> +
> +int
> +mem_convert_to_string0(struct Mem *mem)
> +{
> +	if ((mem->flags & MEM_Str) != 0 && (mem->flags & MEM_Term) != 0)

2. Can be done in one check:

	(mem->flags & (MEM_Str | MEM_Term)) == (MEM_Str | MEM_Term)

> +		return 0;
> +	if ((mem->flags & MEM_Str) != 0)
> +		return mem_convert_string_to_string0(mem);
> +	if ((mem->flags & (MEM_Int | MEM_UInt)) != 0)
> +		return mem_convert_integer_to_string(mem);
> +	if ((mem->flags & MEM_Real) != 0)
> +		return mem_convert_double_to_string(mem);
> +	if ((mem->flags & MEM_Bool) != 0)
> +		return mem_convert_boolean_to_string(mem);
> +	if ((mem->flags & MEM_Blob) != 0) {
> +		if ((mem->flags & MEM_Subtype) == 0)
> +			return mem_convert_binary_to_string0(mem);
> +		if (mp_typeof(*mem->z) == MP_MAP)
> +			return mem_convert_map_to_string(mem);
> +		return mem_convert_array_to_string(mem);
> +	}
> +	return -1;
> +}
> +
> +int
> +mem_convert_to_string(struct Mem *mem)

3. Why would you need a not terminated string? The old function
always terminated the strings AFAIS.

> +{
> +	if ((mem->flags & MEM_Str) != 0)
> +		return 0;
> +	if ((mem->flags & (MEM_Int | MEM_UInt)) != 0)
> +		return mem_convert_integer_to_string(mem);
> +	if ((mem->flags & MEM_Real) != 0)
> +		return mem_convert_double_to_string(mem);
> +	if ((mem->flags & MEM_Bool) != 0)
> +		return mem_convert_boolean_to_string(mem);
> +	if ((mem->flags & MEM_Blob) != 0) {
> +		if ((mem->flags & MEM_Subtype) == 0)
> +			return mem_convert_binary_to_string(mem);
> +		if (mp_typeof(*mem->z) == MP_MAP)
> +			return mem_convert_map_to_string(mem);
> +		return mem_convert_array_to_string(mem);
> +	}
> +	return -1;
> +}
> +
>  int
>  mem_copy(struct Mem *to, const struct Mem *from)
>  {