[tarantool-patches] Re: [PATCH 2/2] sql: remove GLOB from Tarantool
Nikita Tatunov
n.tatunov at tarantool.org
Sat Nov 10 06:38:23 MSK 2018
Hello!
Thank you for comments.
> On Nov 9, 2018, at 15:18, Alexander Turenko <alexander.turenko at 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 *,
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20181110/e069ae5a/attachment.html>
More information about the Tarantool-patches
mailing list