Re: [Tarantool-patches] [PATCH v1 5/8] sql: rework TRIM() function

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

Thanks for the fixes!

See 3 comments below.

> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index d36c83501..1294ff5b3 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -344,6 +344,178 @@ func_nullif(struct sql_context *ctx, int argc, struct Mem *argv)
>  		ctx->is_aborted = true;
>  }
>  
> +static inline void
> +return_empty_str(struct sql_context *ctx, bool is_str)

1. It is called in a single place with is_str = false. Could we maybe
inline it?

> +{
> +	return is_str ? mem_set_str_static(ctx->pOut, "", 0) :
> +	       mem_set_bin_static(ctx->pOut, "", 0);
> +}

...

> +
> +static void
> +func_trim_str(struct sql_context *ctx, int argc, struct Mem *argv)
> +{
> +	if (mem_is_null(&argv[0]) || (argc == 3 && mem_is_null(&argv[2])))
> +		return;
> +	assert(argc == 2 || (argc == 3 && mem_is_str(&argv[2])));
> +	assert(mem_is_str(&argv[0]) && mem_is_uint(&argv[1]));
> +	const char *str = argv[0].z;
> +	int size = argv[0].n;
> +	const char *chars;
> +	int chars_size;
> +	if (argc == 3) {
> +		chars = argv[2].z;
> +		chars_size = argv[2].n;
> +	} else {
> +		chars = " ";
> +		chars_size = 1;
> +	}
> +
> +	uint8_t *chars_len = sqlDbMallocRawNN(sql_get(),
> +					      chars_size * sizeof(uint8_t));

2. Could use fiber region here. Up to you.

> +	if (chars_len == NULL) {
> +		ctx->is_aborted = true;
> +		return;
> +	}
> +	int chars_count = 0;
> +
> +	UErrorCode err = U_ZERO_ERROR;
> +	const char *pos_start = chars;
> +	const char *pos_end = chars + chars_size;
> +	while (pos_start < pos_end) {
> +		const char *cur = pos_start;
> +		ucnv_getNextUChar(icu_utf8_conv, &pos_start, pos_end, &err);
> +		chars_len[chars_count++] = pos_start - cur;
> +	}
> +
> +	uint64_t flags = argv[1].u.u;
> +	int end = trim_str_end(str, size, chars, chars_len, chars_count, flags);
> +	int start = trim_str_start(str, end, chars, chars_len, chars_count,
> +		    flags);

3. The second line of the call is misaligned a bit.