From: Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH v1 8/8] sql: refactor LIKE() function Date: Wed, 20 Oct 2021 20:19:38 +0300 [thread overview] Message-ID: <20211020171938.GG203963@tarantool.org> (raw) In-Reply-To: <762d321c-004d-4ef9-7d8c-1314c19fc673@tarantool.org> Thank you for the review! My answer, diff and new patch below. Also, I replaced self-created and SQLite3 functions to work with UTF8 characters by iucnv_getNextUChar(). And I replaced sql_result_bool() by mem_set_bool(), so I will be able to drop sql_result_bool() later. On Sat, Oct 09, 2021 at 12:02:35AM +0200, Vladislav Shpilevoy wrote: > Thanks for the patch! > > > diff --git a/src/box/sql/func.c b/src/box/sql/func.c > > index 1b4d52225..27106ced6 100644 > > --- a/src/box/sql/func.c > > +++ b/src/box/sql/func.c > > @@ -1281,46 +1281,28 @@ likeFunc(sql_context *context, int argc, struct Mem *argv) > > { > > u32 escape = SQL_END_OF_STRING; > > int nPat; > > - if (argc != 2 && argc != 3) { > > - diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, > > - "LIKE", "2 or 3", argc); > > - context->is_aborted = true; > > - return; > > - } > > - sql *db = sql_context_db_handle(context); > > - int rhs_type = sql_value_type(&argv[0]); > > - int lhs_type = sql_value_type(&argv[1]); > > - > > - if (lhs_type != MP_STR || rhs_type != MP_STR) { > > - if (lhs_type == MP_NIL || rhs_type == MP_NIL) > > - return; > > - const char *str = rhs_type != MP_STR ? > > - mem_str(&argv[0]) : mem_str(&argv[1]); > > - diag_set(ClientError, ER_INCONSISTENT_TYPES, "string", str); > > - context->is_aborted = true; > > + assert(argc == 2 || argc == 3); > > + struct sql *db = sql_context_db_handle(context); > > You can get the db after the is_null check below. It is not > used when the check passes. > Fixed. Dropped this line and used sql_get() instead. > > + if (mem_is_any_null(&argv[0], &argv[1])) > > return; Diff: diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 90417c892..afe34f7f0 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -1261,7 +1261,6 @@ likeFunc(sql_context *context, int argc, struct Mem *argv) u32 escape = SQL_END_OF_STRING; int nPat; assert(argc == 2 || argc == 3); - struct sql *db = sql_context_db_handle(context); if (mem_is_any_null(&argv[0], &argv[1])) return; assert(mem_is_str(&argv[0]) && mem_is_str(&argv[1])); @@ -1276,7 +1275,7 @@ likeFunc(sql_context *context, int argc, struct Mem *argv) * sql_utf8_pattern_compare(). */ nPat = argv[0].n; - if (nPat > db->aLimit[SQL_LIMIT_LIKE_PATTERN_LENGTH]) { + if (nPat > sql_get()->aLimit[SQL_LIMIT_LIKE_PATTERN_LENGTH]) { diag_set(ClientError, ER_SQL_EXECUTE, "LIKE pattern is too "\ "complex"); context->is_aborted = true; @@ -1284,21 +1283,23 @@ likeFunc(sql_context *context, int argc, struct Mem *argv) } if (argc == 3) { + if (mem_is_null(&argv[2])) + return; /* * The escape character string must consist of a * single UTF-8 character. Otherwise, return an * error. */ - const unsigned char *zEsc = (const unsigned char *)argv[2].z; - if (zEsc == 0) - return; - if (sql_utf8_char_count(zEsc, argv[2].n) != 1) { + UErrorCode err = U_ZERO_ERROR; + const char *pos = argv[2].z; + const char *end = argv[2].z + argv[2].n; + escape = ucnv_getNextUChar(icu_utf8_conv, &pos, end, &err); + if (pos != end || err != U_ZERO_ERROR) { diag_set(ClientError, ER_SQL_EXECUTE, "ESCAPE "\ "expression must be a single character"); context->is_aborted = true; return; } - escape = sqlUtf8Read(&zEsc); } if (!zA || !zB) return; @@ -1313,7 +1314,7 @@ likeFunc(sql_context *context, int argc, struct Mem *argv) context->is_aborted = true; return; } - sql_result_bool(context, res == MATCH); + mem_set_bool(context->pOut, res == MATCH); } /** New patch: commit a739926ff3178d499650cff67c8597ff21e18b3c Author: Mergen Imeev <imeevma@gmail.com> Date: Mon Sep 27 19:29:28 2021 +0300 sql: refactor LIKE() function Part of #4145 diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 65bf03250..afe34f7f0 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -1260,63 +1260,46 @@ likeFunc(sql_context *context, int argc, struct Mem *argv) { u32 escape = SQL_END_OF_STRING; int nPat; - if (argc != 2 && argc != 3) { - diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, - "LIKE", "2 or 3", argc); - context->is_aborted = true; - return; - } - sql *db = sql_context_db_handle(context); - int rhs_type = sql_value_type(&argv[0]); - int lhs_type = sql_value_type(&argv[1]); - - if (lhs_type != MP_STR || rhs_type != MP_STR) { - if (lhs_type == MP_NIL || rhs_type == MP_NIL) - return; - const char *str = rhs_type != MP_STR ? - mem_str(&argv[0]) : mem_str(&argv[1]); - diag_set(ClientError, ER_INCONSISTENT_TYPES, "string", str); - context->is_aborted = true; + assert(argc == 2 || argc == 3); + if (mem_is_any_null(&argv[0], &argv[1])) return; - } - const char *zB = mem_as_str0(&argv[0]); - const char *zA = mem_as_str0(&argv[1]); - const char *zB_end = zB + mem_len_unsafe(&argv[0]); - const char *zA_end = zA + mem_len_unsafe(&argv[1]); + assert(mem_is_str(&argv[0]) && mem_is_str(&argv[1])); + const char *zB = argv[0].z; + const char *zA = argv[1].z; + const char *zB_end = zB + argv[0].n; + const char *zA_end = zA + argv[1].n; /* * Limit the length of the LIKE pattern to avoid problems * of deep recursion and N*N behavior in * sql_utf8_pattern_compare(). */ - nPat = mem_len_unsafe(&argv[0]); - testcase(nPat == db->aLimit[SQL_LIMIT_LIKE_PATTERN_LENGTH]); - testcase(nPat == db->aLimit[SQL_LIMIT_LIKE_PATTERN_LENGTH] + 1); - if (nPat > db->aLimit[SQL_LIMIT_LIKE_PATTERN_LENGTH]) { + nPat = argv[0].n; + if (nPat > sql_get()->aLimit[SQL_LIMIT_LIKE_PATTERN_LENGTH]) { diag_set(ClientError, ER_SQL_EXECUTE, "LIKE pattern is too "\ "complex"); context->is_aborted = true; return; } - /* Encoding did not change */ - assert(zB == mem_as_str0(&argv[0])); if (argc == 3) { + if (mem_is_null(&argv[2])) + return; /* * The escape character string must consist of a * single UTF-8 character. Otherwise, return an * error. */ - const unsigned char *zEsc = mem_as_ustr(&argv[2]); - if (zEsc == 0) - return; - if (sql_utf8_char_count(zEsc, mem_len_unsafe(&argv[2])) != 1) { + UErrorCode err = U_ZERO_ERROR; + const char *pos = argv[2].z; + const char *end = argv[2].z + argv[2].n; + escape = ucnv_getNextUChar(icu_utf8_conv, &pos, end, &err); + if (pos != end || err != U_ZERO_ERROR) { diag_set(ClientError, ER_SQL_EXECUTE, "ESCAPE "\ "expression must be a single character"); context->is_aborted = true; return; } - escape = sqlUtf8Read(&zEsc); } if (!zA || !zB) return; @@ -1331,7 +1314,7 @@ likeFunc(sql_context *context, int argc, struct Mem *argv) context->is_aborted = true; return; } - sql_result_bool(context, res == MATCH); + mem_set_bool(context->pOut, res == MATCH); } /**
next prev parent reply other threads:[~2021-10-20 17:19 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 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 [this message] 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 8/8] sql: refactor LIKE() function 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=20211020171938.GG203963@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=imeevma@tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v1 8/8] sql: refactor LIKE() 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