Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH v2 4/7] box: bove some decode functions from alter.cc
       [not found] <cover.1531903378.git.imeevma@gmail.com>
@ 2018-07-18  8:43 ` imeevma
  2018-07-18  9:24   ` [tarantool-patches] " Vladislav Shpilevoy
  0 siblings, 1 reply; 2+ messages in thread
From: imeevma @ 2018-07-18  8:43 UTC (permalink / raw)
  To: tarantool-patches

Move functions to decode space format and
functions to decode index options from
alter.cc to format_def.c and index_def.c.

Part of #3375.
---
Branch: https://github.com/tarantool/tarantool/compare/imeevma/gh-3375-lua-expose-ephemeral-spaces
Issue: https://github.com/tarantool/tarantool/issues/3375

 src/box/alter.cc    | 215 ++--------------------------------------------------
 src/box/index_def.c |  60 +++++++++++++++
 src/box/index_def.h |   8 ++
 src/box/space_def.c | 155 +++++++++++++++++++++++++++++++++++++
 src/box/space_def.h |  18 +++++
 5 files changed, 246 insertions(+), 210 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 7b6bd1a..47729d7 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -30,27 +30,18 @@
  */
 #include "alter.h"
 #include "schema.h"
-#include "user.h"
-#include "space.h"
-#include "index.h"
 #include "func.h"
 #include "coll_id_cache.h"
 #include "coll_id_def.h"
 #include "txn.h"
 #include "tuple.h"
-#include "fiber.h" /* for gc_pool */
 #include "scoped_guard.h"
 #include "third_party/base64.h"
-#include <new> /* for placement new */
-#include <stdio.h> /* snprintf() */
-#include <ctype.h>
 #include "replication.h" /* for replica_set_id() */
 #include "session.h" /* to fetch the current user. */
-#include "vclock.h" /* VCLOCK_MAX */
 #include "xrow.h"
 #include "iproto_constants.h"
 #include "identifier.h"
-#include "version.h"
 #include "sequence.h"
 #include "sql.h"
 
@@ -167,61 +158,6 @@ err:
 }
 
 /**
- * Fill index_opts structure from opts field in tuple of space _index
- * Throw an error is unrecognized option.
- */
-static void
-index_opts_decode(struct index_opts *opts, const char *map,
-		  struct region *region)
-{
-	index_opts_create(opts);
-	if (opts_decode(opts, index_opts_reg, &map, ER_WRONG_INDEX_OPTIONS,
-			BOX_INDEX_FIELD_OPTS, region) != 0)
-		diag_raise();
-	if (opts->distance == rtree_index_distance_type_MAX) {
-		tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
-			  BOX_INDEX_FIELD_OPTS, "distance must be either "\
-			  "'euclid' or 'manhattan'");
-	}
-	if (opts->sql != NULL) {
-		char *sql = strdup(opts->sql);
-		if (sql == NULL) {
-			opts->sql = NULL;
-			tnt_raise(OutOfMemory, strlen(opts->sql) + 1, "strdup",
-				  "sql");
-		}
-		opts->sql = sql;
-	}
-	if (opts->range_size <= 0) {
-		tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
-			  BOX_INDEX_FIELD_OPTS,
-			  "range_size must be greater than 0");
-	}
-	if (opts->page_size <= 0 || opts->page_size > opts->range_size) {
-		tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
-			  BOX_INDEX_FIELD_OPTS,
-			  "page_size must be greater than 0 and "
-			  "less than or equal to range_size");
-	}
-	if (opts->run_count_per_level <= 0) {
-		tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
-			  BOX_INDEX_FIELD_OPTS,
-			  "run_count_per_level must be greater than 0");
-	}
-	if (opts->run_size_ratio <= 1) {
-		tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
-			  BOX_INDEX_FIELD_OPTS,
-			  "run_size_ratio must be greater than 1");
-	}
-	if (opts->bloom_fpr <= 0 || opts->bloom_fpr > 1) {
-		tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
-			  BOX_INDEX_FIELD_OPTS,
-			  "bloom_fpr must be greater than 0 and "
-			  "less than or equal to 1");
-	}
-}
-
-/**
  * Create a index_def object from a record in _index
  * system space.
  *
@@ -250,7 +186,8 @@ index_def_new_from_tuple(struct tuple *tuple, struct space *space)
 	const char *opts_field =
 		tuple_field_with_type_xc(tuple, BOX_INDEX_FIELD_OPTS,
 					 MP_MAP);
-	index_opts_decode(&opts, opts_field, &fiber()->gc);
+	if (index_opts_decode(&opts, opts_field, &fiber()->gc) != 0)
+		diag_raise();
 	const char *parts = tuple_field(tuple, BOX_INDEX_FIELD_PARTS);
 	uint32_t part_count = mp_decode_array(&parts);
 	if (name_len > BOX_NAME_MAX) {
@@ -316,149 +253,6 @@ space_opts_decode(struct space_opts *opts, const char *map,
 }
 
 /**
- * Decode field definition from MessagePack map. Format:
- * {name: <string>, type: <string>}. Type is optional.
- * @param[out] field Field to decode to.
- * @param data MessagePack map to decode.
- * @param space_name Name of a space, from which the field is got.
- *        Used in error messages.
- * @param name_len Length of @a space_name.
- * @param errcode Error code to use for client errors. Either
- *        create or modify space errors.
- * @param fieldno Field number to decode. Used in error messages.
- * @param region Region to allocate field name.
- */
-static void
-field_def_decode(struct field_def *field, const char **data,
-		 const char *space_name, uint32_t name_len,
-		 uint32_t errcode, uint32_t fieldno, struct region *region)
-{
-	if (mp_typeof(**data) != MP_MAP) {
-		tnt_raise(ClientError, errcode, tt_cstr(space_name, name_len),
-			  tt_sprintf("field %d is not map",
-				     fieldno + TUPLE_INDEX_BASE));
-	}
-	int count = mp_decode_map(data);
-	*field = field_def_default;
-	bool is_action_missing = true;
-	uint32_t action_literal_len = strlen("nullable_action");
-	for (int i = 0; i < count; ++i) {
-		if (mp_typeof(**data) != MP_STR) {
-			tnt_raise(ClientError, errcode,
-				  tt_cstr(space_name, name_len),
-				  tt_sprintf("field %d format is not map"\
-					     " with string keys",
-					     fieldno + TUPLE_INDEX_BASE));
-		}
-		uint32_t key_len;
-		const char *key = mp_decode_str(data, &key_len);
-		if (opts_parse_key(field, field_def_reg, key, key_len, data,
-				   ER_WRONG_SPACE_FORMAT,
-				   fieldno + TUPLE_INDEX_BASE, region,
-				   true) != 0)
-			diag_raise();
-		if (is_action_missing &&
-		    key_len == action_literal_len &&
-		    memcmp(key, "nullable_action", action_literal_len) == 0)
-			is_action_missing = false;
-	}
-	if (is_action_missing) {
-		field->nullable_action = field->is_nullable ?
-			ON_CONFLICT_ACTION_NONE
-			: ON_CONFLICT_ACTION_DEFAULT;
-	}
-	if (field->name == NULL) {
-		tnt_raise(ClientError, errcode, tt_cstr(space_name, name_len),
-			  tt_sprintf("field %d name is not specified",
-				     fieldno + TUPLE_INDEX_BASE));
-	}
-	size_t field_name_len = strlen(field->name);
-	if (field_name_len > BOX_NAME_MAX) {
-		tnt_raise(ClientError, errcode, tt_cstr(space_name, name_len),
-			  tt_sprintf("field %d name is too long",
-				     fieldno + TUPLE_INDEX_BASE));
-	}
-	identifier_check_xc(field->name, field_name_len);
-	if (field->type == field_type_MAX) {
-		tnt_raise(ClientError, errcode, tt_cstr(space_name, name_len),
-			  tt_sprintf("field %d has unknown field type",
-				     fieldno + TUPLE_INDEX_BASE));
-	}
-	if (field->nullable_action == on_conflict_action_MAX) {
-		tnt_raise(ClientError, errcode, tt_cstr(space_name, name_len),
-			  tt_sprintf("field %d has unknown field on conflict "
-				     "nullable action",
-				     fieldno + TUPLE_INDEX_BASE));
-	}
-	if (!((field->is_nullable && field->nullable_action ==
-	       ON_CONFLICT_ACTION_NONE)
-	      || (!field->is_nullable
-		  && field->nullable_action != ON_CONFLICT_ACTION_NONE))) {
-		tnt_raise(ClientError, errcode, tt_cstr(space_name, name_len),
-			  tt_sprintf("field %d has conflicting nullability and "
-				     "nullable action properties", fieldno +
-				     TUPLE_INDEX_BASE));
-	}
-	if (field->coll_id != COLL_NONE &&
-	    field->type != FIELD_TYPE_STRING &&
-	    field->type != FIELD_TYPE_SCALAR &&
-	    field->type != FIELD_TYPE_ANY) {
-		tnt_raise(ClientError, errcode, tt_cstr(space_name, name_len),
-			  tt_sprintf("collation is reasonable only for "
-				     "string, scalar and any fields"));
-	}
-
-	const char *dv = field->default_value;
-	if (dv != NULL) {
-		field->default_value_expr = sql_expr_compile(sql_get(), dv,
-							     strlen(dv));
-		if (field->default_value_expr == NULL)
-			diag_raise();
-	}
-}
-
-/**
- * Decode MessagePack array of fields.
- * @param data MessagePack array of fields.
- * @param[out] out_count Length of a result array.
- * @param space_name Space name to use in error messages.
- * @param errcode Errcode for client errors.
- * @param region Region to allocate result array.
- *
- * @retval Array of fields.
- */
-static struct field_def *
-space_format_decode(const char *data, uint32_t *out_count,
-		    const char *space_name, uint32_t name_len,
-		    uint32_t errcode, struct region *region)
-{
-	/* Type is checked by _space format. */
-	assert(mp_typeof(*data) == MP_ARRAY);
-	uint32_t count = mp_decode_array(&data);
-	*out_count = count;
-	if (count == 0)
-		return NULL;
-	size_t size = count * sizeof(struct field_def);
-	struct field_def *region_defs =
-		(struct field_def *) region_alloc_xc(region, size);
-	/*
-	 * Nullify to prevent a case when decoding will fail in
-	 * the middle and space_def_destroy_fields() below will
-	 * work with garbage pointers.
-	 */
-	memset(region_defs, 0, size);
-	auto fields_guard = make_scoped_guard([=] {
-		space_def_destroy_fields(region_defs, count);
-	});
-	for (uint32_t i = 0; i < count; ++i) {
-		field_def_decode(&region_defs[i], &data, space_name, name_len,
-				 errcode, i, region);
-	}
-	fields_guard.is_active = false;
-	return region_defs;
-}
-
-/**
  * Fill space_def structure from struct tuple.
  */
 static struct space_def *
@@ -508,8 +302,9 @@ space_def_new_from_tuple(struct tuple *tuple, uint32_t errcode,
 	const char *format =
 		tuple_field_with_type_xc(tuple, BOX_SPACE_FIELD_FORMAT,
 					 MP_ARRAY);
-	fields = space_format_decode(format, &field_count, name,
-				     name_len, errcode, region);
+	if (space_format_decode(&fields, format, &field_count, name, name_len,
+				errcode, region) != 0)
+		diag_raise();
 	auto fields_guard = make_scoped_guard([=] {
 		space_def_destroy_fields(fields, field_count);
 	});
diff --git a/src/box/index_def.c b/src/box/index_def.c
index 45c74d9..e0d54c0 100644
--- a/src/box/index_def.c
+++ b/src/box/index_def.c
@@ -309,3 +309,63 @@ index_def_is_valid(struct index_def *index_def, const char *space_name)
 	}
 	return true;
 }
+
+int
+index_opts_decode(struct index_opts *opts, const char *map,
+		  struct region *region)
+{
+	index_opts_create(opts);
+	if (opts_decode(opts, index_opts_reg, &map, ER_WRONG_INDEX_OPTIONS,
+			BOX_INDEX_FIELD_OPTS, region) != 0) {
+		return -1;
+	}
+	if (opts->distance == rtree_index_distance_type_MAX) {
+		diag_set(ClientError, ER_WRONG_INDEX_OPTIONS,
+			  BOX_INDEX_FIELD_OPTS, "distance must be either "\
+			  "'euclid' or 'manhattan'");
+		return -1;
+	}
+	if (opts->sql != NULL) {
+		char *sql = strdup(opts->sql);
+		if (sql == NULL) {
+			opts->sql = NULL;
+			diag_set(OutOfMemory, strlen(opts->sql) + 1, "strdup",
+				 "sql");
+			return -1;
+		}
+		opts->sql = sql;
+	}
+	if (opts->range_size <= 0) {
+		diag_set(ClientError, ER_WRONG_INDEX_OPTIONS,
+			 BOX_INDEX_FIELD_OPTS,
+			 "range_size must be greater than 0");
+		return -1;
+	}
+	if (opts->page_size <= 0 || opts->page_size > opts->range_size) {
+		diag_set(ClientError, ER_WRONG_INDEX_OPTIONS,
+			 BOX_INDEX_FIELD_OPTS,
+			 "page_size must be greater than 0 and "
+			 "less than or equal to range_size");
+		return -1;
+	}
+	if (opts->run_count_per_level <= 0) {
+		diag_set(ClientError, ER_WRONG_INDEX_OPTIONS,
+			 BOX_INDEX_FIELD_OPTS,
+			 "run_count_per_level must be greater than 0");
+		return -1;
+	}
+	if (opts->run_size_ratio <= 1) {
+		diag_set(ClientError, ER_WRONG_INDEX_OPTIONS,
+			 BOX_INDEX_FIELD_OPTS,
+			 "run_size_ratio must be greater than 1");
+		return -1;
+	}
+	if (opts->bloom_fpr <= 0 || opts->bloom_fpr > 1) {
+		diag_set(ClientError, ER_WRONG_INDEX_OPTIONS,
+			 BOX_INDEX_FIELD_OPTS,
+			 "bloom_fpr must be greater than 0 and "
+			 "less than or equal to 1");
+		return -1;
+	}
+	return 0;
+}
diff --git a/src/box/index_def.h b/src/box/index_def.h
index 48a7820..4c3625b 100644
--- a/src/box/index_def.h
+++ b/src/box/index_def.h
@@ -358,6 +358,14 @@ index_def_cmp(const struct index_def *key1, const struct index_def *key2);
 bool
 index_def_is_valid(struct index_def *index_def, const char *space_name);
 
+/**
+ * Fill index_opts structure from opts field in tuple of space _index
+ * Throw an error is unrecognized option.
+ */
+int
+index_opts_decode(struct index_opts *opts, const char *map,
+		  struct region *region);
+
 #if defined(__cplusplus)
 } /* extern "C" */
 
diff --git a/src/box/space_def.c b/src/box/space_def.c
index f5ca0b5..8780eec 100644
--- a/src/box/space_def.c
+++ b/src/box/space_def.c
@@ -34,6 +34,10 @@
 #include "error.h"
 #include "sql.h"
 #include "msgpuck.h"
+#include "tuple_format.h"
+#include "schema_def.h"
+#include "identifier.h"
+#include "small/region.h"
 
 /**
  * Make checks from msgpack.
@@ -351,3 +355,154 @@ error:
 	sql_expr_list_delete(db, checks);
 	return  -1;
 }
+
+/**
+ * Decode field definition from MessagePack map. Format:
+ * {name: <string>, type: <string>}. Type is optional.
+ * @param[out] field Field to decode to.
+ * @param data MessagePack map to decode.
+ * @param space_name Name of a space, from which the field is got.
+ *        Used in error messages.
+ * @param name_len Length of @a space_name.
+ * @param errcode Error code to use for client errors. Either
+ *        create or modify space errors.
+ * @param fieldno Field number to decode. Used in error messages.
+ * @param region Region to allocate field name.
+ *
+ * @retval 0 Success.
+ * @retval -1 Error.
+ */
+static inline int
+field_def_decode(struct field_def *field, const char **data,
+		 const char *space_name, uint32_t name_len,
+		 uint32_t errcode, uint32_t fieldno, struct region *region)
+{
+	if (mp_typeof(**data) != MP_MAP) {
+		diag_set(ClientError, errcode, tt_cstr(space_name, name_len),
+			  tt_sprintf("field %d is not map",
+				     fieldno + TUPLE_INDEX_BASE));
+		return -1;
+	}
+	int count = mp_decode_map(data);
+	*field = field_def_default;
+	bool is_action_missing = true;
+	uint32_t action_literal_len = strlen("nullable_action");
+	for (int i = 0; i < count; ++i) {
+		if (mp_typeof(**data) != MP_STR) {
+			diag_set(ClientError, errcode,
+				  tt_cstr(space_name, name_len),
+				  tt_sprintf("field %d format is not map"\
+					     " with string keys",
+					     fieldno + TUPLE_INDEX_BASE));
+			return -1;
+		}
+		uint32_t key_len;
+		const char *key = mp_decode_str(data, &key_len);
+		if (opts_parse_key(field, field_def_reg, key, key_len, data,
+				   ER_WRONG_SPACE_FORMAT,
+				   fieldno + TUPLE_INDEX_BASE, region,
+				   true) != 0)
+			return -1;
+		if (is_action_missing &&
+		    key_len == action_literal_len &&
+		    memcmp(key, "nullable_action", action_literal_len) == 0)
+			is_action_missing = false;
+	}
+	if (is_action_missing) {
+		field->nullable_action = field->is_nullable ?
+			ON_CONFLICT_ACTION_NONE
+			: ON_CONFLICT_ACTION_DEFAULT;
+	}
+	if (field->name == NULL) {
+		diag_set(ClientError, errcode, tt_cstr(space_name, name_len),
+			  tt_sprintf("field %d name is not specified",
+				     fieldno + TUPLE_INDEX_BASE));
+		return -1;
+	}
+	size_t field_name_len = strlen(field->name);
+	if (field_name_len > BOX_NAME_MAX) {
+		diag_set(ClientError, errcode, tt_cstr(space_name, name_len),
+			  tt_sprintf("field %d name is too long",
+				     fieldno + TUPLE_INDEX_BASE));
+		return -1;
+	}
+	if (identifier_check(field->name, field_name_len))
+		return -1;
+	if (field->type == field_type_MAX) {
+		diag_set(ClientError, errcode, tt_cstr(space_name, name_len),
+			  tt_sprintf("field %d has unknown field type",
+				     fieldno + TUPLE_INDEX_BASE));
+		return -1;
+	}
+	if (field->nullable_action == on_conflict_action_MAX) {
+		diag_set(ClientError, errcode, tt_cstr(space_name, name_len),
+			  tt_sprintf("field %d has unknown field on conflict "
+				     "nullable action",
+				     fieldno + TUPLE_INDEX_BASE));
+		return -1;
+	}
+	if (!((field->is_nullable && field->nullable_action ==
+	       ON_CONFLICT_ACTION_NONE)
+	      || (!field->is_nullable
+		  && field->nullable_action != ON_CONFLICT_ACTION_NONE))) {
+		diag_set(ClientError, errcode, tt_cstr(space_name, name_len),
+			  tt_sprintf("field %d has conflicting nullability and "
+				     "nullable action properties", fieldno +
+				     TUPLE_INDEX_BASE));
+		return -1;
+	}
+	if (field->coll_id != COLL_NONE &&
+	    field->type != FIELD_TYPE_STRING &&
+	    field->type != FIELD_TYPE_SCALAR &&
+	    field->type != FIELD_TYPE_ANY) {
+		diag_set(ClientError, errcode, tt_cstr(space_name, name_len),
+			  tt_sprintf("collation is reasonable only for "
+				     "string, scalar and any fields"));
+		return -1;
+	}
+
+	const char *dv = field->default_value;
+	if (dv != NULL) {
+		field->default_value_expr = sql_expr_compile(sql_get(), dv,
+							     strlen(dv));
+		if (field->default_value_expr == NULL)
+			return -1;
+	}
+	return 0;
+}
+
+int
+space_format_decode(struct field_def **field, const char *data,
+		    uint32_t *out_count, const char *space_name,
+		    uint32_t name_len, uint32_t errcode, struct region *region)
+{
+	/* Type is checked by _space format. */
+	assert(mp_typeof(*data) == MP_ARRAY);
+	*field = NULL;
+	uint32_t count = mp_decode_array(&data);
+	*out_count = count;
+	if (count == 0)
+		return 0;
+	size_t size = count * sizeof(struct field_def);
+	struct field_def *region_defs =
+		(struct field_def *) region_alloc(region, size);
+	if (region_defs == NULL) {
+		diag_set(OutOfMemory, size, "region_alloc", "region_defs");
+		return -1;
+	}
+	/*
+	 * Nullify to prevent a case when decoding will fail in
+	 * the middle and space_def_destroy_fields() below will
+	 * work with garbage pointers.
+	 */
+	memset(region_defs, 0, size);
+	for (uint32_t i = 0; i < count; ++i) {
+		if (field_def_decode(&region_defs[i], &data, space_name,
+				     name_len, errcode, i, region) != 0) {
+			space_def_destroy_fields(region_defs, count);
+			return -1;
+		}
+	}
+	*field = region_defs;
+	return 0;
+}
diff --git a/src/box/space_def.h b/src/box/space_def.h
index 0d1e902..7a01b92 100644
--- a/src/box/space_def.h
+++ b/src/box/space_def.h
@@ -182,6 +182,24 @@ space_def_sizeof(uint32_t name_len, const struct field_def *fields,
 		 uint32_t field_count, uint32_t *names_offset,
 		 uint32_t *fields_offset, uint32_t *def_expr_offset);
 
+/**
+ * Decode MessagePack array of fields.
+ * @param[out] field Field to decode to.
+ * @param data MessagePack array of fields.
+ * @param[out] out_count Length of a result array.
+ * @param space_name Space name to use in error messages.
+ * @param name_len Length of @a space_name.
+ * @param errcode Errcode for client errors.
+ * @param region Region to allocate result array.
+ *
+ * @retval 0 Success.
+ * @retval -1 Error.
+ */
+int
+space_format_decode(struct field_def **field, const char *data,
+		    uint32_t *out_count, const char *space_name,
+		    uint32_t name_len, uint32_t errcode, struct region *region);
+
 #if defined(__cplusplus)
 } /* extern "C" */
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [tarantool-patches] Re: [PATCH v2 4/7] box: bove some decode functions from alter.cc
  2018-07-18  8:43 ` [tarantool-patches] [PATCH v2 4/7] box: bove some decode functions from alter.cc imeevma
@ 2018-07-18  9:24   ` Vladislav Shpilevoy
  0 siblings, 0 replies; 2+ messages in thread
From: Vladislav Shpilevoy @ 2018-07-18  9:24 UTC (permalink / raw)
  To: tarantool-patches, imeevma, Kirill Shcherbatov

Kirill, please, do a second review.

Mergen, try to avoid creating new thread on each commit. Either
send responses using --in-reply-to, or send all the emails together
under v2 in a new thread.

On 18/07/2018 11:43, imeevma@tarantool.org wrote:
> Move functions to decode space format and
> functions to decode index options from
> alter.cc to format_def.c and index_def.c.
> 
> Part of #3375.
> ---
> Branch: https://github.com/tarantool/tarantool/compare/imeevma/gh-3375-lua-expose-ephemeral-spaces
> Issue: https://github.com/tarantool/tarantool/issues/3375
> 
>   src/box/alter.cc    | 215 ++--------------------------------------------------
>   src/box/index_def.c |  60 +++++++++++++++
>   src/box/index_def.h |   8 ++
>   src/box/space_def.c | 155 +++++++++++++++++++++++++++++++++++++
>   src/box/space_def.h |  18 +++++
>   5 files changed, 246 insertions(+), 210 deletions(-)
> 
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index 7b6bd1a..47729d7 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -30,27 +30,18 @@
>    */
>   #include "alter.h"
>   #include "schema.h"
> -#include "user.h"
> -#include "space.h"
> -#include "index.h"
>   #include "func.h"
>   #include "coll_id_cache.h"
>   #include "coll_id_def.h"
>   #include "txn.h"
>   #include "tuple.h"
> -#include "fiber.h" /* for gc_pool */
>   #include "scoped_guard.h"
>   #include "third_party/base64.h"
> -#include <new> /* for placement new */
> -#include <stdio.h> /* snprintf() */
> -#include <ctype.h>
>   #include "replication.h" /* for replica_set_id() */
>   #include "session.h" /* to fetch the current user. */
> -#include "vclock.h" /* VCLOCK_MAX */
>   #include "xrow.h"
>   #include "iproto_constants.h"
>   #include "identifier.h"
> -#include "version.h"
>   #include "sequence.h"
>   #include "sql.h"
>   
> @@ -167,61 +158,6 @@ err:
>   }
>   
>   /**
> - * Fill index_opts structure from opts field in tuple of space _index
> - * Throw an error is unrecognized option.
> - */
> -static void
> -index_opts_decode(struct index_opts *opts, const char *map,
> -		  struct region *region)
> -{
> -	index_opts_create(opts);
> -	if (opts_decode(opts, index_opts_reg, &map, ER_WRONG_INDEX_OPTIONS,
> -			BOX_INDEX_FIELD_OPTS, region) != 0)
> -		diag_raise();
> -	if (opts->distance == rtree_index_distance_type_MAX) {
> -		tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
> -			  BOX_INDEX_FIELD_OPTS, "distance must be either "\
> -			  "'euclid' or 'manhattan'");
> -	}
> -	if (opts->sql != NULL) {
> -		char *sql = strdup(opts->sql);
> -		if (sql == NULL) {
> -			opts->sql = NULL;
> -			tnt_raise(OutOfMemory, strlen(opts->sql) + 1, "strdup",
> -				  "sql");
> -		}
> -		opts->sql = sql;
> -	}
> -	if (opts->range_size <= 0) {
> -		tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
> -			  BOX_INDEX_FIELD_OPTS,
> -			  "range_size must be greater than 0");
> -	}
> -	if (opts->page_size <= 0 || opts->page_size > opts->range_size) {
> -		tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
> -			  BOX_INDEX_FIELD_OPTS,
> -			  "page_size must be greater than 0 and "
> -			  "less than or equal to range_size");
> -	}
> -	if (opts->run_count_per_level <= 0) {
> -		tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
> -			  BOX_INDEX_FIELD_OPTS,
> -			  "run_count_per_level must be greater than 0");
> -	}
> -	if (opts->run_size_ratio <= 1) {
> -		tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
> -			  BOX_INDEX_FIELD_OPTS,
> -			  "run_size_ratio must be greater than 1");
> -	}
> -	if (opts->bloom_fpr <= 0 || opts->bloom_fpr > 1) {
> -		tnt_raise(ClientError, ER_WRONG_INDEX_OPTIONS,
> -			  BOX_INDEX_FIELD_OPTS,
> -			  "bloom_fpr must be greater than 0 and "
> -			  "less than or equal to 1");
> -	}
> -}
> -
> -/**
>    * Create a index_def object from a record in _index
>    * system space.
>    *
> @@ -250,7 +186,8 @@ index_def_new_from_tuple(struct tuple *tuple, struct space *space)
>   	const char *opts_field =
>   		tuple_field_with_type_xc(tuple, BOX_INDEX_FIELD_OPTS,
>   					 MP_MAP);
> -	index_opts_decode(&opts, opts_field, &fiber()->gc);
> +	if (index_opts_decode(&opts, opts_field, &fiber()->gc) != 0)
> +		diag_raise();
>   	const char *parts = tuple_field(tuple, BOX_INDEX_FIELD_PARTS);
>   	uint32_t part_count = mp_decode_array(&parts);
>   	if (name_len > BOX_NAME_MAX) {
> @@ -316,149 +253,6 @@ space_opts_decode(struct space_opts *opts, const char *map,
>   }
>   
>   /**
> - * Decode field definition from MessagePack map. Format:
> - * {name: <string>, type: <string>}. Type is optional.
> - * @param[out] field Field to decode to.
> - * @param data MessagePack map to decode.
> - * @param space_name Name of a space, from which the field is got.
> - *        Used in error messages.
> - * @param name_len Length of @a space_name.
> - * @param errcode Error code to use for client errors. Either
> - *        create or modify space errors.
> - * @param fieldno Field number to decode. Used in error messages.
> - * @param region Region to allocate field name.
> - */
> -static void
> -field_def_decode(struct field_def *field, const char **data,
> -		 const char *space_name, uint32_t name_len,
> -		 uint32_t errcode, uint32_t fieldno, struct region *region)
> -{
> -	if (mp_typeof(**data) != MP_MAP) {
> -		tnt_raise(ClientError, errcode, tt_cstr(space_name, name_len),
> -			  tt_sprintf("field %d is not map",
> -				     fieldno + TUPLE_INDEX_BASE));
> -	}
> -	int count = mp_decode_map(data);
> -	*field = field_def_default;
> -	bool is_action_missing = true;
> -	uint32_t action_literal_len = strlen("nullable_action");
> -	for (int i = 0; i < count; ++i) {
> -		if (mp_typeof(**data) != MP_STR) {
> -			tnt_raise(ClientError, errcode,
> -				  tt_cstr(space_name, name_len),
> -				  tt_sprintf("field %d format is not map"\
> -					     " with string keys",
> -					     fieldno + TUPLE_INDEX_BASE));
> -		}
> -		uint32_t key_len;
> -		const char *key = mp_decode_str(data, &key_len);
> -		if (opts_parse_key(field, field_def_reg, key, key_len, data,
> -				   ER_WRONG_SPACE_FORMAT,
> -				   fieldno + TUPLE_INDEX_BASE, region,
> -				   true) != 0)
> -			diag_raise();
> -		if (is_action_missing &&
> -		    key_len == action_literal_len &&
> -		    memcmp(key, "nullable_action", action_literal_len) == 0)
> -			is_action_missing = false;
> -	}
> -	if (is_action_missing) {
> -		field->nullable_action = field->is_nullable ?
> -			ON_CONFLICT_ACTION_NONE
> -			: ON_CONFLICT_ACTION_DEFAULT;
> -	}
> -	if (field->name == NULL) {
> -		tnt_raise(ClientError, errcode, tt_cstr(space_name, name_len),
> -			  tt_sprintf("field %d name is not specified",
> -				     fieldno + TUPLE_INDEX_BASE));
> -	}
> -	size_t field_name_len = strlen(field->name);
> -	if (field_name_len > BOX_NAME_MAX) {
> -		tnt_raise(ClientError, errcode, tt_cstr(space_name, name_len),
> -			  tt_sprintf("field %d name is too long",
> -				     fieldno + TUPLE_INDEX_BASE));
> -	}
> -	identifier_check_xc(field->name, field_name_len);
> -	if (field->type == field_type_MAX) {
> -		tnt_raise(ClientError, errcode, tt_cstr(space_name, name_len),
> -			  tt_sprintf("field %d has unknown field type",
> -				     fieldno + TUPLE_INDEX_BASE));
> -	}
> -	if (field->nullable_action == on_conflict_action_MAX) {
> -		tnt_raise(ClientError, errcode, tt_cstr(space_name, name_len),
> -			  tt_sprintf("field %d has unknown field on conflict "
> -				     "nullable action",
> -				     fieldno + TUPLE_INDEX_BASE));
> -	}
> -	if (!((field->is_nullable && field->nullable_action ==
> -	       ON_CONFLICT_ACTION_NONE)
> -	      || (!field->is_nullable
> -		  && field->nullable_action != ON_CONFLICT_ACTION_NONE))) {
> -		tnt_raise(ClientError, errcode, tt_cstr(space_name, name_len),
> -			  tt_sprintf("field %d has conflicting nullability and "
> -				     "nullable action properties", fieldno +
> -				     TUPLE_INDEX_BASE));
> -	}
> -	if (field->coll_id != COLL_NONE &&
> -	    field->type != FIELD_TYPE_STRING &&
> -	    field->type != FIELD_TYPE_SCALAR &&
> -	    field->type != FIELD_TYPE_ANY) {
> -		tnt_raise(ClientError, errcode, tt_cstr(space_name, name_len),
> -			  tt_sprintf("collation is reasonable only for "
> -				     "string, scalar and any fields"));
> -	}
> -
> -	const char *dv = field->default_value;
> -	if (dv != NULL) {
> -		field->default_value_expr = sql_expr_compile(sql_get(), dv,
> -							     strlen(dv));
> -		if (field->default_value_expr == NULL)
> -			diag_raise();
> -	}
> -}
> -
> -/**
> - * Decode MessagePack array of fields.
> - * @param data MessagePack array of fields.
> - * @param[out] out_count Length of a result array.
> - * @param space_name Space name to use in error messages.
> - * @param errcode Errcode for client errors.
> - * @param region Region to allocate result array.
> - *
> - * @retval Array of fields.
> - */
> -static struct field_def *
> -space_format_decode(const char *data, uint32_t *out_count,
> -		    const char *space_name, uint32_t name_len,
> -		    uint32_t errcode, struct region *region)
> -{
> -	/* Type is checked by _space format. */
> -	assert(mp_typeof(*data) == MP_ARRAY);
> -	uint32_t count = mp_decode_array(&data);
> -	*out_count = count;
> -	if (count == 0)
> -		return NULL;
> -	size_t size = count * sizeof(struct field_def);
> -	struct field_def *region_defs =
> -		(struct field_def *) region_alloc_xc(region, size);
> -	/*
> -	 * Nullify to prevent a case when decoding will fail in
> -	 * the middle and space_def_destroy_fields() below will
> -	 * work with garbage pointers.
> -	 */
> -	memset(region_defs, 0, size);
> -	auto fields_guard = make_scoped_guard([=] {
> -		space_def_destroy_fields(region_defs, count);
> -	});
> -	for (uint32_t i = 0; i < count; ++i) {
> -		field_def_decode(&region_defs[i], &data, space_name, name_len,
> -				 errcode, i, region);
> -	}
> -	fields_guard.is_active = false;
> -	return region_defs;
> -}
> -
> -/**
>    * Fill space_def structure from struct tuple.
>    */
>   static struct space_def *
> @@ -508,8 +302,9 @@ space_def_new_from_tuple(struct tuple *tuple, uint32_t errcode,
>   	const char *format =
>   		tuple_field_with_type_xc(tuple, BOX_SPACE_FIELD_FORMAT,
>   					 MP_ARRAY);
> -	fields = space_format_decode(format, &field_count, name,
> -				     name_len, errcode, region);
> +	if (space_format_decode(&fields, format, &field_count, name, name_len,
> +				errcode, region) != 0)
> +		diag_raise();
>   	auto fields_guard = make_scoped_guard([=] {
>   		space_def_destroy_fields(fields, field_count);
>   	});
> diff --git a/src/box/index_def.c b/src/box/index_def.c
> index 45c74d9..e0d54c0 100644
> --- a/src/box/index_def.c
> +++ b/src/box/index_def.c
> @@ -309,3 +309,63 @@ index_def_is_valid(struct index_def *index_def, const char *space_name)
>   	}
>   	return true;
>   }
> +
> +int
> +index_opts_decode(struct index_opts *opts, const char *map,
> +		  struct region *region)
> +{
> +	index_opts_create(opts);
> +	if (opts_decode(opts, index_opts_reg, &map, ER_WRONG_INDEX_OPTIONS,
> +			BOX_INDEX_FIELD_OPTS, region) != 0) {
> +		return -1;
> +	}
> +	if (opts->distance == rtree_index_distance_type_MAX) {
> +		diag_set(ClientError, ER_WRONG_INDEX_OPTIONS,
> +			  BOX_INDEX_FIELD_OPTS, "distance must be either "\
> +			  "'euclid' or 'manhattan'");
> +		return -1;
> +	}
> +	if (opts->sql != NULL) {
> +		char *sql = strdup(opts->sql);
> +		if (sql == NULL) {
> +			opts->sql = NULL;
> +			diag_set(OutOfMemory, strlen(opts->sql) + 1, "strdup",
> +				 "sql");
> +			return -1;
> +		}
> +		opts->sql = sql;
> +	}
> +	if (opts->range_size <= 0) {
> +		diag_set(ClientError, ER_WRONG_INDEX_OPTIONS,
> +			 BOX_INDEX_FIELD_OPTS,
> +			 "range_size must be greater than 0");
> +		return -1;
> +	}
> +	if (opts->page_size <= 0 || opts->page_size > opts->range_size) {
> +		diag_set(ClientError, ER_WRONG_INDEX_OPTIONS,
> +			 BOX_INDEX_FIELD_OPTS,
> +			 "page_size must be greater than 0 and "
> +			 "less than or equal to range_size");
> +		return -1;
> +	}
> +	if (opts->run_count_per_level <= 0) {
> +		diag_set(ClientError, ER_WRONG_INDEX_OPTIONS,
> +			 BOX_INDEX_FIELD_OPTS,
> +			 "run_count_per_level must be greater than 0");
> +		return -1;
> +	}
> +	if (opts->run_size_ratio <= 1) {
> +		diag_set(ClientError, ER_WRONG_INDEX_OPTIONS,
> +			 BOX_INDEX_FIELD_OPTS,
> +			 "run_size_ratio must be greater than 1");
> +		return -1;
> +	}
> +	if (opts->bloom_fpr <= 0 || opts->bloom_fpr > 1) {
> +		diag_set(ClientError, ER_WRONG_INDEX_OPTIONS,
> +			 BOX_INDEX_FIELD_OPTS,
> +			 "bloom_fpr must be greater than 0 and "
> +			 "less than or equal to 1");
> +		return -1;
> +	}
> +	return 0;
> +}
> diff --git a/src/box/index_def.h b/src/box/index_def.h
> index 48a7820..4c3625b 100644
> --- a/src/box/index_def.h
> +++ b/src/box/index_def.h
> @@ -358,6 +358,14 @@ index_def_cmp(const struct index_def *key1, const struct index_def *key2);
>   bool
>   index_def_is_valid(struct index_def *index_def, const char *space_name);
>   
> +/**
> + * Fill index_opts structure from opts field in tuple of space _index
> + * Throw an error is unrecognized option.
> + */
> +int
> +index_opts_decode(struct index_opts *opts, const char *map,
> +		  struct region *region);
> +
>   #if defined(__cplusplus)
>   } /* extern "C" */
>   
> diff --git a/src/box/space_def.c b/src/box/space_def.c
> index f5ca0b5..8780eec 100644
> --- a/src/box/space_def.c
> +++ b/src/box/space_def.c
> @@ -34,6 +34,10 @@
>   #include "error.h"
>   #include "sql.h"
>   #include "msgpuck.h"
> +#include "tuple_format.h"
> +#include "schema_def.h"
> +#include "identifier.h"
> +#include "small/region.h"
>   
>   /**
>    * Make checks from msgpack.
> @@ -351,3 +355,154 @@ error:
>   	sql_expr_list_delete(db, checks);
>   	return  -1;
>   }
> +
> +/**
> + * Decode field definition from MessagePack map. Format:
> + * {name: <string>, type: <string>}. Type is optional.
> + * @param[out] field Field to decode to.
> + * @param data MessagePack map to decode.
> + * @param space_name Name of a space, from which the field is got.
> + *        Used in error messages.
> + * @param name_len Length of @a space_name.
> + * @param errcode Error code to use for client errors. Either
> + *        create or modify space errors.
> + * @param fieldno Field number to decode. Used in error messages.
> + * @param region Region to allocate field name.
> + *
> + * @retval 0 Success.
> + * @retval -1 Error.
> + */
> +static inline int
> +field_def_decode(struct field_def *field, const char **data,
> +		 const char *space_name, uint32_t name_len,
> +		 uint32_t errcode, uint32_t fieldno, struct region *region)
> +{
> +	if (mp_typeof(**data) != MP_MAP) {
> +		diag_set(ClientError, errcode, tt_cstr(space_name, name_len),
> +			  tt_sprintf("field %d is not map",
> +				     fieldno + TUPLE_INDEX_BASE));
> +		return -1;
> +	}
> +	int count = mp_decode_map(data);
> +	*field = field_def_default;
> +	bool is_action_missing = true;
> +	uint32_t action_literal_len = strlen("nullable_action");
> +	for (int i = 0; i < count; ++i) {
> +		if (mp_typeof(**data) != MP_STR) {
> +			diag_set(ClientError, errcode,
> +				  tt_cstr(space_name, name_len),
> +				  tt_sprintf("field %d format is not map"\
> +					     " with string keys",
> +					     fieldno + TUPLE_INDEX_BASE));
> +			return -1;
> +		}
> +		uint32_t key_len;
> +		const char *key = mp_decode_str(data, &key_len);
> +		if (opts_parse_key(field, field_def_reg, key, key_len, data,
> +				   ER_WRONG_SPACE_FORMAT,
> +				   fieldno + TUPLE_INDEX_BASE, region,
> +				   true) != 0)
> +			return -1;
> +		if (is_action_missing &&
> +		    key_len == action_literal_len &&
> +		    memcmp(key, "nullable_action", action_literal_len) == 0)
> +			is_action_missing = false;
> +	}
> +	if (is_action_missing) {
> +		field->nullable_action = field->is_nullable ?
> +			ON_CONFLICT_ACTION_NONE
> +			: ON_CONFLICT_ACTION_DEFAULT;
> +	}
> +	if (field->name == NULL) {
> +		diag_set(ClientError, errcode, tt_cstr(space_name, name_len),
> +			  tt_sprintf("field %d name is not specified",
> +				     fieldno + TUPLE_INDEX_BASE));
> +		return -1;
> +	}
> +	size_t field_name_len = strlen(field->name);
> +	if (field_name_len > BOX_NAME_MAX) {
> +		diag_set(ClientError, errcode, tt_cstr(space_name, name_len),
> +			  tt_sprintf("field %d name is too long",
> +				     fieldno + TUPLE_INDEX_BASE));
> +		return -1;
> +	}
> +	if (identifier_check(field->name, field_name_len))
> +		return -1;
> +	if (field->type == field_type_MAX) {
> +		diag_set(ClientError, errcode, tt_cstr(space_name, name_len),
> +			  tt_sprintf("field %d has unknown field type",
> +				     fieldno + TUPLE_INDEX_BASE));
> +		return -1;
> +	}
> +	if (field->nullable_action == on_conflict_action_MAX) {
> +		diag_set(ClientError, errcode, tt_cstr(space_name, name_len),
> +			  tt_sprintf("field %d has unknown field on conflict "
> +				     "nullable action",
> +				     fieldno + TUPLE_INDEX_BASE));
> +		return -1;
> +	}
> +	if (!((field->is_nullable && field->nullable_action ==
> +	       ON_CONFLICT_ACTION_NONE)
> +	      || (!field->is_nullable
> +		  && field->nullable_action != ON_CONFLICT_ACTION_NONE))) {
> +		diag_set(ClientError, errcode, tt_cstr(space_name, name_len),
> +			  tt_sprintf("field %d has conflicting nullability and "
> +				     "nullable action properties", fieldno +
> +				     TUPLE_INDEX_BASE));
> +		return -1;
> +	}
> +	if (field->coll_id != COLL_NONE &&
> +	    field->type != FIELD_TYPE_STRING &&
> +	    field->type != FIELD_TYPE_SCALAR &&
> +	    field->type != FIELD_TYPE_ANY) {
> +		diag_set(ClientError, errcode, tt_cstr(space_name, name_len),
> +			  tt_sprintf("collation is reasonable only for "
> +				     "string, scalar and any fields"));
> +		return -1;
> +	}
> +
> +	const char *dv = field->default_value;
> +	if (dv != NULL) {
> +		field->default_value_expr = sql_expr_compile(sql_get(), dv,
> +							     strlen(dv));
> +		if (field->default_value_expr == NULL)
> +			return -1;
> +	}
> +	return 0;
> +}
> +
> +int
> +space_format_decode(struct field_def **field, const char *data,
> +		    uint32_t *out_count, const char *space_name,
> +		    uint32_t name_len, uint32_t errcode, struct region *region)
> +{
> +	/* Type is checked by _space format. */
> +	assert(mp_typeof(*data) == MP_ARRAY);
> +	*field = NULL;
> +	uint32_t count = mp_decode_array(&data);
> +	*out_count = count;
> +	if (count == 0)
> +		return 0;
> +	size_t size = count * sizeof(struct field_def);
> +	struct field_def *region_defs =
> +		(struct field_def *) region_alloc(region, size);
> +	if (region_defs == NULL) {
> +		diag_set(OutOfMemory, size, "region_alloc", "region_defs");
> +		return -1;
> +	}
> +	/*
> +	 * Nullify to prevent a case when decoding will fail in
> +	 * the middle and space_def_destroy_fields() below will
> +	 * work with garbage pointers.
> +	 */
> +	memset(region_defs, 0, size);
> +	for (uint32_t i = 0; i < count; ++i) {
> +		if (field_def_decode(&region_defs[i], &data, space_name,
> +				     name_len, errcode, i, region) != 0) {
> +			space_def_destroy_fields(region_defs, count);
> +			return -1;
> +		}
> +	}
> +	*field = region_defs;
> +	return 0;
> +}
> diff --git a/src/box/space_def.h b/src/box/space_def.h
> index 0d1e902..7a01b92 100644
> --- a/src/box/space_def.h
> +++ b/src/box/space_def.h
> @@ -182,6 +182,24 @@ space_def_sizeof(uint32_t name_len, const struct field_def *fields,
>   		 uint32_t field_count, uint32_t *names_offset,
>   		 uint32_t *fields_offset, uint32_t *def_expr_offset);
>   
> +/**
> + * Decode MessagePack array of fields.
> + * @param[out] field Field to decode to.
> + * @param data MessagePack array of fields.
> + * @param[out] out_count Length of a result array.
> + * @param space_name Space name to use in error messages.
> + * @param name_len Length of @a space_name.
> + * @param errcode Errcode for client errors.
> + * @param region Region to allocate result array.
> + *
> + * @retval 0 Success.
> + * @retval -1 Error.
> + */
> +int
> +space_format_decode(struct field_def **field, const char *data,
> +		    uint32_t *out_count, const char *space_name,
> +		    uint32_t name_len, uint32_t errcode, struct region *region);
> +
>   #if defined(__cplusplus)
>   } /* extern "C" */
>   
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-07-18  9:24 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <cover.1531903378.git.imeevma@gmail.com>
2018-07-18  8:43 ` [tarantool-patches] [PATCH v2 4/7] box: bove some decode functions from alter.cc imeevma
2018-07-18  9:24   ` [tarantool-patches] " Vladislav Shpilevoy

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