From: Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: v.shpilevoy@tarantool.org, tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH v1 3/8] sql: refactor UPPER() and LOWER() functions Date: Wed, 20 Oct 2021 20:02:49 +0300 [thread overview] Message-ID: <20211020170249.GC203963@tarantool.org> (raw) In-Reply-To: <7ac6109c37c1bd073a3693010ef7eaecf14074a8.1633105483.git.imeevma@gmail.com> Thank you for the review! I replaced check of MEM type by mem_is_*() functions. Diff below. On Fri, Oct 01, 2021 at 07:29:29PM +0300, Mergen Imeev via Tarantool-patches wrote: > Part of #4145 > --- > src/box/sql/func.c | 118 +++++++++++++++++++++------------------------ > 1 file changed, 55 insertions(+), 63 deletions(-) > > diff --git a/src/box/sql/func.c b/src/box/sql/func.c > index 2e53b32d8..565ebcabe 100644 > --- a/src/box/sql/func.c > +++ b/src/box/sql/func.c > @@ -295,6 +295,59 @@ func_char_length(struct sql_context *ctx, int argc, struct Mem *argv) > mem_set_uint(ctx->pOut, utf8_len_str(arg->z, arg->n)); > } > > +/** Implementation of the UPPER() and LOWER() functions. */ > +static void > +func_lower_upper(struct sql_context *ctx, int argc, struct Mem *argv) > +{ > + assert(argc == 1); > + (void)argc; > + struct Mem *arg = &argv[0]; > + if (arg->type == MEM_TYPE_NULL) > + return; > + assert(arg->type == MEM_TYPE_STR && arg->n >= 0); > + if (arg->n == 0) > + return mem_set_str0_static(ctx->pOut, ""); > + const char *str = arg->z; > + int32_t len = arg->n; > + struct sql *db = sql_get(); > + char *res = sqlDbMallocRawNN(db, len); > + if (res == NULL) { > + ctx->is_aborted = true; > + return; > + } > + int32_t size = sqlDbMallocSize(db, res); > + assert(size >= len); > + UErrorCode status = U_ZERO_ERROR; > + const char *locale = NULL; > + if (ctx->coll != NULL && ctx->coll->type == COLL_TYPE_ICU) { > + locale = ucol_getLocaleByType(ctx->coll->collator, > + ULOC_VALID_LOCALE, &status); > + } > + UCaseMap *cm = ucasemap_open(locale, 0, &status); > + assert(cm != NULL); > + assert(ctx->func->def->name[0] == 'U' || > + ctx->func->def->name[0] == 'L'); > + bool is_upper = ctx->func->def->name[0] == 'U'; > + int32_t new_len = > + is_upper ? > + ucasemap_utf8ToUpper(cm, res, size, str, len, &status) : > + ucasemap_utf8ToLower(cm, res, size, str, len, &status); > + if (new_len > size) { > + res = sqlDbRealloc(db, res, new_len); > + if (db->mallocFailed != 0) { > + ctx->is_aborted = true; > + return; > + } > + status = U_ZERO_ERROR; > + if (is_upper) > + ucasemap_utf8ToUpper(cm, res, size, str, len, &status); > + else > + ucasemap_utf8ToLower(cm, res, size, str, len, &status); > + } > + ucasemap_close(cm); > + mem_set_str_allocated(ctx->pOut, res, new_len); > +} > + > static const unsigned char * > mem_as_ustr(struct Mem *mem) > { > @@ -808,67 +861,6 @@ contextMalloc(struct sql_context *context, i64 nByte) > return z; > } > > -/* > - * Implementation of the upper() and lower() SQL functions. > - */ > - > -#define ICU_CASE_CONVERT(case_type) \ > -static void \ > -case_type##ICUFunc(sql_context *context, int argc, struct Mem *argv) \ > -{ \ > - char *z1; \ > - const char *z2; \ > - int n; \ > - UNUSED_PARAMETER(argc); \ > - if (mem_is_bin(&argv[0]) || mem_is_map(&argv[0]) || \ > - mem_is_array(&argv[0])) { \ > - diag_set(ClientError, ER_INCONSISTENT_TYPES, "string", \ > - mem_str(&argv[0])); \ > - context->is_aborted = true; \ > - return; \ > - } \ > - z2 = mem_as_str0(&argv[0]); \ > - n = mem_len_unsafe(&argv[0]); \ > - /* \ > - * Verify that the call to _bytes() \ > - * does not invalidate the _text() pointer. \ > - */ \ > - assert(z2 == mem_as_str0(&argv[0])); \ > - if (!z2) \ > - return; \ > - z1 = contextMalloc(context, ((i64) n) + 1); \ > - if (z1 == NULL) { \ > - context->is_aborted = true; \ > - return; \ > - } \ > - UErrorCode status = U_ZERO_ERROR; \ > - struct coll *coll = context->coll; \ > - const char *locale = NULL; \ > - if (coll != NULL && coll->type == COLL_TYPE_ICU) { \ > - locale = ucol_getLocaleByType(coll->collator, \ > - ULOC_VALID_LOCALE, &status); \ > - } \ > - UCaseMap *case_map = ucasemap_open(locale, 0, &status); \ > - assert(case_map != NULL); \ > - int len = ucasemap_utf8To##case_type(case_map, z1, n, z2, n, &status); \ > - if (len > n) { \ > - status = U_ZERO_ERROR; \ > - sql_free(z1); \ > - z1 = contextMalloc(context, ((i64) len) + 1); \ > - if (z1 == NULL) { \ > - context->is_aborted = true; \ > - return; \ > - } \ > - ucasemap_utf8To##case_type(case_map, z1, len, z2, n, &status); \ > - } \ > - ucasemap_close(case_map); \ > - sql_result_text(context, z1, len, sql_free); \ > -} \ > - > -ICU_CASE_CONVERT(Lower); > -ICU_CASE_CONVERT(Upper); > - > - > /* > * Some functions like COALESCE() and IFNULL() and UNLIKELY() are implemented > * as VDBE code so that unused argument values do not have to be computed. > @@ -2007,7 +1999,7 @@ static struct sql_func_definition definitions[] = { > FIELD_TYPE_BOOLEAN, sql_builtin_stub, NULL}, > {"LIKELY", 1, {FIELD_TYPE_ANY}, FIELD_TYPE_BOOLEAN, sql_builtin_stub, > NULL}, > - {"LOWER", 1, {FIELD_TYPE_STRING}, FIELD_TYPE_STRING, LowerICUFunc, > + {"LOWER", 1, {FIELD_TYPE_STRING}, FIELD_TYPE_STRING, func_lower_upper, > NULL}, > > {"MAX", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_INTEGER, step_minmax, NULL}, > @@ -2082,7 +2074,7 @@ static struct sql_func_definition definitions[] = { > NULL}, > {"UNLIKELY", 1, {FIELD_TYPE_ANY}, FIELD_TYPE_BOOLEAN, sql_builtin_stub, > NULL}, > - {"UPPER", 1, {FIELD_TYPE_STRING}, FIELD_TYPE_STRING, UpperICUFunc, > + {"UPPER", 1, {FIELD_TYPE_STRING}, FIELD_TYPE_STRING, func_lower_upper, > NULL}, > {"UUID", 0, {}, FIELD_TYPE_UUID, sql_func_uuid, NULL}, > {"UUID", 1, {FIELD_TYPE_INTEGER}, FIELD_TYPE_UUID, sql_func_uuid, NULL}, > -- > 2.25.1 > Diff: diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 441bcb51d..d6c557c54 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -286,9 +286,9 @@ func_lower_upper(struct sql_context *ctx, int argc, struct Mem *argv) assert(argc == 1); (void)argc; struct Mem *arg = &argv[0]; - if (arg->type == MEM_TYPE_NULL) + if (mem_is_null(arg)) return; - assert(arg->type == MEM_TYPE_STR && arg->n >= 0); + assert(mem_is_str(arg) && arg->n >= 0); if (arg->n == 0) return mem_set_str0_static(ctx->pOut, ""); const char *str = arg->z;
next prev parent reply other threads:[~2021-10-20 17:02 UTC|newest] Thread overview: 36+ 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 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 [this message] 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 2021-11-11 10:45 Mergen Imeev via Tarantool-patches 2021-11-11 10:45 ` [Tarantool-patches] [PATCH v1 3/8] sql: refactor UPPER() and LOWER() functions Mergen Imeev 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=20211020170249.GC203963@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=imeevma@tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v1 3/8] sql: refactor UPPER() and LOWER() functions' \ /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