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-where-like-hangs <https://github.com/tarantool/tarantool/tree/N_Tatunov/gh-3251-where-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’s 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 = sql_utf8_pattern_compare(zB, zA, *is_like_ci, escape);
+ res = sql_utf8_pattern_compare(zB, zA, is_like_ci, escape);
if (res == SQL_INVALID_PATTERN) {
const char *const err_msg =
"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 = (void *)&case_sensitive_like;
- else
- is_like_ci = (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 = 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’m not sure it’s 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’s 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 =
        {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’t really think it does worth opening an issue).

I assume this issue was caused by commit 6b8acd8fde and just

wasn’t fixed.

I guess I should’ve 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 = SQLITE_PTR_TO_INT(is_case_insensitive);

+ int *is_like_ci = 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 =

- SQLITE_INT_TO_PTR(!(sqlite3GetBoolean(zRight, 0)));

- sqlite3RegisterLikeFunctions(db,

- is_like_ci);

+ int is_like_ci =

+ !(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 *,