[tarantool-patches] [PATCH v1 2/3] sql: ban sql functions coinciding with builtins
Kirill Shcherbatov
kshcherbatov at tarantool.org
Wed May 15 20:34:29 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 FUNC_PERFECT_MATCH user session flag.
Closes #4219
Needed for #3691
---
src/box/lua/lua_sql.c | 8 +-
src/box/sql/callback.c | 174 +++++++++++++++-------------------
src/box/sql/expr.c | 18 ++--
src/box/sql/func.c | 33 +++----
src/box/sql/main.c | 33 ++++---
src/box/sql/resolve.c | 7 +-
src/box/sql/sqlInt.h | 25 ++++-
src/box/sql/vdbemem.c | 2 +-
test/sql-tap/lua_sql.test.lua | 11 ++-
9 files changed, 157 insertions(+), 154 deletions(-)
diff --git a/src/box/lua/lua_sql.c b/src/box/lua/lua_sql.c
index 36b75ff08..561353520 100644
--- a/src/box/lua/lua_sql.c
+++ b/src/box/lua/lua_sql.c
@@ -195,7 +195,11 @@ lbox_sql_create_function(struct lua_State *L)
is_deterministic ? SQL_DETERMINISTIC : 0,
func_info, lua_sql_call, NULL, NULL,
lua_sql_destroy);
- if (rc != 0)
- return luaL_error(L, sqlErrStr(rc));
+ if (rc != 0) {
+ const char *err = rc == SQL_TARANTOOL_ERROR ?
+ box_error_message(box_error_last()) :
+ sqlErrStr(rc);
+ return luaL_error(L, err);
+ }
return 0;
}
diff --git a/src/box/sql/callback.c b/src/box/sql/callback.c
index 49197532e..b4247324d 100644
--- a/src/box/sql/callback.c
+++ b/src/box/sql/callback.c
@@ -131,6 +131,19 @@ functionSearch(int h, /* Hash of the name */
return 0;
}
+/**
+ * Cache function is used to organise sqlBuiltinFunctions table.
+ * @param func_name The name of builtin function.
+ * @param func_name_len The length of the @a name.
+ * @retval Hash value is calculated for given name.
+ */
+static int
+sql_builtin_func_name_hash(const char *func_name, uint32_t func_name_len)
+{
+ return (sqlUpperToLower[(u8) func_name[0]] +
+ func_name_len) % SQL_FUNC_HASH_SZ;
+}
+
/*
* Insert a new FuncDef into a FuncDefHash hash table.
*/
@@ -144,9 +157,7 @@ sqlInsertBuiltinFuncs(FuncDef * aDef, /* List of global functions to be inserted
FuncDef *pOther;
const char *zName = aDef[i].zName;
int nName = sqlStrlen30(zName);
- int h =
- (sqlUpperToLower[(u8) zName[0]] +
- nName) % SQL_FUNC_HASH_SZ;
+ int h = sql_builtin_func_name_hash(zName, nName);
pOther = functionSearch(h, zName);
if (pOther) {
assert(pOther != &aDef[i] && pOther->pNext != &aDef[i]);
@@ -160,110 +171,79 @@ sqlInsertBuiltinFuncs(FuncDef * aDef, /* List of global functions to be inserted
}
}
-/*
- * Locate a user function given a name, a number of arguments and a flag
- * indicating whether the function prefers UTF-16 over UTF-8. Return a
- * pointer to the FuncDef structure that defines that function, or return
- * NULL if the function does not exist.
- *
- * If the createFlag argument is true, then a new (blank) FuncDef
- * structure is created and liked into the "db" structure if a
- * no matching function previously existed.
- *
- * If nArg is -2, then the first valid function found is returned. A
- * function is valid if xSFunc is non-zero. The nArg==(-2)
- * case is used to see if zName is a valid function name for some number
- * of arguments. If nArg is -2, then createFlag must be 0.
- *
- * If createFlag is false, then a function with the required name and
- * number of arguments may be returned even if the eTextRep flag does not
- * match that requested.
- */
-FuncDef *
-sqlFindFunction(sql * db, /* An open database */
- const char *zName, /* Name of the function. zero-terminated */
- int nArg, /* Number of arguments. -1 means any number */
- u8 createFlag /* Create new entry if true and does not otherwise exist */
- )
+struct FuncDef *
+sql_find_function(struct sql *db, const char *func_name, int arg_count,
+ bool is_builtin, bool is_create)
{
- FuncDef *p; /* Iterator variable */
- FuncDef *pBest = 0; /* Best match found so far */
- int bestScore = 0; /* Score of best match */
- int h; /* Hash value */
- int nName; /* Length of the name */
- struct session *user_session = current_session();
-
- assert(nArg >= (-2));
- assert(nArg >= (-1) || createFlag == 0);
- nName = sqlStrlen30(zName);
-
- /* First search for a match amongst the application-defined functions.
+ assert(arg_count >= -2);
+ assert(arg_count >= -1 || !is_create);
+ assert(!is_create || !is_builtin);
+ uint32_t func_name_len = strlen(func_name);
+ /*
+ * The 'score' of the best match is calculated
+ * with matchQuality estimator.
+ */
+ int func_score = 0;
+ /*
+ * The pointer of a function having the highest 'score'
+ * for given name and arg_count parameters.
*/
- p = (FuncDef *) sqlHashFind(&db->aFunc, zName);
- while (p) {
- int score = matchQuality(p, nArg);
- if (score > bestScore) {
- pBest = p;
- bestScore = score;
+ struct FuncDef *func = NULL;
+ if (is_builtin)
+ goto lookup_for_builtin;
+ /* Search amongst the user-defined functions. */
+ for (struct FuncDef *p = sqlHashFind(&db->aFunc, func_name);
+ p != NULL; p = p->pNext) {
+ int score = matchQuality(p, arg_count);
+ if (score > func_score) {
+ func = p;
+ func_score = score;
}
- p = p->pNext;
}
-
- /* 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 createFlag is true, that means that we are trying to
- * install a new function. Whatever FuncDef structure is returned it will
- * have fields overwritten with new information appropriate for the
- * new function. But the FuncDefs for built-in functions are read-only.
- * So we must not search for built-ins when creating a new function.
- */
- if (!createFlag &&
- (pBest == 0
- || (user_session->sql_flags & SQL_PreferBuiltin) != 0)) {
- bestScore = 0;
- h = (sqlUpperToLower[(u8) zName[0]] +
- nName) % SQL_FUNC_HASH_SZ;
- p = functionSearch(h, zName);
- while (p) {
- int score = matchQuality(p, nArg);
- if (score > bestScore) {
- pBest = p;
- bestScore = score;
+ if (!is_create && func == NULL) {
+lookup_for_builtin:
+ func_score = 0;
+ int h = sql_builtin_func_name_hash(func_name, func_name_len);
+ for (struct FuncDef *p = functionSearch(h, func_name);
+ p != NULL; p = p->pNext) {
+ int score = matchQuality(p, arg_count);
+ if (score > func_score) {
+ func = p;
+ func_score = score;
}
- p = p->pNext;
}
}
-
- /* If the createFlag parameter is true and the search did not reveal an
- * exact match for the name, number of arguments and encoding, then add a
- * new entry to the hash table and return it.
+ /*
+ * If the is_create parameter is true and the search did
+ * not reveal an exact match for the name, number of
+ * arguments, then add a new entry to the hash table and
+ * return it.
*/
- if (createFlag && bestScore < FUNC_PERFECT_MATCH &&
- (pBest =
- sqlDbMallocZero(db, sizeof(*pBest) + nName + 1)) != 0) {
- FuncDef *pOther;
- pBest->zName = (const char *)&pBest[1];
- pBest->nArg = (u16) nArg;
- pBest->funcFlags = 0;
- memcpy((char *)&pBest[1], zName, nName + 1);
- pOther =
- (FuncDef *) sqlHashInsert(&db->aFunc, pBest->zName,
- pBest);
- if (pOther == pBest) {
- sqlDbFree(db, pBest);
+ if (is_create && func_score < FUNC_PERFECT_MATCH) {
+ uint32_t func_sz = sizeof(func[0]) + func_name_len + 1;
+ func = sqlDbMallocZero(db, func_sz);
+ if (func == NULL) {
+ diag_set(OutOfMemory, func_sz, "sqlDbMallocZero",
+ "func");
+ return NULL;
+ }
+ func->zName = (const char *) &func[1];
+ func->nArg = (u16) arg_count;
+ func->funcFlags = 0;
+ memcpy((char *)&func[1], func_name, func_name_len + 1);
+ struct FuncDef *old_func =
+ (struct FuncDef *) sqlHashInsert(&db->aFunc, func->zName,
+ func);
+ if (old_func == func) {
+ sqlDbFree(db, func);
sqlOomFault(db);
- return 0;
+ diag_set(OutOfMemory, func_sz, "sqlHashInsert",
+ "func");
+ return NULL;
} else {
- pBest->pNext = pOther;
+ func->pNext = old_func;
}
}
-
- if (pBest && (pBest->xSFunc || createFlag)) {
- return pBest;
- }
- return 0;
+ return func != NULL &&
+ (func->xSFunc != NULL || is_create || is_builtin) ? func : NULL;
}
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index ba7eea59d..f7507938d 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -285,9 +285,9 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id,
if (op == TK_FUNCTION) {
uint32_t arg_count = p->x.pList == NULL ? 0 :
p->x.pList->nExpr;
- struct FuncDef *func = sqlFindFunction(parse->db,
- p->u.zToken,
- arg_count, 0);
+ struct FuncDef *func =
+ sql_find_function(parse->db, p->u.zToken,
+ arg_count, false, false);
if (func == NULL)
break;
if (func->is_coll_derived) {
@@ -3991,14 +3991,14 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
nFarg = pFarg ? pFarg->nExpr : 0;
assert(!ExprHasProperty(pExpr, EP_IntValue));
zId = pExpr->u.zToken;
- pDef = sqlFindFunction(db, zId, nFarg, 0);
+ pDef = sql_find_function(db, zId, nFarg, false, false);
#ifdef SQL_ENABLE_UNKNOWN_SQL_FUNCTION
if (pDef == 0 && pParse->explain) {
- pDef =
- sqlFindFunction(db, "unknown", nFarg, 0);
+ pDef = sql_find_function(db, "unknown", nFarg,
+ false, false);
}
#endif
- if (pDef == 0 || pDef->xFinalize != 0) {
+ if (pDef == NULL || pDef->xFinalize != NULL) {
diag_set(ClientError, ER_NO_SUCH_FUNCTION,
zId);
pParse->is_aborted = true;
@@ -5461,12 +5461,12 @@ analyzeAggregate(Walker * pWalker, Expr * pExpr)
pItem->iMem = ++pParse->nMem;
assert(!ExprHasProperty
(pExpr, EP_IntValue));
- pItem->pFunc = sqlFindFunction(
+ pItem->pFunc = sql_find_function(
pParse->db,
pExpr->u.zToken,
pExpr->x.pList ?
pExpr->x.pList->nExpr : 0,
- 0);
+ 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 bb7405e68..cbcf26125 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 (sqlFindFunction(db, zName, nArg, 0) == 0) {
+ if (sql_find_function(db, zName, nArg, false, false) == NULL) {
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 = sqlFindFunction(db, zName, 2, 0);
- 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 = sqlFindFunction(db, expr->u.zToken, 2, 0);
+ 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 fe1135a71..d52ec1c49 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -428,29 +428,28 @@ 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 = sqlFindFunction(db, zFunctionName, nArg, 0);
- if (p && p->nArg == nArg) {
- if (db->nVdbeActive) {
- sqlErrorWithMsg(db, SQL_BUSY,
- "unable to delete/modify user-function due to active statements");
- assert(!db->mallocFailed);
- return SQL_BUSY;
- } else {
- sqlExpirePreparedStatements(db);
- }
- }
-
- p = sqlFindFunction(db, zFunctionName, nArg, 1);
- assert(p || db->mallocFailed);
- if (!p) {
- return SQL_NOMEM;
+ p = sql_find_function(db, zFunctionName, nArg, false, false);
+ if (p != NULL && p->nArg == nArg && db->nVdbeActive > 0) {
+ sqlErrorWithMsg(db, SQL_BUSY, "unable to delete/modify "
+ "user-function due to active statements");
+ return SQL_BUSY;
}
+ /* Create a new function with given signature. */
+ p = sql_find_function(db, zFunctionName, nArg, false, true);
+ if (p == NULL)
+ return SQL_TARANTOOL_ERROR;
/* If an older version of the function with a configured destructor is
* being replaced invoke the destructor function here.
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 504096e6d..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 = sqlFindFunction(pParse->db, zId, n, 0);
+ pDef = sql_find_function(pParse->db, zId, n,
+ false, false);
if (pDef == 0) {
- pDef =
- sqlFindFunction(pParse->db, zId, -2,0);
+ pDef = sql_find_function(pParse->db, zId, -2,
+ 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 d2be6701f..9384ec9e8 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 */
@@ -3927,7 +3926,29 @@ void sqlSelectSetName(Select *, const char *);
#define sqlSelectSetName(A,B)
#endif
void sqlInsertBuiltinFuncs(FuncDef *, int);
-FuncDef *sqlFindFunction(sql *, const char *, int, u8);
+
+/**
+ * Find a user function by given name and number of arguments.
+ * @param db The database handle.
+ * @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 builtin 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
+ * previously existed.
+ * @retval Returns FuncDef pointer when object was found and NULL
+ * otherwise (if is_create is not set true).
+ * When is_create is set true returns NULL only in case of
+ * memory allocation error.
+ * Sets diag message in case of error.
+ */
+struct FuncDef *
+sql_find_function(struct sql *db, const char *func_name, int arg_count,
+ bool is_builtin, bool is_create);
+
void sqlRegisterBuiltinFunctions(void);
void sqlRegisterDateTimeFunctions(void);
void sqlRegisterPerConnectionBuiltinFunctions(sql *);
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index f73ea0a71..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 = sqlFindFunction(db, p->u.zToken, nVal, 0);
+ 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