From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Kirill Shcherbatov Subject: [PATCH v1 2/5] box: refactor field type and nullability checks Date: Sun, 23 Dec 2018 15:40:37 +0300 Message-Id: In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit To: tarantool-patches@freelists.org, vdavydov.dev@gmail.com Cc: kostja@tarantool.org, Kirill Shcherbatov List-ID: Reworked field types and nullability checks to set error message in tuple_init_field_map manually. We need to specify full JSON path to field sting in further patches. - introduced field_mp_type_is_compatible routine making field_type and mp_type compatibility test taking into account field nullability - refactored key_part_validate to pass const char * key argument and to reuse field_mp_type_is_compatible code Needed for #1012 --- src/box/index.cc | 22 +++++++--------------- src/box/key_def.c | 12 +++++------- src/box/key_def.h | 30 ++++++++++++++++++++---------- src/box/tuple_format.c | 18 +++++++++++------- 4 files changed, 43 insertions(+), 39 deletions(-) diff --git a/src/box/index.cc b/src/box/index.cc index cf81eca49..1dbf364a8 100644 --- a/src/box/index.cc +++ b/src/box/index.cc @@ -89,9 +89,7 @@ key_validate(const struct index_def *index_def, enum iterator_type type, return -1; } if (part_count == 1) { - enum mp_type mp_type = mp_typeof(*key); - if (key_mp_type_validate(FIELD_TYPE_ARRAY, mp_type, - ER_KEY_PART_TYPE, 0, false)) + if (key_part_validate(FIELD_TYPE_ARRAY, key, 0, false)) return -1; uint32_t array_size = mp_decode_array(&key); if (array_size != d && array_size != d * 2) { @@ -100,23 +98,17 @@ key_validate(const struct index_def *index_def, enum iterator_type type, return -1; } for (uint32_t part = 0; part < array_size; part++) { - enum mp_type mp_type = mp_typeof(*key); - mp_next(&key); - if (key_mp_type_validate(FIELD_TYPE_NUMBER, - mp_type, - ER_KEY_PART_TYPE, 0, - false)) + if (key_part_validate(FIELD_TYPE_NUMBER, key, + 0, false)) return -1; + mp_next(&key); } } else { for (uint32_t part = 0; part < part_count; part++) { - enum mp_type mp_type = mp_typeof(*key); - mp_next(&key); - if (key_mp_type_validate(FIELD_TYPE_NUMBER, - mp_type, - ER_KEY_PART_TYPE, - part, false)) + if (key_part_validate(FIELD_TYPE_NUMBER, key, + part, false)) return -1; + mp_next(&key); } } } else { diff --git a/src/box/key_def.c b/src/box/key_def.c index 2119ca389..4cc06de47 100644 --- a/src/box/key_def.c +++ b/src/box/key_def.c @@ -88,7 +88,7 @@ const char *mp_type_strs[] = { /* .MP_EXT = */ "extension", }; -const uint32_t key_mp_type[] = { +const uint32_t field_mp_type[] = { /* [FIELD_TYPE_ANY] = */ UINT32_MAX, /* [FIELD_TYPE_UNSIGNED] = */ 1U << MP_UINT, /* [FIELD_TYPE_STRING] = */ 1U << MP_STR, @@ -608,14 +608,12 @@ key_validate_parts(const struct key_def *key_def, const char *key, uint32_t part_count, bool allow_nullable) { for (uint32_t i = 0; i < part_count; i++) { - enum mp_type mp_type = mp_typeof(*key); const struct key_part *part = &key_def->parts[i]; - mp_next(&key); - - if (key_mp_type_validate(part->type, mp_type, ER_KEY_PART_TYPE, - i, key_part_is_nullable(part) - && allow_nullable)) + if (key_part_validate(part->type, key, i, + key_part_is_nullable(part) && + allow_nullable)) return -1; + mp_next(&key); } return 0; } diff --git a/src/box/key_def.h b/src/box/key_def.h index d4da6c5a1..2d9a3a7c2 100644 --- a/src/box/key_def.h +++ b/src/box/key_def.h @@ -392,27 +392,37 @@ key_def_has_collation(const struct key_def *key_def) return false; } -/** A helper table for key_mp_type_validate */ -extern const uint32_t key_mp_type[]; +/** A helper table for field_mp_type_is_compatible */ +extern const uint32_t field_mp_type[]; + +/** Checks if mp_type (MsgPack) is compatible with field type. */ +static inline bool +field_mp_type_is_compatible(enum field_type type, enum mp_type mp_type, + bool is_nullable) +{ + assert(type < field_type_MAX); + assert((size_t)mp_type < CHAR_BIT * sizeof(*field_mp_type)); + uint32_t mask = field_mp_type[type] | (is_nullable * (1U << MP_NIL)); + return (mask & (1U << mp_type)) != 0; +} /** * @brief Checks if \a field_type (MsgPack) is compatible \a type (KeyDef). * @param type KeyDef type - * @param field_type MsgPack type + * @param key Pointer to MsgPack field to be tested. * @param field_no - a field number (is used to store an error message) * * @retval 0 mp_type is valid. * @retval -1 mp_type is invalid. */ static inline int -key_mp_type_validate(enum field_type key_type, enum mp_type mp_type, - int err, uint32_t field_no, bool is_nullable) +key_part_validate(enum field_type key_type, const char *key, + uint32_t field_no, bool is_nullable) { - assert(key_type < field_type_MAX); - assert((size_t) mp_type < CHAR_BIT * sizeof(*key_mp_type)); - uint32_t mask = key_mp_type[key_type] | (is_nullable * (1U << MP_NIL)); - if (unlikely((mask & (1U << mp_type)) == 0)) { - diag_set(ClientError, err, field_no, field_type_strs[key_type]); + if (unlikely(!field_mp_type_is_compatible(key_type, mp_typeof(*key), + is_nullable))) { + diag_set(ClientError, ER_KEY_PART_TYPE, field_no, + field_type_strs[key_type]); return -1; } return 0; diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c index 3af39a37e..04343fd53 100644 --- a/src/box/tuple_format.c +++ b/src/box/tuple_format.c @@ -451,13 +451,15 @@ tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map, } /* first field is simply accessible, so we do not store offset to it */ - enum mp_type mp_type = mp_typeof(*pos); const struct tuple_field *field = tuple_format_field((struct tuple_format *)format, 0); if (validate && - key_mp_type_validate(field->type, mp_type, ER_FIELD_TYPE, - TUPLE_INDEX_BASE, tuple_field_is_nullable(field))) + !field_mp_type_is_compatible(field->type, mp_typeof(*pos), + tuple_field_is_nullable(field))) { + diag_set(ClientError, ER_FIELD_TYPE, TUPLE_INDEX_BASE, + field_type_strs[field->type]); return -1; + } mp_next(&pos); /* other fields...*/ uint32_t i = 1; @@ -474,12 +476,14 @@ tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map, } for (; i < defined_field_count; ++i) { field = tuple_format_field((struct tuple_format *)format, i); - mp_type = mp_typeof(*pos); if (validate && - key_mp_type_validate(field->type, mp_type, ER_FIELD_TYPE, - i + TUPLE_INDEX_BASE, - tuple_field_is_nullable(field))) + !field_mp_type_is_compatible(field->type, mp_typeof(*pos), + tuple_field_is_nullable(field))) { + diag_set(ClientError, ER_FIELD_TYPE, + i + TUPLE_INDEX_BASE, + field_type_strs[field->type]); return -1; + } if (field->offset_slot != TUPLE_OFFSET_SLOT_NIL) { field_map[field->offset_slot] = (uint32_t) (pos - tuple); -- 2.19.2