[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