From 1beb68918a8cb72caab11ab38fbf7b3aedd03bb1 Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov@tarantool.org>
Date: Fri, 2 Sep 2022 11:21:15 +0300
Subject: [PATCH] func: factor out func_def_new and func_def_delete

func_def_new takes function id, name, body, comment, and owner id and
allocates a new func_def struct, setting the rest of the members to
their default values. We need this function to create a new func_def
object for handling space upgrade in read view.

Note, this isn't a pure refactoring - before this patch, we used
FUNC_LANGUAGE_LUA for SQL builtin functions, which were deprecated in
2.9. This worked fine, because we never actually called them - it was
needed solely for upgrade from older versions. In this commit, we create
an SQL builtin function just like any other function, but set its vtab
to a dummy, which raises an error on an attempt to call it. This should
make the code clearer.

Needed for https://github.com/tarantool/tarantool-ee/issues/163

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
---
 src/box/alter.cc       | 138 ++++++++++++-----------------------------
 src/box/func.c         |  36 +++++++++++
 src/box/func_def.c     |  49 +++++++++++++++
 src/box/func_def.h     |  39 +++++-------
 src/box/sql/func.c     |  21 ++-----
 test/unit/func_cache.c |  10 ++-
 6 files changed, 148 insertions(+), 145 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 0d26d9f32e..b991fc19d9 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -3119,36 +3119,6 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
 	return 0;
 }
 
-/**
- * Check if the version of the data dictionary is lower than 2.9.0 and return
- * new func def if it is the case. If it is the case, then it is possible to
- * insert values with the "SQL_BUILTIN" language into _func, otherwise it is
- * prohibited. This is for upgradeability from 2.1.3 to 2.3.0. Since all we need
- * is to allow such inserts, we set func def to its default values.
- */
-static int
-func_def_create_sql_built_in(struct func_def *def)
-{
-	if (dd_version_id >= version_id(2, 9, 0)) {
-		diag_set(ClientError, ER_FUNCTION_LANGUAGE, "SQL_BUILTIN",
-			 def->name);
-		return -1;
-	}
-	def->body = NULL;
-	def->comment = NULL;
-	def->setuid = 1;
-	def->is_deterministic = false;
-	def->is_sandboxed = false;
-	def->param_count = 0;
-	def->returns = FIELD_TYPE_ANY;
-	def->aggregate = FUNC_AGGREGATE_NONE;
-	def->language = FUNC_LANGUAGE_LUA;
-	def->exports.lua = true;
-	def->exports.sql = true;
-	func_opts_create(&def->opts);
-	return 0;
-}
-
 /**
  * Get function identifiers from a tuple.
  *
@@ -3169,9 +3139,9 @@ static struct func_def *
 func_def_new_from_tuple(struct tuple *tuple)
 {
 	uint32_t field_count = tuple_field_count(tuple);
-	uint32_t name_len, body_len, comment_len;
-	const char *name, *body, *comment;
-	name = tuple_field_str(tuple, BOX_FUNC_FIELD_NAME, &name_len);
+	uint32_t name_len;
+	const char *name = tuple_field_str(tuple, BOX_FUNC_FIELD_NAME,
+					   &name_len);
 	if (name == NULL)
 		return NULL;
 	if (name_len > BOX_NAME_MAX) {
@@ -3182,6 +3152,31 @@ func_def_new_from_tuple(struct tuple *tuple)
 	}
 	if (identifier_check(name, name_len) != 0)
 		return NULL;
+	enum func_language language = FUNC_LANGUAGE_LUA;
+	if (field_count > BOX_FUNC_FIELD_LANGUAGE) {
+		const char *language_str =
+			tuple_field_cstr(tuple, BOX_FUNC_FIELD_LANGUAGE);
+		if (language_str == NULL)
+			return NULL;
+		language = STR2ENUM(func_language, language_str);
+		/*
+		 * 'SQL_BUILTIN' was dropped in 2.9, but to support upgrade
+		 * from previous versions, we allow to create such functions
+		 * if the data dictionary version is older.
+		 */
+		if (language == func_language_MAX ||
+		    language == FUNC_LANGUAGE_SQL ||
+		    (language == FUNC_LANGUAGE_SQL_BUILTIN &&
+		     dd_version_id >= version_id(2, 9, 0))) {
+			diag_set(ClientError, ER_FUNCTION_LANGUAGE,
+				 language_str, tt_cstr(name, name_len));
+			return NULL;
+		}
+	}
+	uint32_t body_len = 0;
+	const char *body = NULL;
+	uint32_t comment_len = 0;
+	const char *comment = NULL;
 	if (field_count > BOX_FUNC_FIELD_BODY) {
 		body = tuple_field_str(tuple, BOX_FUNC_FIELD_BODY, &body_len);
 		if (body == NULL)
@@ -3220,77 +3215,24 @@ func_def_new_from_tuple(struct tuple *tuple)
 				  "unsupported is_null_call value");
 			return NULL;
 		}
-	} else {
-		body = NULL;
-		body_len = 0;
-		comment = NULL;
-		comment_len = 0;
-	}
-	uint32_t body_offset, comment_offset;
-	uint32_t def_sz = func_def_sizeof(name_len, body_len, comment_len,
-					  &body_offset, &comment_offset);
-	struct func_def *def = (struct func_def *) malloc(def_sz);
-	if (def == NULL) {
-		diag_set(OutOfMemory, def_sz, "malloc", "def");
-		return NULL;
 	}
-	auto def_guard = make_scoped_guard([=] { free(def); });
-	if (func_def_get_ids_from_tuple(tuple, &def->fid, &def->uid) != 0)
+	uint32_t fid, uid;
+	if (func_def_get_ids_from_tuple(tuple, &fid, &uid) != 0)
 		return NULL;
-	if (def->fid > BOX_FUNCTION_MAX) {
+	if (fid > BOX_FUNCTION_MAX) {
 		diag_set(ClientError, ER_CREATE_FUNCTION,
 			  tt_cstr(name, name_len), "function id is too big");
 		return NULL;
 	}
-	func_opts_create(&def->opts);
-	memcpy(def->name, name, name_len);
-	def->name[name_len] = '\0';
-	def->name_len = name_len;
-	if (body_len > 0) {
-		def->body = (char *)def + body_offset;
-		memcpy(def->body, body, body_len);
-		def->body[body_len] = '\0';
-	} else {
-		def->body = NULL;
-	}
-	if (comment_len > 0) {
-		def->comment = (char *)def + comment_offset;
-		memcpy(def->comment, comment, comment_len);
-		def->comment[comment_len] = '\0';
-	} else {
-		def->comment = NULL;
-	}
+	struct func_def *def = func_def_new(fid, uid, name, name_len,
+					    language, body, body_len,
+					    comment, comment_len);
+	auto def_guard = make_scoped_guard([=] { func_def_delete(def); });
 	if (field_count > BOX_FUNC_FIELD_SETUID) {
 		uint32_t out;
 		if (tuple_field_u32(tuple, BOX_FUNC_FIELD_SETUID, &out) != 0)
 			return NULL;
 		def->setuid = out;
-	} else {
-		def->setuid = false;
-	}
-	if (field_count > BOX_FUNC_FIELD_LANGUAGE) {
-		const char *language =
-			tuple_field_cstr(tuple, BOX_FUNC_FIELD_LANGUAGE);
-		if (language == NULL)
-			return NULL;
-		def->language = STR2ENUM(func_language, language);
-		if (def->language == func_language_MAX ||
-		    def->language == FUNC_LANGUAGE_SQL) {
-			diag_set(ClientError, ER_FUNCTION_LANGUAGE,
-				  language, def->name);
-			return NULL;
-		}
-		if (def->language == FUNC_LANGUAGE_SQL_BUILTIN) {
-			if (func_def_create_sql_built_in(def) != 0)
-				return NULL;
-			if (func_def_check(def) != 0)
-				return NULL;
-			def_guard.is_active = false;
-			return def;
-		}
-	} else {
-		/* Lua is the default. */
-		def->language = FUNC_LANGUAGE_LUA;
 	}
 	if (field_count > BOX_FUNC_FIELD_BODY) {
 		if (tuple_field_bool(tuple, BOX_FUNC_FIELD_IS_DETERMINISTIC,
@@ -3389,14 +3331,8 @@ func_def_new_from_tuple(struct tuple *tuple)
 				ER_WRONG_SPACE_OPTIONS, NULL) != 0)
 			return NULL;
 	} else {
-		def->is_deterministic = false;
-		def->is_sandboxed = false;
-		def->returns = FIELD_TYPE_ANY;
-		def->aggregate = FUNC_AGGREGATE_NONE;
-		def->exports.all = 0;
 		/* By default export to Lua, but not other frontends. */
 		def->exports.lua = true;
-		def->param_count = 0;
 	}
 	if (func_def_check(def) != 0)
 		return NULL;
@@ -3457,7 +3393,9 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
 		struct func_def *def = func_def_new_from_tuple(new_tuple);
 		if (def == NULL)
 			return -1;
-		auto def_guard = make_scoped_guard([=] { free(def); });
+		auto def_guard = make_scoped_guard([=] {
+			func_def_delete(def);
+		});
 		if (access_check_ddl(def->name, def->fid, def->uid, SC_FUNCTION,
 				 PRIV_C) != 0)
 			return -1;
diff --git a/src/box/func.c b/src/box/func.c
index b80ddb4710..432df28623 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -380,6 +380,34 @@ schema_module_reload(const char *package, const char *package_end)
 	return -1;
 }
 
+static int
+func_dummy_call(struct func *func, struct port *args, struct port *ret)
+{
+	(void)func;
+	(void)args;
+	(void)ret;
+	diag_set(ClientError, ER_UNSUPPORTED, func->def->name, "call");
+	return -1;
+}
+
+static void
+func_dummy_destroy(struct func *func)
+{
+	free(func);
+}
+
+static struct func *
+func_dummy_new(void)
+{
+	static const struct func_vtab func_dummy_vtab = {
+		.call = func_dummy_call,
+		.destroy = func_dummy_destroy,
+	};
+	struct func *func = xmalloc(sizeof(*func));
+	func->vtab = &func_dummy_vtab;
+	return func;
+}
+
 static struct func *
 func_c_new(const struct func_def *def);
 
@@ -397,6 +425,14 @@ func_new(struct func_def *def)
 	case FUNC_LANGUAGE_SQL_EXPR:
 		func = func_sql_expr_new(def);
 		break;
+	case FUNC_LANGUAGE_SQL_BUILTIN:
+		/*
+		 * 'SQL_BUILTIN' was dropped in 2.9, but we still need to
+		 * handle creation of such functions to support upgrade from
+		 * previous versions.
+		 */
+		func = func_dummy_new();
+		break;
 	default:
 		unreachable();
 	}
diff --git a/src/box/func_def.c b/src/box/func_def.c
index b2a41a4d2b..974064a501 100644
--- a/src/box/func_def.c
+++ b/src/box/func_def.c
@@ -33,6 +33,13 @@
 #include "string.h"
 #include "diag.h"
 #include "error.h"
+#include "salad/grp_alloc.h"
+#include "trivia/util.h"
+
+#include <assert.h>
+#include <stdint.h>
+#include <stddef.h>
+#include <stdlib.h>
 
 const char *func_language_strs[] = {
 	"LUA", "C", "SQL", "SQL_BUILTIN", "SQL_EXPR"
@@ -50,6 +57,48 @@ const struct opt_def func_opts_reg[] = {
 	OPT_DEF("takes_raw_args", OPT_BOOL, struct func_opts, takes_raw_args),
 };
 
+struct func_def *
+func_def_new(uint32_t fid, uint32_t uid, const char *name, uint32_t name_len,
+	     enum func_language language, const char *body, uint32_t body_len,
+	     const char *comment, uint32_t comment_len)
+{
+	struct func_def *def;
+	struct grp_alloc all = grp_alloc_initializer();
+	grp_alloc_reserve_data(&all, sizeof(*def));
+	grp_alloc_reserve_str(&all, name_len);
+	if (body_len != 0)
+		grp_alloc_reserve_str(&all, body_len);
+	if (comment_len != 0)
+		grp_alloc_reserve_str(&all, comment_len);
+	grp_alloc_use(&all, xmalloc(grp_alloc_size(&all)));
+	def = grp_alloc_create_data(&all, sizeof(*def));
+	def->name = grp_alloc_create_str(&all, name, name_len);
+	def->name_len = name_len;
+	def->body = body_len == 0 ? NULL :
+		    grp_alloc_create_str(&all, body, body_len);
+	def->comment = comment_len == 0 ? NULL :
+		       grp_alloc_create_str(&all, comment, comment_len);
+	assert(grp_alloc_size(&all) == 0);
+	def->fid = fid;
+	def->uid = uid;
+	def->setuid = false;
+	def->is_deterministic = false;
+	def->is_sandboxed = false;
+	def->param_count = 0;
+	def->returns = FIELD_TYPE_ANY;
+	def->aggregate = FUNC_AGGREGATE_NONE;
+	def->language = language;
+	def->exports.all = 0;
+	func_opts_create(&def->opts);
+	return def;
+}
+
+void
+func_def_delete(struct func_def *def)
+{
+	free(def);
+}
+
 static int
 func_opts_cmp(const struct func_opts *o1, const struct func_opts *o2)
 {
diff --git a/src/box/func_def.h b/src/box/func_def.h
index de1d2672cd..14edb432b4 100644
--- a/src/box/func_def.h
+++ b/src/box/func_def.h
@@ -34,7 +34,9 @@
 #include "trivia/util.h"
 #include "field_def.h"
 #include "opt_def.h"
+
 #include <stdbool.h>
+#include <stdint.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -94,6 +96,10 @@ struct func_def {
 	uint32_t fid;
 	/** Owner of the function. */
 	uint32_t uid;
+	/** Function name. */
+	const char *name;
+	/** The length of the function name. */
+	uint32_t name_len;
 	/** Definition of the persistent function. */
 	char *body;
 	/** User-defined comment for a function. */
@@ -124,8 +130,6 @@ struct func_def {
 	 * The language of the stored function.
 	 */
 	enum func_language language;
-	/** The length of the function name. */
-	uint32_t name_len;
 	/** Frontends where function must be available. */
 	union {
 		struct {
@@ -136,30 +140,21 @@ struct func_def {
 	} exports;
 	/** The function options. */
 	struct func_opts opts;
-	/** Function name. */
-	char name[0];
 };
 
 /**
- * @param name_len length of func_def->name
- * @returns size in bytes needed to allocate for struct func_def
- * for a function of length @a a name_len, body @a body_len and
- * with comment @a comment_len.
+ * Allocates and initializes a function definition.
+ * Fields unspecified in the arguments are set to their default values.
+ * This function never fails (never returns NULL).
  */
-static inline size_t
-func_def_sizeof(uint32_t name_len, uint32_t body_len, uint32_t comment_len,
-		uint32_t *body_offset, uint32_t *comment_offset)
-{
-	/* +1 for '\0' name terminating. */
-	size_t sz = sizeof(struct func_def) + name_len + 1;
-	*body_offset = sz;
-	if (body_len > 0)
-		sz += body_len + 1;
-	*comment_offset = sz;
-	if (comment_len > 0)
-		sz += comment_len + 1;
-	return sz;
-}
+struct func_def *
+func_def_new(uint32_t fid, uint32_t uid, const char *name, uint32_t name_len,
+	     enum func_language language, const char *body, uint32_t body_len,
+	     const char *comment, uint32_t comment_len);
+
+/** Frees a function definition object. */
+void
+func_def_delete(struct func_def *def);
 
 /** Compare two given function definitions. */
 int
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 6970a1912e..f580cb7640 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -2256,32 +2256,19 @@ sql_built_in_functions_cache_init(void)
 		assert(dict != NULL);
 
 		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;
+		struct func_def *def = func_def_new(i, ADMIN, name, len,
+						    FUNC_LANGUAGE_SQL_BUILTIN,
+						    NULL, 0, NULL, 0);
 		def->setuid = true;
 		def->is_deterministic = dict->is_deterministic;
-		def->is_sandboxed = false;
 		assert(desc->argc != -1 || dict->argc_min != dict->argc_max);
 		def->param_count = desc->argc;
 		def->returns = desc->result;
 		def->aggregate = (dict->flags & SQL_FUNC_AGG) == 0 ?
 				 FUNC_AGGREGATE_NONE : FUNC_AGGREGATE_GROUP;
-		def->language = FUNC_LANGUAGE_SQL_BUILTIN;
-		def->name_len = len;
 		def->exports.sql = true;
-		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");
 
+		struct func_sql_builtin *func = xmalloc(sizeof(*func));
 		func->base.def = def;
 		rlist_create(&func->base.func_cache_pin_list);
 		func->base.vtab = &func_sql_builtin_vtab;
diff --git a/test/unit/func_cache.c b/test/unit/func_cache.c
index 3b9892cf18..f6d3fabc45 100644
--- a/test/unit/func_cache.c
+++ b/test/unit/func_cache.c
@@ -9,11 +9,9 @@ struct func *
 test_func_new(uint32_t id, const char *name)
 {
 	uint32_t name_len = strlen(name);
-	struct func_def *def = xmalloc(offsetof(struct func_def,
-						name[name_len + 1]));
-	def->fid = id;
-	strcpy(def->name, name);
-	def->name_len = strlen(name);
+	struct func_def *def = func_def_new(id, ADMIN, name, name_len,
+					    FUNC_LANGUAGE_LUA,
+					    NULL, 0, NULL, 0);
 	struct func *f = xmalloc(sizeof(*f));
 	f->def = def;
 	rlist_create(&f->func_cache_pin_list);
@@ -23,7 +21,7 @@ test_func_new(uint32_t id, const char *name)
 static void
 test_func_delete(struct func *f)
 {
-	free(f->def);
+	func_def_delete(f->def);
 	free(f);
 }
 
-- 
GitLab