From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 8FACB2ECC3 for ; Fri, 9 Nov 2018 22:38:33 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id oU00OyqtpiVl for ; Fri, 9 Nov 2018 22:38:33 -0500 (EST) Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id BB8712EC36 for ; Fri, 9 Nov 2018 22:38:32 -0500 (EST) From: Nikita Tatunov Message-Id: <34EFA3FE-95C1-49FC-9668-11DDD93EC42D@tarantool.org> Content-Type: multipart/alternative; boundary="Apple-Mail=_2251700A-BC41-45A6-86E6-198023A4FC8E" Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\)) Subject: [tarantool-patches] Re: [PATCH 2/2] sql: remove GLOB from Tarantool Date: Sat, 10 Nov 2018 06:38:23 +0300 In-Reply-To: <20181109121828.mjdoouycwvonimms@tkn_work_nb> References: <76466086-2a5f-8f12-cbc3-4ddf26e30fd9@tarantool.org> <32D1E5EA-EA21-4E4B-B5F5-80B6578BFBED@tarantool.org> <3f8354cb-4a47-c3de-164b-c776c792b991@tarantool.org> <090683CD-6F88-492D-AF48-631220A24E36@tarantool.org> <20181021034805.alwhmkpz3fbopqjz@tkn_work_nb> <20181029121529.2d6ccnh5qayiz7ld@tkn_work_nb> <6F5AC823-3016-4918-A588-6BB6B621B777@tarantool.org> <20181109121828.mjdoouycwvonimms@tkn_work_nb> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: Alexander Turenko Cc: tarantool-patches@freelists.org, korablev@tarantool.org --Apple-Mail=_2251700A-BC41-45A6-86E6-198023A4FC8E Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Hello! Thank you for comments. > On Nov 9, 2018, at 15:18, Alexander Turenko = wrote: >=20 > Hi! >=20 > See comments below. >=20 > WBR, Alexander Turenko. >=20 > On Thu, Nov 08, 2018 at 06:09:23PM +0300, Nikita Tatunov wrote: >> Hello! thank you for the review. >>=20 >> Issues: >> https://github.com/tarantool/tarantool/issues/3251 = >> https://github.com/tarantool/tarantool/issues/3334 = >>=20 >> Branch: >> = https://github.com/tarantool/tarantool/tree/N_Tatunov/gh-3251-where-like-h= angs = >>=20 >=20 > It seems you forgot to push the new changes. Ermm, did I? I have just opened the branch and it contains a commit which was pushed at 8th of November (and as I see it=E2=80=99s exactly = the commit from the diff) >=20 >> @@ -913,7 +899,7 @@ likeFunc(sqlite3_context *context, int argc, = sqlite3_value **argv) >> sqlite3_like_count++; >> #endif >> int res; >> - res =3D sql_utf8_pattern_compare(zB, zA, *is_like_ci, escape); >> + res =3D sql_utf8_pattern_compare(zB, zA, is_like_ci, escape); >> if (res =3D=3D SQL_INVALID_PATTERN) { >> const char *const err_msg =3D >> "LIKE pattern can only contain UTF-8 = characters"; >> @@ -1698,24 +1684,20 @@ setLikeOptFlag(sqlite3 * db, const char = *zName, u8 flagVal) >> * @retval none. >> */ >> void >> -sqlite3RegisterLikeFunctions(sqlite3 *db, int is_case_sensitive) >> +sqlite3RegisterLikeFunctions(sqlite3 *db, int *is_case_insensitive) >> { >> /* >> * FIXME: after introducing type LIKE must >> * return that type: TRUE if the string matches the >> * supplied pattern and FALSE otherwise. >> */ >> - int *is_like_ci; >> - if (is_case_sensitive) >> - is_like_ci =3D (void *)&case_sensitive_like; >> - else >> - is_like_ci =3D (void *)&case_insensitive_like; >> - sqlite3CreateFunc(db, "LIKE", AFFINITY_INTEGER, 2, 0, = is_like_ci, >> - likeFunc, 0, 0, 0); >> - sqlite3CreateFunc(db, "LIKE", AFFINITY_INTEGER, 3, 0, = is_like_ci, >> - likeFunc, 0, 0, 0); >> + int is_like_ci =3D SQLITE_PTR_TO_INT(is_case_insensitive); >> + sqlite3CreateFunc(db, "LIKE", AFFINITY_INTEGER, 2, 0, >> + is_case_insensitive, likeFunc, 0, 0, 0); >> + sqlite3CreateFunc(db, "LIKE", AFFINITY_INTEGER, 3, 0, >> + is_case_insensitive, likeFunc, 0, 0, 0); >> setLikeOptFlag(db, "LIKE", >> - is_case_sensitive ? (SQLITE_FUNC_LIKE | >> + !(is_like_ci) ? (SQLITE_FUNC_LIKE | >> SQLITE_FUNC_CASE) : SQLITE_FUNC_LIKE); >> } >=20 > I think sqlite3RegisterLikeFunctions should accept int as the second > parameter. Sure, fixed it. > BTW, it seems that we add SQLITE_FUNC_CASE into flags, so maybe we can > read it in likeFunc and don't pass additional parameter at all? It is > just raw idea, I don't know the mechanics of sql functions much. I=E2=80=99m not sure it=E2=80=99s a good idea, at least not in terms of = this patch. Call to likeFunc comes right from OP_FUNCTION which is used for sql functions (of course it=E2=80=99s not only LIKE). Thus the list of = parameters is pretty much defined there. (*pCtx->pFunc->xSFunc)(pCtx, pCtx->argc, pCtx->argv);/* IMP: = R-24505-23230 */ >=20 >> diff --git a/test/sql-tap/collation.test.lua = b/test/sql-tap/collation.test.lua >> index eb4f43a90..dbbe1c0fe 100755 >> --- a/test/sql-tap/collation.test.lua >> +++ b/test/sql-tap/collation.test.lua >> @@ -219,7 +219,7 @@ local like_testcases =3D >> {0, {"Aab", "aaa"}} }, >> {"2.1.2", >> "EXPLAIN QUERY PLAN SELECT * FROM tx1 WHERE s1 LIKE 'A%';", >> - {0, {0, 0, 0, "/USING COVERING INDEX I1/"}} }, >> + {0, {0, 0, 0, "SEARCH TABLE TX1 USING COVERING INDEX I1 = (S1>? AND S1=20 > What is this hunk about? I found that this changed message occurs even on fresh 2.1. I was using this and few other test cases to test my patch so it was kind of concerned with the patch and i decided to fix it at the same = time. (I don=E2=80=99t really think it does worth opening an issue). I assume this issue was caused by commit 6b8acd8fde and just wasn=E2=80=99t fixed. I guess I should=E2=80=99ve wrote about it in the previous letter. diff with the previous version: diff --git a/src/box/sql/func.c b/src/box/sql/func.c index 5feab52f6..ed7926b18 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -1684,20 +1684,20 @@ setLikeOptFlag(sqlite3 * db, const char *zName, = u8 flagVal) * @retval none. */ void -sqlite3RegisterLikeFunctions(sqlite3 *db, int *is_case_insensitive) +sqlite3RegisterLikeFunctions(sqlite3 *db, int is_case_insensitive) { /* * FIXME: after introducing type LIKE must * return that type: TRUE if the string matches the * supplied pattern and FALSE otherwise. */ - int is_like_ci =3D SQLITE_PTR_TO_INT(is_case_insensitive); + int *is_like_ci =3D SQLITE_INT_TO_PTR(is_case_insensitive); sqlite3CreateFunc(db, "LIKE", AFFINITY_INTEGER, 2, 0, - is_case_insensitive, likeFunc, 0, 0, 0); + is_like_ci, likeFunc, 0, 0, 0); sqlite3CreateFunc(db, "LIKE", AFFINITY_INTEGER, 3, 0, - is_case_insensitive, likeFunc, 0, 0, 0); + is_like_ci, likeFunc, 0, 0, 0); setLikeOptFlag(db, "LIKE", - !(is_like_ci) ? (SQLITE_FUNC_LIKE | + !(is_case_insensitive) ? (SQLITE_FUNC_LIKE | SQLITE_FUNC_CASE) : SQLITE_FUNC_LIKE); } =20 diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c index 546b18ae2..8939aa702 100644 --- a/src/box/sql/pragma.c +++ b/src/box/sql/pragma.c @@ -586,10 +586,9 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First = part of [schema.]id field */ */ case PragTyp_CASE_SENSITIVE_LIKE:{ if (zRight) { - int *is_like_ci =3D - = SQLITE_INT_TO_PTR(!(sqlite3GetBoolean(zRight, 0))); - sqlite3RegisterLikeFunctions(db, - = is_like_ci); + int is_like_ci =3D + !(sqlite3GetBoolean(zRight, 0)); + sqlite3RegisterLikeFunctions(db, = is_like_ci); } break; } diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index dd819622b..07e83e444 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -4551,7 +4551,7 @@ sql_key_info_unref(struct sql_key_info *key_info); struct key_def * sql_key_info_to_key_def(struct sql_key_info *key_info); =20 -void sqlite3RegisterLikeFunctions(sqlite3 *, int *); +void sqlite3RegisterLikeFunctions(sqlite3 *, int); int sql_is_like_func(sqlite3 *, Expr *, int *); int sqlite3CreateFunc(sqlite3 *, const char *, enum affinity_type, int, int, void *, --Apple-Mail=_2251700A-BC41-45A6-86E6-198023A4FC8E Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Hello!

Thank you for comments.

On Nov = 9, 2018, at 15:18, Alexander Turenko <alexander.turenko@tarantool.org> wrote:

Hi!
See comments below.

WBR, Alexander Turenko.

On Thu, = Nov 08, 2018 at 06:09:23PM +0300, Nikita Tatunov wrote:
Hello! thank you for the = review.

Issues:
https://github.com/tarantool/tarantool/issues/3251 <https://github.com/tarantool/tarantool/issues/3251>
https://github.com/tarantool/tarantool/issues/3334 <https://github.com/tarantool/tarantool/issues/3334>

Branch:
https://github.com/tarantool/tarantool/tree/N_Tatunov/gh-3251-w= here-like-hangs <https://github.com/tarantool/tarantool/tree/N_Tatunov/gh-3251-w= here-like-hangs>


It seems you forgot to push the new changes.

Ermm, = did I? I have just opened the branch and it contains a = commit
which was pushed at 8th of November (and as I see = it=E2=80=99s exactly the commit
from the diff)


@@ -913,7 = +899,7 @@ likeFunc(sqlite3_context *context, int argc, sqlite3_value = **argv)
sqlite3_like_count++;
#endif
int res;
- res =3D = sql_utf8_pattern_compare(zB, zA, *is_like_ci, escape);
+ = res =3D sql_utf8_pattern_compare(zB, zA, is_like_ci, escape);
= if (res =3D=3D SQL_INVALID_PATTERN) {
const = char *const err_msg =3D
"LIKE pattern can only contain = UTF-8 characters";
@@ -1698,24 +1684,20 @@ = setLikeOptFlag(sqlite3 * db, const char *zName, u8 flagVal)
=  * @retval none.
 */
void
-sqlite3RegisterLikeFunctions(sqlite3 *db, int = is_case_sensitive)
+sqlite3RegisterLikeFunctions(sqlite3 = *db, int *is_case_insensitive)
{
/*
= * FIXME: after introducing type <BOOLEAN> LIKE must
= * return that type: TRUE if the string matches the
= = * supplied pattern and FALSE otherwise.
*/
- = int *is_like_ci;
- if (is_case_sensitive)
- = = is_like_ci =3D (void *)&case_sensitive_like;
- = else
- is_like_ci =3D (void = *)&case_insensitive_like;
- = sqlite3CreateFunc(db, "LIKE", AFFINITY_INTEGER, 2, 0, = is_like_ci,
-  likeFunc, 0, 0, 0);
- = sqlite3CreateFunc(db, "LIKE", AFFINITY_INTEGER, 3, 0, = is_like_ci,
-  likeFunc, 0, 0, 0);
+ = int is_like_ci =3D SQLITE_PTR_TO_INT(is_case_insensitive);
+ = sqlite3CreateFunc(db, "LIKE", AFFINITY_INTEGER, 2, 0,
+ = = =  is_case_insensitive, likeFunc, 0, 0, 0);
+ = sqlite3CreateFunc(db, "LIKE", AFFINITY_INTEGER, 3, 0,
+ = = =  is_case_insensitive, likeFunc, 0, 0, 0);
= = setLikeOptFlag(db, "LIKE",
- =       is_case_sensitive ? = (SQLITE_FUNC_LIKE |
+ =       !(is_like_ci) ? (SQLITE_FUNC_LIKE = |
= =       SQLITE_FUNC_CASE) : = SQLITE_FUNC_LIKE);
}

I think sqlite3RegisterLikeFunctions should accept int as the = second
parameter.

Sure, = fixed it.

BTW, it seems that we add SQLITE_FUNC_CASE = into flags, so maybe we can
read it in likeFunc and don't = pass additional parameter at all? It is
just raw idea, I = don't know the mechanics of sql functions much.

I=E2=80= =99m not sure it=E2=80=99s a good idea, at least not in terms of this = patch.
Call to likeFunc comes right from OP_FUNCTION which is = used for sql
functions (of course it=E2=80=99s not only LIKE). = Thus the list of parameters is
pretty much defined = there.

= (*pCtx->pFunc->xSFunc)(pCtx, pCtx->argc, = pCtx->argv);/* IMP: R-24505-23230 */


diff --git = a/test/sql-tap/collation.test.lua b/test/sql-tap/collation.test.lua
index eb4f43a90..dbbe1c0fe 100755
--- = a/test/sql-tap/collation.test.lua
+++ = b/test/sql-tap/collation.test.lua
@@ -219,7 +219,7 @@ = local like_testcases =3D
=         {0, {"Aab", "aaa"}} = },
    {"2.1.2",
=         "EXPLAIN QUERY PLAN = SELECT * FROM tx1 WHERE s1 LIKE 'A%';",
- =        {0, {0, 0, 0, "/USING COVERING = INDEX I1/"}} },
+ =        {0, {0, 0, 0, "SEARCH TABLE = TX1 USING COVERING INDEX I1 (S1>? AND S1<?)"}}},

What is this hunk about?

I found = that this changed message occurs even on fresh 2.1.
I was = using this and few other test cases to test my patch so it = was
kind of concerned with the patch and i decided to fix it = at the same time.
(I don=E2=80=99t really think it does worth = opening an issue).
I assume this issue was caused by commit 6b8acd8fde and just
wasn=E2=80=99t fixed.
I guess = I should=E2=80=99ve wrote about it in the previous letter.


diff with the previous = version:

diff --git = a/src/box/sql/func.c b/src/box/sql/func.c
index = 5feab52f6..ed7926b18 100644
--- = a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ = -1684,20 +1684,20 @@ setLikeOptFlag(sqlite3 * db, const char *zName, u8 = flagVal)
  * @retval none.
  = */
 void
-sqlite3RegisterLikeFunctions(sqlite3 = *db, int = *is_case_insensitive)
+sqlite3RegisterLikeFunctions(sqlite3 = *db, int is_case_insensitive)
 {
  = /*
  * FIXME: after introducing type = <BOOLEAN> LIKE must
  * return that type: TRUE if the = string matches the
  * supplied pattern and FALSE = otherwise.
  */
- int = is_like_ci =3D SQLITE_PTR_TO_INT(is_case_insensitive);
+ int = *is_like_ci =3D = SQLITE_INT_TO_PTR(is_case_insensitive);
  = sqlite3CreateFunc(db, "LIKE", AFFINITY_INTEGER, 2, = 0,
- =  is_case_insensitive, likeFunc, 0, 0, = 0);
+ =  is_like_ci, likeFunc, 0, 0, = 0);
  sqlite3CreateFunc(db, "LIKE", = AFFINITY_INTEGER, 3, 0,
- =  is_case_insensitive, likeFunc, 0, 0, 0);
+ =  is_like_ci, likeFunc, 0, 0, 0);
  = setLikeOptFlag(db, "LIKE",
- =       !(is_like_ci) ? (SQLITE_FUNC_LIKE = |
+ =       !(is_case_insensitive) ? (SQLITE_FUNC_LIKE = |
        = SQLITE_FUNC_CASE) : = SQLITE_FUNC_LIKE);
 }
 
diff = --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index = 546b18ae2..8939aa702 100644
--- = a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ = -586,10 +586,9 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First = part of [schema.]id field */
  = */
  case = PragTyp_CASE_SENSITIVE_LIKE:{
  = if (zRight) {
- int = *is_like_ci =3D
- = SQLITE_INT_TO_PTR(!(sqlite3GetBoolean(zRight, = 0)));
-= = sqlite3RegisterLikeFunctions(db,
- =     = is_like_ci);
+ int = is_like_ci =3D
+ = !(sqlite3GetBoolean(zRight, 0));
+ = sqlite3RegisterLikeFunctions(db, = is_like_ci);
  = }
  = break;
  }
diff --git = a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index = dd819622b..07e83e444 100644
--- = a/src/box/sql/sqliteInt.h
+++ = b/src/box/sql/sqliteInt.h
@@ -4551,7 +4551,7 @@ = sql_key_info_unref(struct sql_key_info = *key_info);
 struct key_def = *
 sql_key_info_to_key_def(struct sql_key_info = *key_info);
 
-void = sqlite3RegisterLikeFunctions(sqlite3 *, int *);
+void = sqlite3RegisterLikeFunctions(sqlite3 *, int);
 int = sql_is_like_func(sqlite3 *, Expr *, int *);
 int = sqlite3CreateFunc(sqlite3 *, const char *, enum = affinity_type,
       int, = int, void *,


= --Apple-Mail=_2251700A-BC41-45A6-86E6-198023A4FC8E--