Re: [PATCH 5/6] reftable/reader: make table iterator reseekable
- From
karthik nayak <karthik.188@gmail.com>
- Date
- Sep 13, 2024, 12:11 UTC
- Message-ID
- <CAOLa=ZRA=vFjnWZC7PQBAxOxDfD9X6767ddgBqwaen5n6JLt0w@mail.gmail.com>
- In-Reply-To
- <a281f936a2b3a697b32f57652a2120afd54f7e4f.1725881266.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
[snip]
Show 70 quoted lines
> diff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c
> index b8c92fdb365..19e54bdfb8b 100644
> --- a/t/unit-tests/t-reftable-merged.c
> +++ b/t/unit-tests/t-reftable-merged.c
> @@ -194,6 +194,81 @@ static void t_merged_refs(void)
> reftable_free(bs);
> }
>
> +static void t_merged_seek_multiple_times(void)
> +{
> + struct reftable_ref_record r1[] = {
> + {
> + .refname = (char *) "a",
> + .update_index = 1,
> + .value_type = REFTABLE_REF_VAL1,
> + .value.val1 = { 1 },
> + },
> + {
> + .refname = (char *) "c",
> + .update_index = 1,
> + .value_type = REFTABLE_REF_VAL1,
> + .value.val1 = { 2 },
> + }
> + };
> + struct reftable_ref_record r2[] = {
> + {
> + .refname = (char *) "b",
> + .update_index = 2,
> + .value_type = REFTABLE_REF_VAL1,
> + .value.val1 = { 3 },
> + },
> + {
> + .refname = (char *) "d",
> + .update_index = 2,
> + .value_type = REFTABLE_REF_VAL1,
> + .value.val1 = { 4 },
> + },
> + };
> + struct reftable_ref_record *refs[] = {
> + r1, r2,
> + };
> + size_t sizes[] = {
> + ARRAY_SIZE(r1), ARRAY_SIZE(r2),
> + };
> + struct strbuf bufs[] = {
> + STRBUF_INIT, STRBUF_INIT,
> + };
> + struct reftable_block_source *sources = NULL;
> + struct reftable_reader **readers = NULL;
> + struct reftable_ref_record rec = { 0 };
> + struct reftable_iterator it = { 0 };
> + struct reftable_merged_table *mt;
> +
> + mt = merged_table_from_records(refs, &sources, &readers, sizes, bufs, 2);
> + merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
> +
> + for (size_t i = 0; i < 5; i++) {
> + int err = reftable_iterator_seek_ref(&it, "c");
> + check(!err);
> +
> + err = reftable_iterator_next_ref(&it, &rec);
> + check(!err);
> + err = reftable_ref_record_equal(&rec, &r1[1], GIT_SHA1_RAWSZ);
> + check(err == 1);
> +
> + err = reftable_iterator_next_ref(&it, &rec);
> + check(!err);
> + err = reftable_ref_record_equal(&rec, &r2[1], GIT_SHA1_RAWSZ);
> + check(err == 1);
> +So this skips r2[0] because when we seek, we seek all sub-iterators. So in r2 we seek to 'd' since that is the next alphabetical value.
[snip]
Show 40 quoted lines
> diff --git a/t/unit-tests/t-reftable-reader.c b/t/unit-tests/t-reftable-reader.c
> new file mode 100644
> index 00000000000..7a46580b6f1
> --- /dev/null
> +++ b/t/unit-tests/t-reftable-reader.c
> @@ -0,0 +1,96 @@
> +#include "test-lib.h"
> +#include "lib-reftable.h"
> +#include "reftable/blocksource.h"
> +#include "reftable/reader.h"
> +
> +static int t_reader_seek_once(void)
> +{
> + struct reftable_ref_record records[] = {
> + {
> + .refname = (char *) "refs/heads/main",
> + .value_type = REFTABLE_REF_VAL1,
> + .value.val1 = { 42 },
> + },
> + };
> + struct reftable_block_source source = { 0 };
> + struct reftable_ref_record ref = { 0 };
> + struct reftable_iterator it = { 0 };
> + struct reftable_reader *reader;
> + struct strbuf buf = STRBUF_INIT;
> + int ret;
> +
> + t_reftable_write_to_buf(&buf, records, ARRAY_SIZE(records), NULL, 0, NULL);
> + block_source_from_strbuf(&source, &buf);
> +
> + ret = reftable_reader_new(&reader, &source, "name");
> + check_int(ret, ==, 0);
> +
> + reftable_reader_init_ref_iterator(reader, &it);
> + ret = reftable_iterator_seek_ref(&it, "");
> + check_int(ret, ==, 0);
> + ret = reftable_iterator_next_ref(&it, &ref);
> + check_int(ret, ==, 0);
> +
> + ret = reftable_ref_record_equal(&ref, &records[0], 20);s/20/GIT_SHA1_RAWSZ
Also here and elsewhere, shouldn't we just do `check(reftable_ref_record_equal(...))` or even `!check(reftable_iterator_seek_ref(...))` ?
[snip]