diff mbox series

[05/10] reftable/iter: drop double-checking logic

Message ID ab7538d0bbb8b3c02b5db71a421163fbf87a45f6.1723528765.git.ps@pks.im (mailing list archive)
State Superseded
Headers show
Series reftable: drop generic `reftable_table` interface | expand

Commit Message

Patrick Steinhardt Aug. 13, 2024, 6:24 a.m. UTC
The filtering ref iterator can be used to only yield refs which are not
in a specific skip list. This iterator has an option to double-check the
results it returns, which causes us to seek tho reference we are about
to yield via a separate table such that we detect whether the reference
that the first iterator has yielded actually exists.

The value of this is somewhat dubious, and I cannot think of any usecase
where this functionality should be required. Furthermore, this option is
never set in our codebase, which means that it is essentially untested.
And last but not least, the `struct reftable_table` that is used to
implement it is about to go away.

So while we could refactor the code to not use a `reftable_table`, it
very much feels like a wasted effort. Let's just drop this code.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/iter.c   | 20 --------------------
 reftable/iter.h   |  2 --
 reftable/reader.c |  2 --
 3 files changed, 24 deletions(-)

Comments

Eric Sunshine Aug. 13, 2024, 6:57 a.m. UTC | #1
On Tue, Aug 13, 2024 at 2:24 AM Patrick Steinhardt <ps@pks.im> wrote:
> The filtering ref iterator can be used to only yield refs which are not
> in a specific skip list. This iterator has an option to double-check the
> results it returns, which causes us to seek tho reference we are about

s/tho/the/

> to yield via a separate table such that we detect whether the reference
> that the first iterator has yielded actually exists.
>
> The value of this is somewhat dubious, and I cannot think of any usecase
> where this functionality should be required. Furthermore, this option is
> never set in our codebase, which means that it is essentially untested.
> And last but not least, the `struct reftable_table` that is used to
> implement it is about to go away.
>
> So while we could refactor the code to not use a `reftable_table`, it
> very much feels like a wasted effort. Let's just drop this code.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
diff mbox series

Patch

diff --git a/reftable/iter.c b/reftable/iter.c
index fddea31e51..a7484aba60 100644
--- a/reftable/iter.c
+++ b/reftable/iter.c
@@ -42,26 +42,6 @@  static int filtering_ref_iterator_next(void *iter_arg,
 			break;
 		}
 
-		if (fri->double_check) {
-			struct reftable_iterator it = { NULL };
-
-			reftable_table_init_ref_iter(&fri->tab, &it);
-
-			err = reftable_iterator_seek_ref(&it, ref->refname);
-			if (err == 0)
-				err = reftable_iterator_next_ref(&it, ref);
-
-			reftable_iterator_destroy(&it);
-
-			if (err < 0) {
-				break;
-			}
-
-			if (err > 0) {
-				continue;
-			}
-		}
-
 		if (ref->value_type == REFTABLE_REF_VAL2 &&
 		    (!memcmp(fri->oid.buf, ref->value.val2.target_value,
 			     fri->oid.len) ||
diff --git a/reftable/iter.h b/reftable/iter.h
index 537431baba..b75d7ac2ac 100644
--- a/reftable/iter.h
+++ b/reftable/iter.h
@@ -18,8 +18,6 @@  license that can be found in the LICENSE file or at
 
 /* iterator that produces only ref records that point to `oid` */
 struct filtering_ref_iterator {
-	int double_check;
-	struct reftable_table tab;
 	struct strbuf oid;
 	struct reftable_iterator it;
 };
diff --git a/reftable/reader.c b/reftable/reader.c
index f7ae35da72..e3f5854229 100644
--- a/reftable/reader.c
+++ b/reftable/reader.c
@@ -735,8 +735,6 @@  static int reftable_reader_refs_for_unindexed(struct reftable_reader *r,
 	*filter = empty;
 
 	strbuf_add(&filter->oid, oid, oid_len);
-	reftable_table_from_reader(&filter->tab, r);
-	filter->double_check = 0;
 	iterator_from_table_iter(&filter->it, ti);
 
 	iterator_from_filtering_ref_iterator(it, filter);