| Message ID | f0f42cd56b9e54e9c7d58be41fcc4e226d5c76ff.1715166175.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | reftable: prepare for re-seekable iterators | expand |
On 24/05/08 01:04PM, Patrick Steinhardt wrote: > When seeking on a merged table, we perform the seek for each of the > subiterators. If the subiterator hasa the desired record we add it to s/hasa/has/ > the priority queue, otherwise we skip it and don't add it to the stack > of subiterators hosted by the merged table. > > The consequence of this is that the index of the subiterator in the > merged table does not necessarily correspond to the index of it in the > merged iterator. Next to being potentially confusing, it also means that > we won't easily be able to re-seek the merged iterator because we have > no clear connection between both of the data structures. Ah, I also found this a bit confusing. I think this is a good change. > > Refactor the code so that the index stays the same in both structures. Was there any advantage to not adding subiterators to the stack originally? It looks like it adding them doesn't affect anything. -Justin
On Fri, May 10, 2024 at 02:25:09PM -0500, Justin Tobler wrote: > On 24/05/08 01:04PM, Patrick Steinhardt wrote: > > When seeking on a merged table, we perform the seek for each of the > > subiterators. If the subiterator hasa the desired record we add it to > > s/hasa/has/ Fixed. > > the priority queue, otherwise we skip it and don't add it to the stack > > of subiterators hosted by the merged table. > > > > The consequence of this is that the index of the subiterator in the > > merged table does not necessarily correspond to the index of it in the > > merged iterator. Next to being potentially confusing, it also means that > > we won't easily be able to re-seek the merged iterator because we have > > no clear connection between both of the data structures. > > Ah, I also found this a bit confusing. I think this is a good change. > > > > Refactor the code so that the index stays the same in both structures. > > Was there any advantage to not adding subiterators to the stack > originally? It looks like it adding them doesn't affect anything. Not to the best of my knowledge, no. Patrick
diff --git a/reftable/merged.c b/reftable/merged.c index 4e1b78e93f..18a2a6f09b 100644 --- a/reftable/merged.c +++ b/reftable/merged.c @@ -37,6 +37,7 @@ static void merged_iter_init(struct merged_iter *mi, mi->advance_index = -1; mi->suppress_deletions = mt->suppress_deletions; REFTABLE_CALLOC_ARRAY(mi->subiters, mt->stack_len); + mi->stack_len = mt->stack_len; } static void merged_iter_close(void *p) @@ -236,21 +237,19 @@ static int merged_table_seek_record(struct reftable_merged_table *mt, 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_init(&merged.subiters[i].rec, reftable_record_type(rec)); err = reftable_table_seek_record(&mt->stack[i], - &merged.subiters[merged.stack_len].iter, rec); + &merged.subiters[i].iter, rec); if (err < 0) goto out; if (err > 0) continue; - err = merged_iter_advance_subiter(&merged, merged.stack_len); + err = merged_iter_advance_subiter(&merged, i); if (err < 0) goto out; - - merged.stack_len++; } p = reftable_malloc(sizeof(*p));
When seeking on a merged table, we perform the seek for each of the subiterators. If the subiterator hasa the desired record we add it to the priority queue, otherwise we skip it and don't add it to the stack of subiterators hosted by the merged table. The consequence of this is that the index of the subiterator in the merged table does not necessarily correspond to the index of it in the merged iterator. Next to being potentially confusing, it also means that we won't easily be able to re-seek the merged iterator because we have no clear connection between both of the data structures. Refactor the code so that the index stays the same in both structures. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- reftable/merged.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)