Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: imeevma@tarantool.org
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v1 2/8] sql: refactor CHAR_LENGTH() function
Date: Fri, 8 Oct 2021 23:56:59 +0200	[thread overview]
Message-ID: <a4a45019-137c-d7bd-52bd-509388b45eea@tarantool.org> (raw)
In-Reply-To: <9cc35ba4625d4e3017725c35fbc4a7ed90341917.1633105483.git.imeevma@gmail.com>

Thanks for the patch!

On 01.10.2021 18:29, imeevma@tarantool.org wrote:
> Part of #4145
> ---
>  src/box/sql/func.c | 38 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index 54b03f359..2e53b32d8 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -263,6 +263,38 @@ func_abs_double(struct sql_context *ctx, int argc, struct Mem *argv)
>  	mem_set_double(ctx->pOut, arg->u.r < 0 ? -arg->u.r : arg->u.r);
>  }
>  
> +/** Implementation of the CHAR_LENGTH() function. */
> +static inline uint8_t
> +utf8_len_char(char c)
> +{
> +	uint8_t u = (uint8_t)c;
> +	return 1 + (u >= 0xc2) + (u >= 0xe0) + (u >= 0xf0);

It is not that simple really. Consider either using the old
lengthFunc() and other sqlite utf8 helpers or use the approach
similar to utf8_len() in utf8.c. It uses ICU macro U8_NEXT()
and has handling for special symbols like U_SENTINEL.

Otherwise you are making already third version of functions to
work with utf8.

I would even prefer to refactor lengthFunc() to stop using sqlite
legacy and drop sqlite utf8 entirely, but I suspect it might be
not so trivial to do and should be done later.

  reply	other threads:[~2021-10-08 21:57 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-01 16:29 [Tarantool-patches] [PATCH v1 0/8] Rework standard function Mergen Imeev via Tarantool-patches
2021-10-01 16:29 ` [Tarantool-patches] [PATCH v1 1/8] sql: refactor ABS() funcion Mergen Imeev via Tarantool-patches
2021-10-08 21:55   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-20 16:52     ` Mergen Imeev via Tarantool-patches
2021-10-28 22:11       ` Vladislav Shpilevoy via Tarantool-patches
2021-11-01 10:11         ` Mergen Imeev via Tarantool-patches
2021-11-01 13:37           ` Vladislav Shpilevoy via Tarantool-patches
2021-10-01 16:29 ` [Tarantool-patches] [PATCH v1 2/8] sql: refactor CHAR_LENGTH() function Mergen Imeev via Tarantool-patches
2021-10-08 21:56   ` Vladislav Shpilevoy via Tarantool-patches [this message]
2021-10-20 16:58     ` Mergen Imeev via Tarantool-patches
2021-10-28 22:11       ` Vladislav Shpilevoy via Tarantool-patches
2021-11-01 10:20         ` Mergen Imeev via Tarantool-patches
2021-10-01 16:29 ` [Tarantool-patches] [PATCH v1 3/8] sql: refactor UPPER() and LOWER() functions Mergen Imeev via Tarantool-patches
2021-10-20 17:02   ` Mergen Imeev via Tarantool-patches
2021-10-01 16:29 ` [Tarantool-patches] [PATCH v1 4/8] sql: refactor NULLIF() function Mergen Imeev via Tarantool-patches
2021-10-01 16:29 ` [Tarantool-patches] [PATCH v1 5/8] sql: rework TRIM() function Mergen Imeev via Tarantool-patches
2021-10-20 17:05   ` Mergen Imeev via Tarantool-patches
2021-10-28 22:12     ` Vladislav Shpilevoy via Tarantool-patches
2021-11-01 10:35       ` Mergen Imeev via Tarantool-patches
2021-10-01 16:29 ` [Tarantool-patches] [PATCH v1 6/8] sql: rework POSITION() function Mergen Imeev via Tarantool-patches
2021-10-08 21:58   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-20 17:08     ` Mergen Imeev via Tarantool-patches
2021-11-01 10:41       ` Mergen Imeev via Tarantool-patches
2021-10-01 16:29 ` [Tarantool-patches] [PATCH v1 7/8] sql: rework SUBSTR() function Mergen Imeev via Tarantool-patches
2021-10-08 22:02   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-20 17:15     ` Mergen Imeev via Tarantool-patches
2021-10-28 22:13       ` Vladislav Shpilevoy via Tarantool-patches
2021-11-01 10:45         ` Mergen Imeev via Tarantool-patches
2021-10-01 16:29 ` [Tarantool-patches] [PATCH v1 8/8] sql: refactor LIKE() function Mergen Imeev via Tarantool-patches
2021-10-08 22:02   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-20 17:19     ` Mergen Imeev via Tarantool-patches
2021-11-01 10:48       ` Mergen Imeev via Tarantool-patches
2021-11-01 10:53         ` Mergen Imeev via Tarantool-patches
2021-10-04 13:32 ` [Tarantool-patches] [PATCH v1 0/8] Rework standard function Mergen Imeev via Tarantool-patches
2021-11-01 13:38 ` Vladislav Shpilevoy via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a4a45019-137c-d7bd-52bd-509388b45eea@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imeevma@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v1 2/8] sql: refactor CHAR_LENGTH() function' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox