feat: Fuse partition and metrics filtering into manifest deserialization

[zenfenan]

Closes #3657

Rationale for this change

_open_manifest() currently deserializes all live manifest entries via fetch_manifest_entry() and then applies partitions & metrics filters as a second pass. This materialises the full list before discarding non-matching entries.

This PR adds ManifestFile#prune_manifest_entry() that fuses filtering into the deserialization loop, avoiding the intermediate list allocation for non-matching entries.

Total Entries	Entries Matching Filter	Two-pass Time	Fused Time	Improvement
116k	78 (0.07%)	9.13s	7.18s	-21.3%
438k	1 (~0%)	35.12s	27.81s	-20.8%
823k	4,962 (0.6%)	54.38s	41.38s	-23.9%
18k	269 (1.5%)	1.23s	0.90s	-27.0%
8k	185 (2.2%)	0.57s	0.40s	-30.2%

Are these changes tested?

Yes, existing test suit passes. No behavioral change. Correctness was validated across all benchmark runs (matched entry counts identical between both paths).

Are there any user-facing changes?

No.

[abnobdoss]

This looks good. My main concern is whether we can extend fetch_manifest_entry and avoid introducing prune_manifest_entry.

[abnobdoss]

Thank you for addressing the feedback. This looks good to me!

One thought: a simpler alternative might be to convert fetch_manifest_entry into a generator instead of adding entry_filter. That way _open_manifest keeps its existing comprehension filter and the memory win comes for free, with no new parameter. It would be a wider change though, so not sure what maintainers would prefer here.

[zenfenan]

Thank you for addressing the feedback. This looks good to me!

One thought: a simpler alternative might be to convert fetch_manifest_entry into a generator instead of adding entry_filter. That way _open_manifest keeps its existing comprehension filter and the memory win comes for free, with no new parameter. It would be a wider change though, so not sure what maintainers would prefer here.

Thanks! Good idea. A generator would give the memory win without a new parameter.

Two things make me lean toward keeping the current approach here, though: it changes the return-type contract (callers doing len()/indexing including some existing tests would need list() wrapping), and it changes resource semantics since the Avro file handle would stay open for the lifetime of iteration rather than being read and closed up front.

Happy to explore the generator conversion as a separate PR if maintainers think the broader change is worth it.

[zenfenan]

@Fokko @kevinjqliu

Whenever you have a free moment, could you please take a look at this PR? All checks are green and previous feedback is addressed. Really appreciate it!

[github-actions]

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

[github-actions]

This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

[zenfenan]

@kevinjqliu Could you please re-open this PR, and review this? Highly appreciated!

[Fokko]

This is a very clean optimization, thanks @zenfenan for working on this 👍