[tarantool-patches] Re: [PATCH 2/2] sql: remove GLOB from Tarantool
n.pettik
korablev at tarantool.org
Wed Nov 14 17:16:52 MSK 2018
> On 13 Nov 2018, at 22:23, Alexander Turenko <alexander.turenko at tarantool.org> wrote:
>
> Hi!
>
> I gave some minor comments and I think I cannot add any value anymore.
> Nikita N., can you look into this patch?
> Part of #3589
> Part of #3572
> Needed for #3251
> Needed for #3334
Why this patch is ’needed for’ but comes after
these issues are closed by previous patch?
Also I don’t like the fact that your patch not only removes
GLOB function, but also changes some user-visible interface.
It still would be OK if you wrote about it in commit message
or in doc request. For instance, as I understand from code,
percent symbol (‘%’) now works as star (‘*’) did before.
Personally I don’t like many code style fixes because
they obfuscate patch and make it complicate to review.
What is more, they don’t deal with all problems: they
partially fix comments but don’t fix naming. It looks awful.
But since it is done and two reviewers already gave their
OK I guess it would be irrational to start from scratch.
Finally, to be honest I didn’t review ALL changed tests.
I looked through and they seem to be OK.
I’ve pushed also several non-functional fixes -
see branch np/gh-3251-where-like-hangs.
Let's assume this as formal LGTM.
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index f0baf3db0..c3d3ccfb2 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1674,12 +1674,6 @@ setLikeOptFlag(sqlite3 * db, const char *zName, u8 flagVal)
/**
* Register the built-in LIKE function.
- *
- * @param db database structure.
- * @param is_case_sensitive whether like should be case sensitive
- * or not.
- *
- * @retval none.
*/
void
sqlite3RegisterLikeFunctions(sqlite3 *db, int is_case_insensitive)
@@ -1699,30 +1693,18 @@ sqlite3RegisterLikeFunctions(sqlite3 *db, int is_case_insensitive)
SQLITE_FUNC_CASE) : SQLITE_FUNC_LIKE);
}
-/**
- * Check if the function implements LIKE-style comparison & if it
- * is appropriate to apply a LIKE query optimization.
- *
- * @param db database structure.
- * @param pExpr pointer to a function-implementing expression.
- * @param is_like_ci true if LIKE is case insensitive.
- *
- * @retval 0 if it's appropriate to apply optimization.
- * 1 if it's not.
- */
int
-sql_is_like_func(sqlite3 *db, Expr *pExpr, int *is_like_ci)
+sql_is_like_func(struct sqlite3 *db, struct Expr *expr, bool *is_like_ci)
{
- FuncDef *pDef;
- if (pExpr->op != TK_FUNCTION || !pExpr->x.pList ||
- pExpr->x.pList->nExpr != 2)
+ if (expr->op != TK_FUNCTION || !expr->x.pList ||
+ expr->x.pList->nExpr != 2)
return 0;
- assert(!ExprHasProperty(pExpr, EP_xIsSelect));
- pDef = sqlite3FindFunction(db, pExpr->u.zToken, 2, 0);
- if (NEVER(pDef == 0) || (pDef->funcFlags & SQLITE_FUNC_LIKE) == 0) {
+ assert(!ExprHasProperty(expr, EP_xIsSelect));
+ struct FuncDef *func = sqlite3FindFunction(db, expr->u.zToken, 2, 0);
+ assert(func != NULL);
+ if ((func->funcFlags & SQLITE_FUNC_LIKE) == 0)
return 0;
- }
- *is_like_ci = (pDef->funcFlags & SQLITE_FUNC_CASE) == 0;
+ *is_like_ci = (func->funcFlags & SQLITE_FUNC_CASE) == 0;
return 1;
}
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 07e83e444..af124b731 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -4552,7 +4552,20 @@ struct key_def *
sql_key_info_to_key_def(struct sql_key_info *key_info);
void sqlite3RegisterLikeFunctions(sqlite3 *, int);
-int sql_is_like_func(sqlite3 *, Expr *, int *);
+
+/**
+ * Check if the function implements LIKE-style comparison & if it
+ * is appropriate to apply a LIKE query optimization.
+ *
+ * @param db database structure.
+ * @param pExpr pointer to a function-implementing expression.
+ * @param[out] is_like_ci true if LIKE is case insensitive.
+ *
+ * @retval 1 if LIKE optimization can be used, 0 otherwise.
+ */
+int
+sql_is_like_func(struct sqlite3 *db, struct Expr *expr, bool *is_like_ci);
+
int sqlite3CreateFunc(sqlite3 *, const char *, enum affinity_type,
int, int, void *,
void (*)(sqlite3_context *, int, sqlite3_value **),
diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c
index 5f6344405..5d6b04418 100644
--- a/src/box/sql/whereexpr.c
+++ b/src/box/sql/whereexpr.c
@@ -232,21 +232,17 @@ operatorMask(int op)
* @param pParse Parsing and code generating context.
* @param pExpr Test this expression.
* @param ppPrefix Pointer to TK_STRING expression with
- * pattern prefix.
+ * pattern prefix.
* @param pisComplete True if the only wildcard is '%' in the
- * last character.
+ * last character.
* @param pnoCase True if case insensitive.
*
* @retval True if the given expr is a LIKE operator & is
- * optimizable using inequality constraints.
- * False if not.
+ * optimizable using inequality constraints.
*/
static int
-is_like(Parse *pParse,
- Expr *pExpr,
- Expr **ppPrefix,
- int *pisComplete,
- int *pnoCase)
+like_optimization_is_valid(Parse *pParse, Expr *pExpr, Expr **ppPrefix,
+ int *pisComplete, int *pnoCase)
{
/* String on RHS of LIKE operator. */
const char *z = 0;
@@ -1158,17 +1154,16 @@ exprAnalyze(SrcList * pSrc, /* the FROM clause */
* is made all lowercase so that the bounds also work
* when comparing BLOBs.
*/
- if (pWC->op == TK_AND
- && is_like(pParse, pExpr, &pStr1, &isComplete, &noCase)) {
- /* LHS of LIKE operator */
+ if (pWC->op == TK_AND &&
+ like_optimization_is_valid(pParse, pExpr, &pStr1,
+ &isComplete, &noCase)) {
Expr *pLeft;
- /* Copy of pStr1 - RHS of LIKE operator */
+ /* Copy of pStr1 - RHS of LIKE operator. */
Expr *pStr2;
Expr *pNewExpr1;
Expr *pNewExpr2;
int idxNew1;
int idxNew2;
- /* Name of collating sequence */
const char *zCollSeqName;
const u16 wtFlags = TERM_LIKEOPT | TERM_VIRTUAL | TERM_DYNAMIC;
@@ -1179,7 +1174,7 @@ exprAnalyze(SrcList * pSrc, /* the FROM clause */
* Convert the lower bound to upper-case and the
* upper bound to lower-case (upper-case is less
* than lower-case in ASCII) so that the range
- * constraints also work for BLOBs
+ * constraints also work for BLOBs.
*/
if (noCase && !pParse->db->mallocFailed) {
int i;
diff --git a/test/sql-tap/e_expr.test.lua b/test/sql-tap/e_expr.test.lua
index f49810999..db44999a6 100755
--- a/test/sql-tap/e_expr.test.lua
+++ b/test/sql-tap/e_expr.test.lua
@@ -1264,7 +1264,6 @@ test:do_execsql_test(
test:execsql [[
CREATE TABLE tblname(cname INT PRIMARY KEY);
]]
-
local function glob(args)
return 1
end
More information about the Tarantool-patches
mailing list