[tarantool-patches] Re: [PATCH 2/2] sql: remove GLOB from Tarantool
Nikita Tatunov
n.tatunov at tarantool.org
Thu Nov 8 18:09:23 MSK 2018
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>
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 at 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}},
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20181108/81d8d7ee/attachment.html>
More information about the Tarantool-patches
mailing list