[tarantool-patches] Re: [PATCH v5 1/4] box: introduce tuple_format_iterator class
Kirill Shcherbatov
kshcherbatov at tarantool.org
Mon May 6 17:32:37 MSK 2019
>> + uint32_t total_sz = frames_sz + 2 * it->required_fields_sz;
>
> 2 * ?
>
> Shouldn't this be a part of the next patch?
Yep. Moved to the last patch.
>> + 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?
Yes, this works.
>> +/**
>> + * 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.
Moved to the last patch.
>> + 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?
Both of them are redundant, entry->data must be only != NULL.
But I've updated data_end correspondingly:
case MP_MAP:
if (mp_typeof(*it->pos) != MP_STR) {
entry->field = NULL;
mp_next(&it->pos);
entry->data = it->pos;
mp_next(&it->pos);
entry->data_end = it->pos;
return 0;
}
==============================================
Updated patch on branch.
More information about the Tarantool-patches
mailing list