[tarantool-patches] Re: [PATCH v1 2/3] sql: ban sql functions coinciding with builtins
Kirill Shcherbatov
kshcherbatov at tarantool.org
Fri May 17 11:22:50 MSK 2019
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
More information about the Tarantool-patches
mailing list