Hello! Thank you for comments. > On Nov 9, 2018, at 15:18, Alexander Turenko 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/3334 >> >> Branch: >> 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 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 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 *,