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 <alexander.turenko@tarantool.org> 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 <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);
 }
 
@@ -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<?)"}}},
     {"2.2.0",
         "PRAGMA case_sensitive_like = true;",
         {0}},