[tarantool-patches] [PATCH v2 4/7] box: bove some decode functions from alter.cc

  • From: imeevma@xxxxxxxxxxxxx
  • To: tarantool-patches@xxxxxxxxxxxxx
  • Date: Wed, 18 Jul 2018 11:43:40 +0300

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


Other related posts: