From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id AD21C2EFF4 for ; Wed, 14 Nov 2018 09:16:59 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 71hA75K3RQbB for ; Wed, 14 Nov 2018 09:16:59 -0500 (EST) Received: from smtp49.i.mail.ru (smtp49.i.mail.ru [94.100.177.109]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id E0FF62EFDB for ; Wed, 14 Nov 2018 09:16:58 -0500 (EST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.0 \(3445.100.39\)) Subject: [tarantool-patches] Re: [PATCH 2/2] sql: remove GLOB from Tarantool From: "n.pettik" In-Reply-To: <20181113192305.v3uxbrp6ebqrb4gs@tkn_work_nb> Date: Wed, 14 Nov 2018 17:16:52 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <086379A5-15D0-4D16-951E-5F5DBE852068@tarantool.org> References: <32D1E5EA-EA21-4E4B-B5F5-80B6578BFBED@tarantool.org> <3f8354cb-4a47-c3de-164b-c776c792b991@tarantool.org> <090683CD-6F88-492D-AF48-631220A24E36@tarantool.org> <20181021034805.alwhmkpz3fbopqjz@tkn_work_nb> <20181029121529.2d6ccnh5qayiz7ld@tkn_work_nb> <6F5AC823-3016-4918-A588-6BB6B621B777@tarantool.org> <20181109121828.mjdoouycwvonimms@tkn_work_nb> <34EFA3FE-95C1-49FC-9668-11DDD93EC42D@tarantool.org> <20181113192305.v3uxbrp6ebqrb4gs@tkn_work_nb> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Nikita Tatunov , Alexander Turenko > On 13 Nov 2018, at 22:23, Alexander Turenko = wrote: >=20 > Hi! >=20 > 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 =E2=80=99needed for=E2=80=99 but comes after these issues are closed by previous patch? Also I don=E2=80=99t 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 (=E2=80=98%=E2=80=99) now works as star (=E2=80=98*=E2=80=99= ) did before. Personally I don=E2=80=99t like many code style fixes because they obfuscate patch and make it complicate to review. What is more, they don=E2=80=99t deal with all problems: they partially fix comments but don=E2=80=99t 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=E2=80=99t review ALL changed tests. I looked through and they seem to be OK. I=E2=80=99ve 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) =20 /** * 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); } =20 -/** - * 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 !=3D TK_FUNCTION || !pExpr->x.pList || - pExpr->x.pList->nExpr !=3D 2) + if (expr->op !=3D TK_FUNCTION || !expr->x.pList || + expr->x.pList->nExpr !=3D 2) return 0; - assert(!ExprHasProperty(pExpr, EP_xIsSelect)); - pDef =3D sqlite3FindFunction(db, pExpr->u.zToken, 2, 0); - if (NEVER(pDef =3D=3D 0) || (pDef->funcFlags & SQLITE_FUNC_LIKE) = =3D=3D 0) { + assert(!ExprHasProperty(expr, EP_xIsSelect)); + struct FuncDef *func =3D sqlite3FindFunction(db, expr->u.zToken, = 2, 0); + assert(func !=3D NULL); + if ((func->funcFlags & SQLITE_FUNC_LIKE) =3D=3D 0) return 0; - } - *is_like_ci =3D (pDef->funcFlags & SQLITE_FUNC_CASE) =3D=3D 0; + *is_like_ci =3D (func->funcFlags & SQLITE_FUNC_CASE) =3D=3D 0; return 1; } =20 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); =20 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 =3D 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 =3D=3D TK_AND - && is_like(pParse, pExpr, &pStr1, &isComplete, &noCase)) { - /* LHS of LIKE operator */ + if (pWC->op =3D=3D 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 =3D TERM_LIKEOPT | TERM_VIRTUAL | = TERM_DYNAMIC; =20 @@ -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=