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 DE2D22F44B for ; Fri, 17 May 2019 04:22:52 -0400 (EDT) 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 Matri4mywU_L for ; Fri, 17 May 2019 04:22:52 -0400 (EDT) Received: from smtp51.i.mail.ru (smtp51.i.mail.ru [94.100.177.111]) (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 788D02F447 for ; Fri, 17 May 2019 04:22:52 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 2/3] sql: ban sql functions coinciding with builtins References: <068d058c4b8354f332e67e95761f722ab72cbae0.1557941573.git.kshcherbatov@tarantool.org> <0356C5B8-C1B1-4986-BBC7-5202E877F656@tarantool.org> From: Kirill Shcherbatov Message-ID: Date: Fri, 17 May 2019 11:22:50 +0300 MIME-Version: 1.0 In-Reply-To: <0356C5B8-C1B1-4986-BBC7-5202E877F656@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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, "n.pettik" Previously, it was possible to create a function with name that is already reserved for some predefined builtin. It is a bug. This patch bans sql_create_function for such cases. Also removed SQL_PreferBuiltin user session flag. Closes #4219 Needed for #3691 --- src/box/sql/callback.c | 12 ++---------- src/box/sql/expr.c | 8 ++++---- src/box/sql/func.c | 33 +++++++++++---------------------- src/box/sql/main.c | 12 +++++++++--- src/box/sql/resolve.c | 5 +++-- src/box/sql/sqlInt.h | 5 +++-- src/box/sql/vdbemem.c | 2 +- test/sql-tap/lua_sql.test.lua | 11 ++++++++++- 8 files changed, 43 insertions(+), 45 deletions(-) diff --git a/src/box/sql/callback.c b/src/box/sql/callback.c index 54d59bbd3..b5163c208 100644 --- a/src/box/sql/callback.c +++ b/src/box/sql/callback.c @@ -37,7 +37,6 @@ #include "box/coll_id_cache.h" #include "sqlInt.h" -#include "box/session.h" struct coll * sql_get_coll_seq(Parse *parser, const char *name, uint32_t *coll_id) @@ -173,7 +172,7 @@ sqlInsertBuiltinFuncs(FuncDef * aDef, /* List of global functions to be inserted struct FuncDef * sql_find_function(struct sql *db, const char *func_name, int arg_count, - bool is_create) + bool is_builtin, bool is_create) { assert(arg_count >= -2); assert(arg_count >= -1 || !is_create); @@ -185,9 +184,6 @@ sql_find_function(struct sql *db, const char *func_name, int arg_count, int func_score = 0; /* Best match found so far. */ struct FuncDef *func = NULL; - - struct session *user_session = current_session(); - bool is_builtin = (user_session->sql_flags & SQL_PreferBuiltin) != 0; if (is_builtin) goto lookup_for_builtin; @@ -207,10 +203,6 @@ sql_find_function(struct sql *db, const char *func_name, int arg_count, /** * If no match is found, search the built-in functions. * - * If the SQL_PreferBuiltin flag is set, then search the - * built-in functions even if a prior app-defined function - * was found. And give priority to built-in functions. - * * Except, if is_create is true, that means that we are * trying to install a new function. Whatever FuncDef * structure is returned it will have fields overwritten @@ -219,7 +211,7 @@ sql_find_function(struct sql *db, const char *func_name, int arg_count, * So we must not search for built-ins when creating a * new function. */ - if (!is_create && (func == NULL || is_builtin)) { + if (!is_create && func == NULL) { lookup_for_builtin: func_score = 0; int h = sql_builtin_func_name_hash(func_name, func_name_len); diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index 12b54ff65..7133ec3ac 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -287,7 +287,7 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id, p->x.pList->nExpr; struct FuncDef *func = sql_find_function(parse->db, p->u.zToken, - arg_count, false); + arg_count, false, false); if (func == NULL) break; if (func->is_coll_derived) { @@ -3991,11 +3991,11 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) nFarg = pFarg ? pFarg->nExpr : 0; assert(!ExprHasProperty(pExpr, EP_IntValue)); zId = pExpr->u.zToken; - pDef = sql_find_function(db, zId, nFarg, false); + pDef = sql_find_function(db, zId, nFarg, false, false); #ifdef SQL_ENABLE_UNKNOWN_SQL_FUNCTION if (pDef == 0 && pParse->explain) { pDef = sql_find_function(db, "unknown", nFarg, - false); + false, false); } #endif if (pDef == 0 || pDef->xFinalize != 0) { @@ -5466,7 +5466,7 @@ analyzeAggregate(Walker * pWalker, Expr * pExpr) pExpr->u.zToken, pExpr->x.pList ? pExpr->x.pList->nExpr : 0, - false); + false, false); if (pExpr->flags & EP_Distinct) { pItem->iDistinct = pParse->nTab++; diff --git a/src/box/sql/func.c b/src/box/sql/func.c index c110b205c..afc25aab5 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -1811,7 +1811,7 @@ sql_overload_function(sql * db, const char *zName, { int rc = SQL_OK; - if (sql_find_function(db, zName, nArg, false) == 0) { + if (sql_find_function(db, zName, nArg, false, false) == 0) { rc = sqlCreateFunc(db, zName, type, nArg, 0, 0, sqlInvalidFunction, 0, 0, 0); } @@ -1834,19 +1834,6 @@ sqlRegisterPerConnectionBuiltinFunctions(sql * db) } } -/* - * Set the LIKEOPT flag on the 2-argument function with the given name. - */ -static void -setLikeOptFlag(sql * db, const char *zName, u8 flagVal) -{ - FuncDef *pDef; - pDef = sql_find_function(db, zName, 2, false); - if (ALWAYS(pDef)) { - pDef->funcFlags |= flagVal; - } -} - /** * Register the built-in LIKE function. */ @@ -1859,13 +1846,14 @@ sqlRegisterLikeFunctions(sql *db, int is_case_insensitive) * supplied pattern and FALSE otherwise. */ int *is_like_ci = SQL_INT_TO_PTR(is_case_insensitive); - sqlCreateFunc(db, "LIKE", FIELD_TYPE_BOOLEAN, 2, 0, - is_like_ci, likeFunc, 0, 0, 0); - sqlCreateFunc(db, "LIKE", FIELD_TYPE_BOOLEAN, 3, 0, - is_like_ci, likeFunc, 0, 0, 0); - setLikeOptFlag(db, "LIKE", - !(is_case_insensitive) ? (SQL_FUNC_LIKE | - SQL_FUNC_CASE) : SQL_FUNC_LIKE); + struct FuncDef *like2 = sql_find_function(db, "LIKE", 2, true, false); + like2->funcFlags = is_case_insensitive ? SQL_FUNC_LIKE : + SQL_FUNC_LIKE | SQL_FUNC_CASE; + like2->pUserData = is_like_ci; + assert(like2 != NULL); + struct FuncDef *like3 = sql_find_function(db, "LIKE", 2, true, false); + assert(like3 != NULL); + like3->pUserData = is_like_ci; } int @@ -1875,7 +1863,8 @@ sql_is_like_func(struct sql *db, struct Expr *expr, int *is_like_ci) expr->x.pList->nExpr != 2) return 0; assert(!ExprHasProperty(expr, EP_xIsSelect)); - struct FuncDef *func = sql_find_function(db, expr->u.zToken, 2, false); + struct FuncDef *func = + sql_find_function(db, expr->u.zToken, 2, false, false); assert(func != NULL); if ((func->funcFlags & SQL_FUNC_LIKE) == 0) return 0; diff --git a/src/box/sql/main.c b/src/box/sql/main.c index 0e8f3e559..cd2d00642 100644 --- a/src/box/sql/main.c +++ b/src/box/sql/main.c @@ -428,13 +428,19 @@ sqlCreateFunc(sql * db, assert(SQL_FUNC_CONSTANT == SQL_DETERMINISTIC); extraFlags = flags & SQL_DETERMINISTIC; - + /* Ensure there is no builtin function with this name. */ + p = sql_find_function(db, zFunctionName, -2, true, false); + if (p != NULL) { + diag_set(ClientError, ER_SQL, "cannot create a function with " + "a signature that coincides builtin function"); + return SQL_TARANTOOL_ERROR; + } /* Check if an existing function is being overridden or deleted. If so, * and there are active VMs, then return SQL_BUSY. If a function * is being overridden/deleted but there are no active VMs, allow the * operation to continue but invalidate all precompiled statements. */ - p = sql_find_function(db, zFunctionName, nArg, false); + p = sql_find_function(db, zFunctionName, nArg, false, false); if (p && p->nArg == nArg) { if (db->nVdbeActive) { sqlErrorWithMsg(db, SQL_BUSY, @@ -446,7 +452,7 @@ sqlCreateFunc(sql * db, } } - p = sql_find_function(db, zFunctionName, nArg, true); + p = sql_find_function(db, zFunctionName, nArg, false, true); if (p == NULL) return SQL_TARANTOOL_ERROR; diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c index 7e2eed7bb..7cf79692c 100644 --- a/src/box/sql/resolve.c +++ b/src/box/sql/resolve.c @@ -602,10 +602,11 @@ resolveExprStep(Walker * pWalker, Expr * pExpr) assert(!ExprHasProperty(pExpr, EP_xIsSelect)); zId = pExpr->u.zToken; nId = sqlStrlen30(zId); - pDef = sql_find_function(pParse->db, zId, n, false); + pDef = sql_find_function(pParse->db, zId, n, + false, false); if (pDef == 0) { pDef = sql_find_function(pParse->db, zId, -2, - false); + false, false); if (pDef == 0) { no_such_func = 1; } else { diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index 61406cdc9..8cdad4749 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -1506,7 +1506,6 @@ struct sql { #define SQL_ReverseOrder 0x00020000 /* Reverse unordered SELECTs */ #define SQL_RecTriggers 0x00040000 /* Enable recursive triggers */ #define SQL_AutoIndex 0x00100000 /* Enable automatic indexes */ -#define SQL_PreferBuiltin 0x00200000 /* Preference to built-in funcs */ #define SQL_EnableTrigger 0x01000000 /* True to enable triggers */ #define SQL_DeferFKs 0x02000000 /* Defer all FK constraints */ #define SQL_VdbeEQP 0x08000000 /* Debug EXPLAIN QUERY PLAN */ @@ -3934,6 +3933,8 @@ void sqlInsertBuiltinFuncs(FuncDef *, int); * @param func_name The function name 0-terminated string. * @param arg_count The count of function's arguments. * May be set -1 for any number. + * @param is_builtin If this flag is set true, perform lookup + * for the built-in function. * @param is_create If this flag is set true, then a new (blank) * FuncDef structure is created and liked into * the "db" structure if a no matching function @@ -3946,7 +3947,7 @@ void sqlInsertBuiltinFuncs(FuncDef *, int); */ struct FuncDef * sql_find_function(struct sql *db, const char *func_name, int arg_count, - bool is_create); + bool is_builtin, bool is_create); void sqlRegisterBuiltinFunctions(void); void sqlRegisterDateTimeFunctions(void); diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c index c56eb73e6..537a72f10 100644 --- a/src/box/sql/vdbemem.c +++ b/src/box/sql/vdbemem.c @@ -1245,7 +1245,7 @@ valueFromFunction(sql * db, /* The database connection */ pList = p->x.pList; if (pList) nVal = pList->nExpr; - pFunc = sql_find_function(db, p->u.zToken, nVal, false); + pFunc = sql_find_function(db, p->u.zToken, nVal, false, false); assert(pFunc); if ((pFunc->funcFlags & (SQL_FUNC_CONSTANT | SQL_FUNC_SLOCHNG)) == 0 || (pFunc->funcFlags & SQL_FUNC_NEEDCOLL) diff --git a/test/sql-tap/lua_sql.test.lua b/test/sql-tap/lua_sql.test.lua index b0913e7f4..fd7cd732a 100755 --- a/test/sql-tap/lua_sql.test.lua +++ b/test/sql-tap/lua_sql.test.lua @@ -1,7 +1,7 @@ #!/usr/bin/env tarantool test = require("sqltester") NULL = require('msgpack').NULL -test:plan(24) +test:plan(25) local function func1(a) return a @@ -18,6 +18,15 @@ test:do_test( end, {2}) +test:do_test( + "lua_sql-1.1", + function () + local ret, errmsg = pcall(box.internal.sql_create_function, + "upper", "TEXT", func1) + return {ret, errmsg} + end, + {0, "SQL error: cannot create a function with a signature that coincides builtin function"}) + -- new function should replace prewious one test:do_test( "lua_sql-1.1", -- 2.21.0