Tarantool development patches archive
 help / color / mirror / Atom feed
From: Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: v.shpilevoy@tarantool.org
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH v2 3/6] sql: remove SQL built-in functions from _func
Date: Wed,  4 Aug 2021 15:58:41 +0300	[thread overview]
Message-ID: <cd575f0326c6276a0b44470532acff22eb97f694.1628081224.git.imeevma@gmail.com> (raw)
In-Reply-To: <cover.1628081224.git.imeevma@gmail.com>

Hi! Thank you for the review! I did not include diff since there were too many
changes and part of them were moved to the other patches. My answers and new
patch below.

On 02.08.2021 23:12, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
>
> See 11 comments below.
>
> On 30.07.2021 00:48, imeevma@tarantool.org wrote:
>> This patch removes SQL built-in functions from _func. These functions
>> could be called directly from Lua, however all they did was returnd an
>
> 1. returnd -> returned.
>
Fixed.

>> error. After this patch, no SQL built-in functions can be called
>> directly from LUA.
>>
>> Closes #6106
>> ---
>> https://github.com/tarantool/tarantool/issues/6106
>> https://github.com/tarantool/tarantool/tree/imeevma/gh-6106-remove-sql-builtins-from-_func
>>> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
>> index 97afc0b4d..d0ca3db97 100644
>> --- a/src/box/lua/upgrade.lua
>> +++ b/src/box/lua/upgrade.lua
>> @@ -1003,19 +1003,19 @@ end
>>  --------------------------------------------------------------------------------
>>  -- Tarantool 2.9.1
>>  --------------------------------------------------------------------------------
>> -local function sql_builtin_function_uuid()
>> +local function remove_sql_builtin_functions_from_func()
>>      local _func = box.space._func
>>      local _priv = box.space._priv
>> -    local datetime = os.date("%Y-%m-%d %H:%M:%S")
>> -    local t = _func:auto_increment({ADMIN, 'UUID', 1, 'SQL_BUILTIN', '',
>> -                                    'function', {}, 'any', 'none', 'none',
>> -                                    false, false, true, {}, setmap({}), '',
>> -                                    datetime, datetime})
>> -    _priv:replace{ADMIN, PUBLIC, 'function', t.id, box.priv.X}
>> +    for _, v in _func:pairs() do
>> +        if (v.language == "SQL_BUILTIN") then
>
> 2. You do not need () around the condition.
>
Fixed.

>> +            _priv:delete({2, 'function', v.id})
>> +            _func:delete({v.id})
>> +        end
>> +    end
>>  end
>> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
>> index 3772596d6..7c1616a3a 100644
>> --- a/src/box/sql/expr.c
>> +++ b/src/box/sql/expr.c
>> @@ -326,11 +326,13 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id,
>>  			break;
>>  		}
>>  		if (op == TK_FUNCTION) {
>> -			uint32_t arg_count = p->x.pList == NULL ? 0 :
>> -					     p->x.pList->nExpr;
>> +			int arg_count = p->x.pList == NULL ? 0 :
>> +					p->x.pList->nExpr;
>>  			struct func *func =
>> -				sql_func_by_signature(p->u.zToken, arg_count);
>> -			if (func == NULL)
>> +				sql_func_by_name(p->u.zToken);
>> +			if (func == NULL || !func->def->exports.sql ||
>
> 3. Why don't you filter by !func->def->exports.sql in sql_func_by_name()?
> Is it possible in some places to call a function not exported to SQL?
>
Done, I moved all similar checks to new function sql_func_find().

>> +			    (func->def->param_count != -1 &&
>> +			     func->def->param_count != arg_count))
>
> 4. Why is argument count important? Isn't SQL_FUNC_DERIVEDCOLL below
> enough?
>
Fixed. I think here it is not important. Also, in some cases we only need
function flags without function itself. I used this place to introduce it, but
its real usage will be later, during addition of static type checking for SQL
built-in functions.

> The same 2 comments in the next diff block.
>
In these two places and one place in resolve.c I used new function, that have
all needed checks inside of it.

>>  				break;
>>  			if (sql_func_flag_is_set(func, SQL_FUNC_DERIVEDCOLL) &&
>>  			    arg_count > 0) {
>> @@ -3979,8 +3981,10 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
>>  			nFarg = pFarg ? pFarg->nExpr : 0;
>>  			assert(!ExprHasProperty(pExpr, EP_IntValue));
>>  			zId = pExpr->u.zToken;
>> -			struct func *func = sql_func_by_signature(zId, nFarg);
>> -			if (func == NULL) {
>> +			struct func *func = sql_func_by_name(zId);
>> +			if (func == NULL || !func->def->exports.sql ||
>> +			    (func->def->param_count != -1 &&
>> +			     func->def->param_count != nFarg)) {
>>  				diag_set(ClientError, ER_NO_SUCH_FUNCTION,
>>  					 zId);
>>  				pParse->is_aborted = true;
>> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
>> index 6ca852dec..b05038e2e 100644
>> --- a/src/box/sql/func.c
>> +++ b/src/box/sql/func.c
>> @@ -47,6 +47,37 @@
>>  #include <unicode/ucol.h>
>>  #include "box/coll_id_cache.h"
>>  #include "box/schema.h"
>> +#include "core/assoc.h"
>
> 5. core/ prefix is not not necessary. Also it might look better if
> you group all box/ includes together.
>
Fixed.

>> +#include "box/user.h"
>> +
>> +static struct mh_strnptr_t *built_in_functions = NULL;
>> +
>> +static struct func *
>> +get_built_in_function(const char *name)
>> +{
>> +	uint32_t len = strlen(name);
>> +	mh_int_t func = mh_strnptr_find_inp(built_in_functions, name, len);
>> +	if (func == mh_end(built_in_functions))
>> +		return NULL;
>> +	return (struct func *) mh_strnptr_node(built_in_functions, func)->val;
>
> 6. In the new code we do not use whitespaces after unary
> operators like cast. But even better - in C you do not need
> an explicit cast from void * to any other pointer.
>
Thanks, fixed.
>> +}
>> +
>> +static int
>> +built_in_functions_cache_insert(struct func *func)
>
> 7. Please, make the hash methods have the same prefix. For instance,
> sql_builtin_func_get(), sql_builtin_func_put().
>
Thanks, replaced to suggested names.

>> +{
>> +	const char *name = func->def->name;
>> +	uint32_t len = strlen(name);
>> +	assert(get_built_in_function(name) == NULL);
>> +
>> +	uint32_t hash = mh_strn_hash(name, len);
>> +	const struct mh_strnptr_node_t strnode = {name, len, hash, func};
>> +	mh_int_t k = mh_strnptr_put(built_in_functions, &strnode, NULL, NULL);
>> +	if (k == mh_end(built_in_functions)) {
>> +		diag_set(OutOfMemory, sizeof(strnode), "malloc", "strnode");
>> +		return -1;
>> +	}
>> +	return 0;
>> +}
>> @@ -2657,6 +2685,67 @@ static struct {
>>  
>>  static struct func_vtab func_sql_builtin_vtab;
>>  
>> +int
>> +sql_built_in_functions_cache_init(void)
>> +{
>> +	built_in_functions = mh_strnptr_new();
>
> 8. It might make sense to check the result for NULL and call
> panic() if it is.
>
Fixed.

>> +	for (uint32_t i = 0; i < nelem(sql_builtins); ++i) {
>> +		const char *name = sql_builtins[i].name;
>> +		uint32_t len = strlen(name);
>> +		uint32_t size = sizeof(struct func_def) + len + 1;
>> +		struct func_def *def = malloc(size);
>
> 9. Ditto.
>
Fixed.

>> +		def->fid = i;
>> +		def->uid = 1;
>> +		def->body = NULL;
>> +		def->comment = NULL;
>> +		def->setuid = true;
>> +		def->is_deterministic = sql_builtins[i].is_deterministic;
>> +		def->is_sandboxed = false;
>> +		def->param_count = sql_builtins[i].param_count;
>> +		def->returns = sql_builtins[i].returns;
>> +		def->aggregate = sql_builtins[i].aggregate;
>> +		def->language = FUNC_LANGUAGE_SQL_BUILTIN;
>> +		def->name_len = len;
>> +		def->exports.sql = sql_builtins[i].export_to_sql;
>> +		func_opts_create(&def->opts);
>> +		memcpy(def->name, name, len + 1);
>> +
>> +		struct func_sql_builtin *func = malloc(sizeof(*func));
>> +		if (func == NULL) {
>> +			diag_set(OutOfMemory, sizeof(*func), "malloc", "func");
>> +			return -1;
>
> 10. The result of sql_built_in_functions_cache_init() is never checked.
> I would suggest to make it void and panic on all errors.
>
Fixed.

>> +		}
>> +
>> +		func->base.def = def;
>> +		func->base.vtab = &func_sql_builtin_vtab;
>> +		credentials_create_empty(&func->base.owner_credentials);
>> +		memset(func->base.access, 0, sizeof(func->base.access));
>> +
>> +		func->flags = sql_builtins[i].flags;
>> +		func->call = sql_builtins[i].call;
>> +		func->finalize = sql_builtins[i].finalize;
>> +		if (built_in_functions_cache_insert(&func->base) != 0)
>> +			return -1;
>> +	}
>> +	return 0;
>> +}
>> +
>> +void
>> +sql_built_in_functions_cache_free(void)
>> +{
>> +	if (built_in_functions == NULL)
>> +		return;
>> +	for (uint32_t i = 0; i < nelem(sql_builtins); ++i) {
>> +		const char *name = sql_builtins[i].name;
>> +		uint32_t len = strlen(name);
>> +		mh_int_t k = mh_strnptr_find_inp(built_in_functions, name, len);
>> +		if (k == mh_end(built_in_functions))
>> +			continue;
>> +		mh_strnptr_del(built_in_functions, k, NULL);
>
> 11. Perhaps a more 'canonical' way would be like
>
> 	while (mh_size() > 0) { mh_del(mh_first()); ... }
>
I believe it cannot be implemented on hash table with string key. Added an
assert to check that hash table is empty.

> Also you didn't free the function objects.
>
Fixed.

>> +	}
>> +	mh_strnptr_delete(built_in_functions);
>> +}



New patch:

commit cd575f0326c6276a0b44470532acff22eb97f694
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Thu Jul 29 13:54:53 2021 +0300

    sql: remove SQL built-in functions from _func
    
    This patch removes SQL built-in functions from _func. These functions
    could be called directly from Lua, however all they did was returned an
    error. After this patch, no SQL built-in functions can be called
    directly from LUA.
    
    Part of #6106

diff --git a/changelogs/unreleased/gh-6106-remove-sql-built-ins-from-_func.md b/changelogs/unreleased/gh-6106-remove-sql-built-ins-from-_func.md
new file mode 100644
index 000000000..88b458540
--- /dev/null
+++ b/changelogs/unreleased/gh-6106-remove-sql-built-ins-from-_func.md
@@ -0,0 +1,7 @@
+## feature/sql
+
+* SQL built-in functions were removed from \_func system space (gh-6106).
+* After this patch, functions will be looked up first in SQL built-in functions
+  and then in user-defined functions.
+* Fixed incorrect error message in case of misuse of the function used to set
+  the default value.
\ No newline at end of file
diff --git a/src/box/alter.cc b/src/box/alter.cc
index 390199298..935790df4 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -3548,13 +3548,6 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
 				  "function has references");
 			return -1;
 		}
-		/* Can't' drop a builtin function. */
-		if (old_func->def->language == FUNC_LANGUAGE_SQL_BUILTIN) {
-			diag_set(ClientError, ER_DROP_FUNCTION,
-				  (unsigned) old_func->def->uid,
-				  "function is SQL built-in");
-			return -1;
-		}
 		struct trigger *on_commit =
 			txn_alter_trigger_new(on_drop_func_commit, old_func);
 		struct trigger *on_rollback =
diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap
index 57374decc..018670d2a 100644
Binary files a/src/box/bootstrap.snap and b/src/box/bootstrap.snap differ
diff --git a/src/box/box.cc b/src/box/box.cc
index 0ff7ee468..a7eda4214 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -2914,6 +2914,7 @@ box_free(void)
 		gc_free();
 		engine_shutdown();
 		wal_free();
+		sql_built_in_functions_cache_free();
 	}
 }
 
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index 97afc0b4d..6abce50f4 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -1003,19 +1003,19 @@ end
 --------------------------------------------------------------------------------
 -- Tarantool 2.9.1
 --------------------------------------------------------------------------------
-local function sql_builtin_function_uuid()
+local function remove_sql_builtin_functions_from_func()
     local _func = box.space._func
     local _priv = box.space._priv
-    local datetime = os.date("%Y-%m-%d %H:%M:%S")
-    local t = _func:auto_increment({ADMIN, 'UUID', 1, 'SQL_BUILTIN', '',
-                                    'function', {}, 'any', 'none', 'none',
-                                    false, false, true, {}, setmap({}), '',
-                                    datetime, datetime})
-    _priv:replace{ADMIN, PUBLIC, 'function', t.id, box.priv.X}
+    for _, v in _func:pairs() do
+        if v.language == "SQL_BUILTIN" then
+            _priv:delete({2, 'function', v.id})
+            _func:delete({v.id})
+        end
+    end
 end
 
 local function upgrade_to_2_9_1()
-    sql_builtin_function_uuid()
+    remove_sql_builtin_functions_from_func()
 end
 
 --------------------------------------------------------------------------------
diff --git a/src/box/sql.c b/src/box/sql.c
index 433264abe..f18dfa063 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -75,6 +75,7 @@ sql_init(void)
 		panic("failed to initialize SQL subsystem");
 
 	sql_stmt_cache_init();
+	sql_built_in_functions_cache_init();
 
 	assert(db != NULL);
 }
diff --git a/src/box/sql.h b/src/box/sql.h
index 4c364306c..2ac97c762 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -56,6 +56,15 @@ sql_init(void);
 struct sql *
 sql_get(void);
 
+/** Initialize global cache for built-in functions. */
+void
+sql_built_in_functions_cache_init(void);
+
+/** Free global cache for built-in functions. */
+void
+sql_built_in_functions_cache_free(void);
+
+
 struct Expr;
 struct Parse;
 struct Select;
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 9514d070a..2a3a5d457 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -47,6 +47,10 @@
 #include <unicode/ucol.h>
 #include "box/coll_id_cache.h"
 #include "box/schema.h"
+#include "box/user.h"
+#include "assoc.h"
+
+static struct mh_strnptr_t *built_in_functions = NULL;
 
 static const unsigned char *
 mem_as_ustr(struct Mem *mem)
@@ -2643,11 +2647,49 @@ static struct {
 	},
 };
 
+static struct func *
+built_in_func_get(const char *name)
+{
+	uint32_t len = strlen(name);
+	mh_int_t k = mh_strnptr_find_inp(built_in_functions, name, len);
+	if (k == mh_end(built_in_functions))
+		return NULL;
+	return mh_strnptr_node(built_in_functions, k)->val;
+}
+
+static void
+built_in_func_put(struct func *func)
+{
+	const char *name = func->def->name;
+	uint32_t len = strlen(name);
+	assert(built_in_func_get(name) == NULL);
+
+	uint32_t hash = mh_strn_hash(name, len);
+	const struct mh_strnptr_node_t strnode = {name, len, hash, func};
+	mh_int_t k = mh_strnptr_put(built_in_functions, &strnode, NULL, NULL);
+	if (k == mh_end(built_in_functions)) {
+		panic("Out of memory on insertion into SQL built-in functions "
+		      "hash");
+	}
+}
+
 struct func *
 sql_func_find(struct Expr *expr)
 {
 	const char *name = expr->u.zToken;
-	struct func *func = func_by_name(name, strlen(name));
+	int n = expr->x.pList ? expr->x.pList->nExpr : 0;
+	struct func *func = built_in_func_get(name);
+	if (func != NULL) {
+		assert(func->def->exports.sql);
+		int param_count = func->def->param_count;
+		if (param_count != -1 && param_count != n) {
+			diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, name,
+				 tt_sprintf("%d", func->def->param_count), n);
+			return NULL;
+		}
+		return func;
+	}
+	func = func_by_name(name, strlen(name));
 	if (func == NULL) {
 		diag_set(ClientError, ER_NO_SUCH_FUNCTION, name);
 		return NULL;
@@ -2658,8 +2700,7 @@ sql_func_find(struct Expr *expr)
 				     name));
 		return NULL;
 	}
-	int n = expr->x.pList ? expr->x.pList->nExpr : 0;
-	if (func->def->param_count != -1 && func->def->param_count != n) {
+	if (func->def->param_count != n) {
 		diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, name,
 			 tt_sprintf("%d", func->def->param_count), n);
 		return NULL;
@@ -2670,9 +2711,10 @@ sql_func_find(struct Expr *expr)
 uint32_t
 sql_func_flags(const char *name)
 {
-	struct func *func = func_by_name(name, strlen(name));
-	if (func == NULL || func->def->language != FUNC_LANGUAGE_SQL_BUILTIN)
+	struct func *func = built_in_func_get(name);
+	if (func == NULL)
 		return 0;
+	assert(func->def->language == FUNC_LANGUAGE_SQL_BUILTIN);
 	uint32_t flags = ((struct func_sql_builtin *)func)->flags;
 	if (func->def->aggregate == FUNC_AGGREGATE_GROUP)
 		flags |= SQL_FUNC_AGG;
@@ -2681,6 +2723,72 @@ sql_func_flags(const char *name)
 
 static struct func_vtab func_sql_builtin_vtab;
 
+void
+sql_built_in_functions_cache_init(void)
+{
+	built_in_functions = mh_strnptr_new();
+	if (built_in_functions == NULL)
+		panic("Out of memory on creating SQL built-in functions hash");
+	for (uint32_t i = 0; i < nelem(sql_builtins); ++i) {
+		const char *name = sql_builtins[i].name;
+		if (!sql_builtins[i].export_to_sql)
+			continue;
+		uint32_t len = strlen(name);
+		uint32_t size = sizeof(struct func_def) + len + 1;
+		struct func_def *def = malloc(size);
+		if (def == NULL)
+			panic("Out of memory on creating SQL built-in");
+		def->fid = i;
+		def->uid = 1;
+		def->body = NULL;
+		def->comment = NULL;
+		def->setuid = true;
+		def->is_deterministic = sql_builtins[i].is_deterministic;
+		def->is_sandboxed = false;
+		def->param_count = sql_builtins[i].param_count;
+		def->returns = sql_builtins[i].returns;
+		def->aggregate = sql_builtins[i].aggregate;
+		def->language = FUNC_LANGUAGE_SQL_BUILTIN;
+		def->name_len = len;
+		def->exports.sql = sql_builtins[i].export_to_sql;
+		func_opts_create(&def->opts);
+		memcpy(def->name, name, len + 1);
+
+		struct func_sql_builtin *func = malloc(sizeof(*func));
+		if (func == NULL)
+			panic("Out of memory on creating SQL built-in");
+
+		func->base.def = def;
+		func->base.vtab = &func_sql_builtin_vtab;
+		credentials_create_empty(&func->base.owner_credentials);
+		memset(func->base.access, 0, sizeof(func->base.access));
+
+		func->flags = sql_builtins[i].flags;
+		func->call = sql_builtins[i].call;
+		func->finalize = sql_builtins[i].finalize;
+		built_in_func_put(&func->base);
+	}
+}
+
+void
+sql_built_in_functions_cache_free(void)
+{
+	if (built_in_functions == NULL)
+		return;
+	for (uint32_t i = 0; i < nelem(sql_builtins); ++i) {
+		const char *name = sql_builtins[i].name;
+		uint32_t len = strlen(name);
+		mh_int_t k = mh_strnptr_find_inp(built_in_functions, name, len);
+		if (k == mh_end(built_in_functions))
+			continue;
+		struct func *func = mh_strnptr_node(built_in_functions, k)->val;
+		mh_strnptr_del(built_in_functions, k, NULL);
+		func_delete(func);
+	}
+	assert(mh_size(built_in_functions) == 0);
+	mh_strnptr_delete(built_in_functions);
+}
+
 struct func *
 func_sql_builtin_new(struct func_def *def)
 {
diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result
index b2328487c..cea440c64 100644
--- a/test/box-py/bootstrap.result
+++ b/test/box-py/bootstrap.result
@@ -176,73 +176,7 @@ box.space._priv:select{}
   - [1, 0, 'universe', 0, 24]
   - [1, 1, 'universe', 0, 4294967295]
   - [1, 2, 'function', 1, 4]
-  - [1, 2, 'function', 2, 4]
-  - [1, 2, 'function', 3, 4]
-  - [1, 2, 'function', 4, 4]
-  - [1, 2, 'function', 5, 4]
-  - [1, 2, 'function', 6, 4]
-  - [1, 2, 'function', 7, 4]
-  - [1, 2, 'function', 8, 4]
-  - [1, 2, 'function', 9, 4]
-  - [1, 2, 'function', 10, 4]
-  - [1, 2, 'function', 11, 4]
-  - [1, 2, 'function', 12, 4]
-  - [1, 2, 'function', 13, 4]
-  - [1, 2, 'function', 14, 4]
-  - [1, 2, 'function', 15, 4]
-  - [1, 2, 'function', 16, 4]
-  - [1, 2, 'function', 17, 4]
-  - [1, 2, 'function', 18, 4]
-  - [1, 2, 'function', 19, 4]
-  - [1, 2, 'function', 20, 4]
-  - [1, 2, 'function', 21, 4]
-  - [1, 2, 'function', 22, 4]
-  - [1, 2, 'function', 23, 4]
-  - [1, 2, 'function', 24, 4]
-  - [1, 2, 'function', 25, 4]
-  - [1, 2, 'function', 26, 4]
-  - [1, 2, 'function', 27, 4]
-  - [1, 2, 'function', 28, 4]
-  - [1, 2, 'function', 29, 4]
-  - [1, 2, 'function', 30, 4]
-  - [1, 2, 'function', 31, 4]
-  - [1, 2, 'function', 32, 4]
-  - [1, 2, 'function', 33, 4]
-  - [1, 2, 'function', 34, 4]
-  - [1, 2, 'function', 35, 4]
-  - [1, 2, 'function', 36, 4]
-  - [1, 2, 'function', 37, 4]
-  - [1, 2, 'function', 38, 4]
-  - [1, 2, 'function', 39, 4]
-  - [1, 2, 'function', 40, 4]
-  - [1, 2, 'function', 41, 4]
-  - [1, 2, 'function', 42, 4]
-  - [1, 2, 'function', 43, 4]
-  - [1, 2, 'function', 44, 4]
-  - [1, 2, 'function', 45, 4]
-  - [1, 2, 'function', 46, 4]
-  - [1, 2, 'function', 47, 4]
-  - [1, 2, 'function', 48, 4]
-  - [1, 2, 'function', 49, 4]
-  - [1, 2, 'function', 50, 4]
-  - [1, 2, 'function', 51, 4]
-  - [1, 2, 'function', 52, 4]
-  - [1, 2, 'function', 53, 4]
-  - [1, 2, 'function', 54, 4]
-  - [1, 2, 'function', 55, 4]
-  - [1, 2, 'function', 56, 4]
-  - [1, 2, 'function', 57, 4]
-  - [1, 2, 'function', 58, 4]
-  - [1, 2, 'function', 59, 4]
-  - [1, 2, 'function', 60, 4]
-  - [1, 2, 'function', 61, 4]
-  - [1, 2, 'function', 62, 4]
-  - [1, 2, 'function', 63, 4]
-  - [1, 2, 'function', 64, 4]
   - [1, 2, 'function', 65, 4]
-  - [1, 2, 'function', 66, 4]
-  - [1, 2, 'function', 67, 4]
-  - [1, 2, 'function', 68, 4]
   - [1, 2, 'space', 276, 2]
   - [1, 2, 'space', 277, 1]
   - [1, 2, 'space', 281, 1]
diff --git a/test/box/access_bin.result b/test/box/access_bin.result
index aeb8b3bd8..7c720192f 100644
--- a/test/box/access_bin.result
+++ b/test/box/access_bin.result
@@ -295,10 +295,10 @@ test:drop()
 box.schema.user.grant('guest', 'execute', 'universe')
 ---
 ...
-function f1() return box.space._func:get(1)[4] end
+function f1() return box.space._func.index[2]:get({'f1'})[4] end
 ---
 ...
-function f2() return box.space._func:get(69)[4] end
+function f2() return box.space._func.index[2]:get({'f1'})[4] end
 ---
 ...
 box.schema.func.create('f1')
diff --git a/test/box/access_bin.test.lua b/test/box/access_bin.test.lua
index 954266858..e82ec759c 100644
--- a/test/box/access_bin.test.lua
+++ b/test/box/access_bin.test.lua
@@ -111,8 +111,8 @@ test:drop()
 --
 -- notice that guest can execute stuff, but can't read space _func
 box.schema.user.grant('guest', 'execute', 'universe')
-function f1() return box.space._func:get(1)[4] end
-function f2() return box.space._func:get(69)[4] end
+function f1() return box.space._func.index[2]:get({'f1'})[4] end
+function f2() return box.space._func.index[2]:get({'f1'})[4] end
 box.schema.func.create('f1')
 box.schema.func.create('f2',{setuid=true})
 c = net.connect(box.cfg.listen)
diff --git a/test/box/access_sysview.result b/test/box/access_sysview.result
index d7a7b7534..071fc8de2 100644
--- a/test/box/access_sysview.result
+++ b/test/box/access_sysview.result
@@ -258,11 +258,11 @@ box.session.su('guest')
 ...
 #box.space._vpriv:select{}
 ---
-- 83
+- 17
 ...
 #box.space._vfunc:select{}
 ---
-- 68
+- 2
 ...
 #box.space._vcollation:select{}
 ---
@@ -290,11 +290,11 @@ box.session.su('guest')
 ...
 #box.space._vpriv:select{}
 ---
-- 83
+- 17
 ...
 #box.space._vfunc:select{}
 ---
-- 68
+- 2
 ...
 #box.space._vsequence:select{}
 ---
diff --git a/test/box/function1.result b/test/box/function1.result
index 0166c828f..a49a133f7 100644
--- a/test/box/function1.result
+++ b/test/box/function1.result
@@ -97,7 +97,7 @@ box.func["function1.args"]
   exports:
     lua: true
     sql: false
-  id: 69
+  id: 66
   setuid: false
   is_multikey: false
   is_deterministic: false
@@ -593,7 +593,7 @@ func
   exports:
     lua: true
     sql: false
-  id: 69
+  id: 66
   setuid: false
   is_multikey: false
   is_deterministic: false
@@ -665,7 +665,7 @@ func
   exports:
     lua: true
     sql: false
-  id: 69
+  id: 66
   setuid: false
   is_multikey: false
   is_deterministic: false
@@ -1032,7 +1032,7 @@ box.func.test ~= nil
 box.func.test:drop()
 ---
 ...
--- Check SQL builtins
+-- Make sure there is no SQL built-in functions in _func.
 test_run:cmd("setopt delimiter ';'")
 ---
 - true
@@ -1048,7 +1048,7 @@ sql_builtin_list = {
 	"RANDOMBLOB", "NULLIF", "ZEROBLOB", "MIN", "MAX", "COALESCE", "EVERY",
 	"EXISTS", "EXTRACT", "SOME", "GREATER", "LESSER", "SOUNDEX",
 	"LIKELIHOOD", "LIKELY", "UNLIKELY", "_sql_stat_get", "_sql_stat_push",
-	"_sql_stat_init", "GREATEST", "LEAST"
+	"_sql_stat_init", "GREATEST", "LEAST", "UUID"
 }
 test_run:cmd("setopt delimiter ''");
 ---
@@ -1056,7 +1056,7 @@ test_run:cmd("setopt delimiter ''");
 ok = true
 ---
 ...
-for _, v in pairs(sql_builtin_list) do ok = ok and (box.space._func.index.name:get(v) ~= nil) end
+for _, v in pairs(sql_builtin_list) do ok = ok and (box.space._func.index.name:get(v) == nil) end
 ---
 ...
 ok == true
@@ -1067,26 +1067,6 @@ box.func.LUA:call({"return 1 + 1"})
 ---
 - 2
 ...
-box.schema.user.grant('guest', 'execute', 'function', 'SUM')
----
-...
-c = net.connect(box.cfg.listen)
----
-...
-c:call("SUM")
----
-- error: sql builtin function does not support Lua frontend
-...
-c:close()
----
-...
-box.schema.user.revoke('guest', 'execute', 'function', 'SUM')
----
-...
-box.schema.func.drop("SUM")
----
-- error: 'Can''t drop function 1: function is SQL built-in'
-...
 -- Introduce function options
 box.schema.func.create('test', {body = "function(tuple) return tuple end", is_deterministic = true, opts = {is_multikey = true}})
 ---
diff --git a/test/box/function1.test.lua b/test/box/function1.test.lua
index ab7b586a0..4fdd48520 100644
--- a/test/box/function1.test.lua
+++ b/test/box/function1.test.lua
@@ -363,7 +363,7 @@ f == nil
 box.func.test ~= nil
 box.func.test:drop()
 
--- Check SQL builtins
+-- Make sure there is no SQL built-in functions in _func.
 test_run:cmd("setopt delimiter ';'")
 sql_builtin_list = {
 	"TRIM", "TYPEOF", "PRINTF", "UNICODE", "CHAR", "HEX", "VERSION",
@@ -376,22 +376,15 @@ sql_builtin_list = {
 	"RANDOMBLOB", "NULLIF", "ZEROBLOB", "MIN", "MAX", "COALESCE", "EVERY",
 	"EXISTS", "EXTRACT", "SOME", "GREATER", "LESSER", "SOUNDEX",
 	"LIKELIHOOD", "LIKELY", "UNLIKELY", "_sql_stat_get", "_sql_stat_push",
-	"_sql_stat_init", "GREATEST", "LEAST"
+	"_sql_stat_init", "GREATEST", "LEAST", "UUID"
 }
 test_run:cmd("setopt delimiter ''");
 ok = true
-for _, v in pairs(sql_builtin_list) do ok = ok and (box.space._func.index.name:get(v) ~= nil) end
+for _, v in pairs(sql_builtin_list) do ok = ok and (box.space._func.index.name:get(v) == nil) end
 ok == true
 
 box.func.LUA:call({"return 1 + 1"})
 
-box.schema.user.grant('guest', 'execute', 'function', 'SUM')
-c = net.connect(box.cfg.listen)
-c:call("SUM")
-c:close()
-box.schema.user.revoke('guest', 'execute', 'function', 'SUM')
-box.schema.func.drop("SUM")
-
 -- Introduce function options
 box.schema.func.create('test', {body = "function(tuple) return tuple end", is_deterministic = true, opts = {is_multikey = true}})
 box.func['test'].is_multikey == true
diff --git a/test/sql-tap/func5.test.lua b/test/sql-tap/func5.test.lua
index 13698582b..bd4561afc 100755
--- a/test/sql-tap/func5.test.lua
+++ b/test/sql-tap/func5.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 local test = require("sqltester")
-test:plan(27)
+test:plan(29)
 
 --!./tcltestrunner.lua
 -- 2010 August 27
@@ -346,4 +346,27 @@ box.func.F2:drop()
 box.space.T01:drop()
 box.space.T02:drop()
 
+--
+-- gh-6105:  Make sure that functions that were described in _func but were not
+-- implemented are now removed.
+--
+test:do_catchsql_test(
+    "func-7.3",
+    [[
+        SELECT SQRT();
+    ]], {
+        1, "Function 'SQRT' does not exist"
+    })
+
+-- Make sure that functions are looked up in built-in functions first.
+box.schema.func.create('ABS', {language = 'Lua', param_list = {"INTEGER"},
+                       body = body, returns = 'number', exports = {'LUA'}});
+test:do_execsql_test(
+    "func-7.4",
+    [[
+        SELECT ABS(-111);
+    ]], {
+        111
+    })
+
 test:finish_test()
diff --git a/test/wal_off/func_max.result b/test/wal_off/func_max.result
index cc5bcc141..a3ab5b431 100644
--- a/test/wal_off/func_max.result
+++ b/test/wal_off/func_max.result
@@ -42,11 +42,11 @@ test_run:cmd("setopt delimiter ''");
 ...
 func_limit()
 ---
-- error: 'Failed to create function ''func31933'': function id is too big'
+- error: 'Failed to create function ''func31936'': function id is too big'
 ...
 drop_limit_func()
 ---
-- error: Function 'func31933' does not exist
+- error: Function 'func31936' does not exist
 ...
 box.schema.user.create('testuser')
 ---
@@ -62,11 +62,11 @@ session.su('testuser')
 ...
 func_limit()
 ---
-- error: 'Failed to create function ''func31933'': function id is too big'
+- error: 'Failed to create function ''func31936'': function id is too big'
 ...
 drop_limit_func()
 ---
-- error: Function 'func31933' does not exist
+- error: Function 'func31936' does not exist
 ...
 session.su('admin')
 ---

  parent reply	other threads:[~2021-08-04 13:00 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-04 12:58 [Tarantool-patches] [PATCH v2 0/6] Remove " Mergen Imeev via Tarantool-patches
2021-08-04 12:58 ` [Tarantool-patches] [PATCH v2 1/6] sql: introduce sql_func_flags() Mergen Imeev via Tarantool-patches
2021-08-05 22:14   ` Vladislav Shpilevoy via Tarantool-patches
2021-08-06 19:41     ` Mergen Imeev via Tarantool-patches
2021-08-04 12:58 ` [Tarantool-patches] [PATCH v2 2/6] sql: introduce sql_func_find() Mergen Imeev via Tarantool-patches
2021-08-05 22:15   ` Vladislav Shpilevoy via Tarantool-patches
2021-08-06 19:42     ` Mergen Imeev via Tarantool-patches
2021-08-04 12:58 ` Mergen Imeev via Tarantool-patches [this message]
2021-08-05 22:17   ` [Tarantool-patches] [PATCH v2 3/6] sql: remove SQL built-in functions from _func Vladislav Shpilevoy via Tarantool-patches
2021-08-06 19:45     ` Mergen Imeev via Tarantool-patches
2021-08-04 12:58 ` [Tarantool-patches] [PATCH v2 4/6] alter: parse data dictionary version Mergen Imeev via Tarantool-patches
2021-08-05 22:17   ` Vladislav Shpilevoy via Tarantool-patches
2021-08-06 19:47     ` Mergen Imeev via Tarantool-patches
2021-08-04 12:58 ` [Tarantool-patches] [PATCH v2 5/6] alter: disallow creation of SQL built-in function Mergen Imeev via Tarantool-patches
2021-08-05 22:18   ` Vladislav Shpilevoy via Tarantool-patches
2021-08-06 19:54     ` Mergen Imeev via Tarantool-patches
2021-08-04 12:58 ` [Tarantool-patches] [PATCH v2 6/6] sql: remove unnecessary function initialization Mergen Imeev via Tarantool-patches
2021-08-06 19:59   ` Mergen Imeev via Tarantool-patches
2021-08-08 12:08 ` [Tarantool-patches] [PATCH v2 0/6] Remove SQL built-in functions from _func Vladislav Shpilevoy via Tarantool-patches
2021-08-09  7:18 Mergen Imeev via Tarantool-patches
2021-08-09  7:18 ` [Tarantool-patches] [PATCH v2 3/6] sql: remove " Mergen Imeev via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cd575f0326c6276a0b44470532acff22eb97f694.1628081224.git.imeevma@gmail.com \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imeevma@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2 3/6] sql: remove SQL built-in functions from _func' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox