[PATCH v5 1/4] box: introduce tuple_format_iterator class
Vladimir Davydov
vdavydov.dev at gmail.com
Mon May 6 16:55:55 MSK 2019
On Mon, May 06, 2019 at 02:57:38PM +0300, Kirill Shcherbatov wrote:
> The similar code in tuple_field_map_create and
> vy_stmt_new_surrogate_delete_raw that performs tuple decode
> with tuple_format has been refactored as reusable
> tuple_format_iterator class.
>
> Being thus encapsulated, this code will be uniformly managed and
> extended in the further patches in scope of multikey indexes.
>
> Extended engine/json test with vy_stmt_new_surrogate_delete_raw
> corner case test. There was no problem before this patch, but
> small bug appeared during tuple_format_iterator_next
> implementation was not covered.
>
> Needed for #1257
> ---
> src/box/tuple_format.c | 356 ++++++++++++++++++++------------------
> src/box/tuple_format.h | 118 +++++++++++++
> src/box/vy_stmt.c | 117 +++++--------
> test/engine/json.result | 17 ++
> test/engine/json.test.lua | 5 +
> 5 files changed, 371 insertions(+), 242 deletions(-)
Looks good. See a few really minor nit picks below.
>
> diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
> index 804a678a1..de48e9bb0 100644
> --- a/src/box/tuple_format.c
> +++ b/src/box/tuple_format.c
> @@ -1033,3 +883,179 @@ box_tuple_format_unref(box_tuple_format_t *format)
> tuple_format_unref(format);
> }
>
> +int
> +tuple_format_iterator_create(struct tuple_format_iterator *it,
> + struct tuple_format *format, const char *tuple,
> + uint8_t flags, uint32_t *defined_field_count,
> + struct region *region)
> +{
> + assert(mp_typeof(*tuple) == MP_ARRAY);
> + *defined_field_count = mp_decode_array(&tuple);
> + bool validate = flags & TUPLE_FORMAT_ITERATOR_VALIDATE;
> + if (validate && format->exact_field_count > 0 &&
> + format->exact_field_count != *defined_field_count) {
> + diag_set(ClientError, ER_EXACT_FIELD_COUNT,
> + (unsigned) *defined_field_count,
> + (unsigned) format->exact_field_count);
> + return -1;
> + }
> + it->parent = &format->fields.root;
> + it->format = format;
> + it->pos = tuple;
> + it->flags = flags;
> + it->required_fields = NULL;
> + it->required_fields_sz = 0;
> +
> + uint32_t frames_sz = format->fields_depth * sizeof(struct mp_frame);
> + if (validate)
> + it->required_fields_sz = bitmap_size(format->total_field_count);
> + uint32_t total_sz = frames_sz + 2 * it->required_fields_sz;
2 * ?
Shouldn't this be a part of the next patch?
> + struct mp_frame *frames = region_alloc(region, total_sz);
> + if (frames == NULL) {
> + diag_set(OutOfMemory, total_sz, "region",
> + "tuple_format_iterator");
> + return -1;
> + }
> + mp_stack_create(&it->stack, format->fields_depth, frames);
> + *defined_field_count = MIN(*defined_field_count, validate ?
Shouldn't we use KEY_PARTS_ONLY here rather than VALIDATE?
> + tuple_format_field_count(format) :
> + format->index_field_count);
> + mp_stack_push(&it->stack, MP_ARRAY, *defined_field_count);
> +
> + if (validate) {
> + it->required_fields = (char *)frames + frames_sz;
> + memcpy(it->required_fields, format->required_fields,
> + it->required_fields_sz);
> + }
> + return 0;
> +}
> +
> +/**
> + * Scan required_fields bitmap and raise error when it is
> + * non-empty.
> + * @sa format:required_fields and field:multikey_required_fields
> + * definition.
There's no such thing as multikey_required_fields yet.
> + */
> +static int
> +tuple_format_required_fields_validate(struct tuple_format *format,
> + void *required_fields,
> + uint32_t required_fields_sz)
> +{
> + struct bit_iterator it;
> + bit_iterator_init(&it, required_fields, required_fields_sz, true);
> + size_t id = bit_iterator_next(&it);
> + if (id < SIZE_MAX) {
> + /* A field is missing, report an error. */
> + struct tuple_field *field =
> + tuple_format_field_by_id(format, id);
> + assert(field != NULL);
> + diag_set(ClientError, ER_FIELD_MISSING,
> + tuple_field_path(field));
> + return -1;
> + }
> + return 0;
> +}
> +
> +int
> +tuple_format_iterator_next(struct tuple_format_iterator *it,
> + struct tuple_format_iterator_entry *entry)
> +{
> + entry->data = it->pos;
> + struct mp_frame *frame = mp_stack_top(&it->stack);
> + while (!mp_frame_advance(frame)) {
> + /*
> + * If the elements of the current frame
> + * are over, pop this frame out of stack
> + * and climb one position in the format::fields
> + * tree to match the changed JSON path to the
> + * data in the tuple.
> + */
> + mp_stack_pop(&it->stack);
> + if (mp_stack_is_empty(&it->stack))
> + goto eof;
> + frame = mp_stack_top(&it->stack);
> + it->parent = it->parent->parent;
> + }
> + entry->parent =
> + it->parent != &it->format->fields.root ?
> + json_tree_entry(it->parent, struct tuple_field, token) : NULL;
> + /*
> + * Use the top frame of the stack and the
> + * current data offset to prepare the JSON token
> + * and perform subsequent format::fields lookup.
> + */
> + struct json_token token;
> + switch (frame->type) {
> + case MP_ARRAY:
> + token.type = JSON_TOKEN_NUM;
> + token.num = frame->idx;
> + break;
> + case MP_MAP:
> + if (mp_typeof(*it->pos) != MP_STR) {
> + entry->data = it->pos;
> + entry->field = NULL;
> + mp_next(&it->pos);
> + mp_next(&it->pos);
What about entry->data_end?
> + return 0;
> + }
> + token.type = JSON_TOKEN_STR;
> + token.str = mp_decode_str(&it->pos, (uint32_t *)&token.len);
> + break;
> + default:
> + unreachable();
> + }
> + assert(it->parent != NULL);
> + struct tuple_field *field =
> + json_tree_lookup_entry(&it->format->fields, it->parent, &token,
> + struct tuple_field, token);
> + if (it->flags & TUPLE_FORMAT_ITERATOR_KEY_PARTS_ONLY &&
> + field != NULL && !field->is_key_part)
> + field = NULL;
> + entry->field = field;
> + entry->data = it->pos;
> + /*
> + * If the current position of the data in tuple
> + * matches the container type (MP_MAP or MP_ARRAY)
> + * and the format::fields tree has such a record,
> + * prepare a new stack frame because it needs to
> + * be analyzed in the next iterations.
> + */
> + enum mp_type type = mp_typeof(*it->pos);
> + if ((type == MP_ARRAY || type == MP_MAP) &&
> + !mp_stack_is_full(&it->stack) && field != NULL) {
> + uint32_t size = type == MP_ARRAY ?
> + mp_decode_array(&it->pos) :
> + mp_decode_map(&it->pos);
> + entry->count = size;
> + mp_stack_push(&it->stack, type, size);
> + it->parent = &field->token;
> + } else {
> + entry->count = 0;
> + mp_next(&it->pos);
> + }
> + entry->data_end = it->pos;
> + if (field == NULL || (it->flags & TUPLE_FORMAT_ITERATOR_VALIDATE) == 0)
> + return 0;
> +
> + /*
> + * Check if field mp_type is compatible with type
> + * defined in format.
> + */
> + bool is_nullable = tuple_field_is_nullable(field);
> + if (!field_mp_type_is_compatible(field->type, mp_typeof(*entry->data),
> + is_nullable) != 0) {
> + diag_set(ClientError, ER_FIELD_TYPE,
> + tuple_field_path(field),
> + field_type_strs[field->type]);
> + return -1;
> + }
> + bit_clear(it->required_fields, field->id);
> + return 0;
> +eof:
> + if (it->flags & TUPLE_FORMAT_ITERATOR_VALIDATE &&
> + tuple_format_required_fields_validate(it->format,
> + it->required_fields, it->required_fields_sz) != 0)
> + return -1;
> + entry->data = NULL;
> + return 0;
> +}
More information about the Tarantool-patches
mailing list