Re: [Tarantool-patches] [PATCH 1/4] box: replace session_settings modules with a single array

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the fixes!

Now I noticed that struct session_setting_metadata is not
really needed. Mask field is never used. I reverted it back
to build.c, and added field type directly to struct
session_setting.

================================================================================

commit 5b80dce870f045417e63458f6672a82ed5e29bfb
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date:   Thu Feb 6 22:06:14 2020 +0100

    Review fixes

diff --git a/src/box/session_settings.h b/src/box/session_settings.h
index 1f18f147b..de24e3c6f 100644
--- a/src/box/session_settings.h
+++ b/src/box/session_settings.h
@@ -62,20 +62,12 @@ enum {
 	SESSION_SETTING_COUNT = SESSION_SETTING_SQL_END,
 };
 
-/**
- * A structure that allows to establish a connection between
- * parameter and its field type and mask, if it has one.
-*/
-struct session_setting_metadata {
-	/** Setting's value type. */
-	enum field_type field_type;
-	/** Stores setting's bit flags. */
-	uint32_t mask;
-};
-
 struct session_setting {
-	/** Setting's value type and the corresponding mask. */
-	struct session_setting_metadata metadata;
+	/**
+	 * Setting's value type. Used for error checking and
+	 * reporting only.
+	 */
+	enum field_type field_type;
 	/**
 	 * Get a MessagePack encoded pair [name, value] for a
 	 * setting having index @a id. Index is from the settings
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 3da60df86..e5fde08cc 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -3310,13 +3310,23 @@ sql_fieldno_by_name(struct Parse *parse_context, struct Expr *field_name,
 	return 0;
 }
 
+/**
+ * A local structure that allows to establish a connection between
+ * parameter and its field type and mask, if it has one.
+ */
+struct sql_option_metadata
+{
+	uint32_t field_type;
+	uint32_t mask;
+};
+
 /**
  * Variable that contains SQL session option field types and masks
  * if they have one or 0 if don't have.
  *
  * It is IMPORTANT that these options sorted by name.
  */
-static struct session_setting_metadata sql_session_opts[] = {
+static struct sql_option_metadata sql_session_opts[] = {
 	/** SESSION_SETTING_SQL_DEFAULT_ENGINE */
 	{FIELD_TYPE_STRING, 0},
 	/** SESSION_SETTING_SQL_DEFER_FOREIGN_KEYS */
@@ -3345,7 +3355,7 @@ sql_session_setting_get(int id, const char **mp_pair, const char **mp_pair_end)
 	assert(id >= SESSION_SETTING_SQL_BEGIN && id < SESSION_SETTING_SQL_END);
 	struct session *session = current_session();
 	uint32_t flags = session->sql_flags;
-	struct session_setting_metadata *opt = &sql_session_opts[id];
+	struct sql_option_metadata *opt = &sql_session_opts[id];
 	uint32_t mask = opt->mask;
 	const char *name = session_setting_strs[id];
 	size_t name_len = strlen(name);
@@ -3382,7 +3392,7 @@ static int
 sql_set_boolean_option(int id, bool value)
 {
 	struct session *session = current_session();
-	struct session_setting_metadata *option = &sql_session_opts[id];
+	struct sql_option_metadata *option = &sql_session_opts[id];
 	assert(option->field_type == FIELD_TYPE_BOOLEAN);
 #ifdef NDEBUG
 	if ((session->sql_flags & SQL_SqlTrace) == 0) {
@@ -3454,7 +3464,7 @@ sql_session_settings_init()
 	for (int id = SESSION_SETTING_SQL_BEGIN; id < SESSION_SETTING_SQL_END;
 	     ++id) {
 		struct session_setting *setting = &session_settings[id];
-		setting->metadata = sql_session_opts[id];
+		setting->field_type = sql_session_opts[id].field_type;
 		setting->get = sql_session_setting_get;
 		setting->set = sql_session_setting_set;
 	}