On 24/05/08 01:04PM, Patrick Steinhardt wrote:
> To initialize a `struct merged_iter`, we need to seek all subiterators
> to the wanted record and then add their results to the priority queue
> used to sort the records. This logic is split up across two functions,
> `merged_table_seek_record()` and `merged_table_iter()`. The scope of
Did we mean `merged_iter_init` instead of `merged_table_iter()` here?
> these functions is somewhat weird though, where `merged_table_iter()` is
> only responsible for adding the records of the subiterators to the
> priority queue.
>
> Clarify the scope of those functions such that `merged_table_iter()` is
> only responsible for initializing the iterator's structure. Performing
> the subiterator seeks are now part of `merged_table_seek_record()`.
>
> This step is required to move seeking of records into the generic
> `struct reftable_iterator` infrastructure.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
...
> @@ -246,32 +230,33 @@ static int merged_table_seek_record(struct reftable_merged_table *mt,
> struct reftable_iterator *it,
> struct reftable_record *rec)
> {
> - struct merged_iter merged = {
> - .typ = reftable_record_type(rec),
> - .hash_id = mt->hash_id,
> - .suppress_deletions = mt->suppress_deletions,
> - .advance_index = -1,
> - };
> - struct merged_iter *p;
> + struct merged_iter merged, *p;
> int err;
>
> - REFTABLE_CALLOC_ARRAY(merged.subiters, mt->stack_len);
> + merged_iter_init(&merged, mt);
> +
> for (size_t i = 0; i < mt->stack_len; i++) {
> + reftable_record_init(&merged.subiters[merged.stack_len].rec,
> + reftable_record_type(rec));
I find it somewhat confusing how we increment the subiterator index
here. I assume this is because when a record is not found we don't want
to add it to the index. Might be nice to add a comment here explaining
this.
Edit: Looks like you address this in the next commit so I guess we are
good :)
> +
> err = reftable_table_seek_record(&mt->stack[i],
> &merged.subiters[merged.stack_len].iter, rec);
> if (err < 0)
> goto out;
> - if (!err)
> - merged.stack_len++;
> - }
> + if (err > 0)
> + continue;
>
> - err = merged_iter_init(&merged);
> - if (err < 0)
> - goto out;
> + err = merged_iter_advance_subiter(&merged, merged.stack_len);
> + if (err < 0)
> + goto out;
> +
> + merged.stack_len++;
> + }
>
> - p = reftable_malloc(sizeof(struct merged_iter));
> + p = reftable_malloc(sizeof(*p));
> *p = merged;
> iterator_from_merged_iter(it, p);
> + err = 0;
>
> out:
> if (err < 0)
> --
> 2.45.0
>
