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 Please consider following changes: 1. I rebased the whole branch on fresh 2.1. > On Oct 29, 2018, at 15:15, Alexander Turenko wrote: > > Hi! > > See remaining comments below. > > WBR, Alexander Turenko. > >>>> +static const int case_insensitive_like = 1; >>>> +static const int case_sensitive_like = 0; >>> >>> Are not they are mutually exclusive? Can we leave just one? >>> >>> Why they are not defines? >> >> I guess it’s a bad idea since we need pointers to them in some functions. >> e.g: >> >> 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", 2, 0, is_like_ci, likeFunc, 0, 0, 0); >> sqlite3CreateFunc(db, "LIKE", 3, 0, is_like_ci, likeFunc, 0, 0, 0); > > I think we can use SQLITE_INT_TO_PTR and SQLITE_PTR_TO_INT to pass an > integer value as a pointer and then cast it back. Maybe we also should > change LIKEFUNC macro. > >>>> + int *is_like_ci; >>>> + if (is_case_sensitive) >>>> + is_like_ci = (int *)&case_sensitive_like; >>>> + else >>>> + is_like_ci = (int *)&case_insensitive_like; >>> >>> Discarding const qualifier? It seems not being right thing to do. It is >>> better to define is_like_ci pointer as const. >> >> It’s not possible due to so-called “down qualification” and will cause >> compiler warnings. > > We'll anyway discard it here or there, so nevermind. I proposed to > change it to SQLITE_INT_TO_PTR. > 2. Changed manipulations with “sensitiveness” of LIKE using your advice thus there’s no static consts in func.c anymore. diff with the previous version of the patch: diff --git a/src/box/sql/func.c b/src/box/sql/func.c index b1983e594..5feab52f6 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -627,18 +627,6 @@ total_changes(sqlite3_context * context, int NotUsed, sqlite3_value ** NotUsed2) #define SQL_END_OF_STRING 0xffff #define SQL_INVALID_UTF8_SYMBOL 0xfffd -/** - * If SQLITE_CASE_SENSITIVE_LIKE is not defined, then the LIKE - * operator is not case sensitive. - */ -static const int case_insensitive_like = 1; - -/** - * If SQLITE_CASE_SENSITIVE_LIKE is defined, then the LIKE - * operator is case sensitive causing 'a' LIKE 'A' to be false. - */ -static const int case_sensitive_like = 0; - /** * Possible error returns from sql_utf8_pattern_compare(). */ @@ -819,8 +807,7 @@ sql_utf8_pattern_compare(const char *pattern, int sql_strlike_cs(const char *zPattern, const char *zStr, unsigned int esc) { - return sql_utf8_pattern_compare(zPattern, zStr, case_sensitive_like, - esc); + return sql_utf8_pattern_compare(zPattern, zStr, 0, esc); } /** @@ -830,8 +817,7 @@ sql_strlike_cs(const char *zPattern, const char *zStr, unsigned int esc) int sql_strlike_ci(const char *zPattern, const char *zStr, unsigned int esc) { - return sql_utf8_pattern_compare(zPattern, zStr, case_insensitive_like, - esc); + return sql_utf8_pattern_compare(zPattern, zStr, 1, esc); } /** @@ -859,7 +845,7 @@ likeFunc(sqlite3_context *context, int argc, sqlite3_value **argv) u32 escape = SQL_END_OF_STRING; int nPat; sqlite3 *db = sqlite3_context_db_handle(context); - int *is_like_ci = sqlite3_user_data(context); + int is_like_ci = SQLITE_PTR_TO_INT(sqlite3_user_data(context)); #ifdef SQLITE_LIKE_DOESNT_MATCH_BLOBS if (sqlite3_value_type(argv[0]) == SQLITE_BLOB @@ -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); } @@ -1841,14 +1823,14 @@ sqlite3RegisterBuiltinFunctions(void) groupConcatFinalize, AFFINITY_TEXT), #ifdef SQLITE_CASE_SENSITIVE_LIKE - LIKEFUNC(like, 2, &case_sensitive_like, - SQLITE_FUNC_LIKE | SQLITE_FUNC_CASE, AFFINITY_INTEGER), - LIKEFUNC(like, 3, &case_sensitive_like, - SQLITE_FUNC_LIKE | SQLITE_FUNC_CASE, AFFINITY_INTEGER), + LIKEFUNC(like, 2, 0,SQLITE_FUNC_LIKE | SQLITE_FUNC_CASE, + AFFINITY_INTEGER), + LIKEFUNC(like, 3, 0,SQLITE_FUNC_LIKE | SQLITE_FUNC_CASE, + AFFINITY_INTEGER), #else - LIKEFUNC(like, 2, &case_insensitive_like, SQLITE_FUNC_LIKE, + LIKEFUNC(like, 2, 1, SQLITE_FUNC_LIKE, AFFINITY_INTEGER), - LIKEFUNC(like, 3, &case_insensitive_like, SQLITE_FUNC_LIKE, + LIKEFUNC(like, 3, 1, SQLITE_FUNC_LIKE, AFFINITY_INTEGER), #endif #ifdef SQLITE_ENABLE_UNKNOWN_SQL_FUNCTION diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c index cd6db2b34..546b18ae2 100644 --- a/src/box/sql/pragma.c +++ b/src/box/sql/pragma.c @@ -586,9 +586,10 @@ 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, - sqlite3GetBoolean - (zRight, 0)); + is_like_ci); } break; } diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index f92d4e6b8..dd819622b 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -1759,7 +1759,7 @@ struct FuncDestructor { pArg, 0, xFunc, 0, #zName, {SQLITE_AFF_STRING, {0}}} #define LIKEFUNC(zName, nArg, arg, flags, type) \ {nArg, SQLITE_FUNC_CONSTANT|flags, \ - (void *)arg, 0, likeFunc, 0, #zName, {0}, type } + (void *)(SQLITE_INT_TO_PTR(arg)), 0, likeFunc, 0, #zName, {0}, type } #define AGGREGATE(zName, nArg, arg, nc, xStep, xFinal, type) \ {nArg, (nc*SQLITE_FUNC_NEEDCOLL), \ SQLITE_INT_TO_PTR(arg), 0, xStep,xFinal,#zName, {0}, type} @@ -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 *, 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