Skip to content

feat: Fuse partition and metrics filtering into manifest deserialization#3658

Open
zenfenan wants to merge 1 commit into
apache:mainfrom
zenfenan:fuse-partition-filter-manifest-entry
Open

feat: Fuse partition and metrics filtering into manifest deserialization#3658
zenfenan wants to merge 1 commit into
apache:mainfrom
zenfenan:fuse-partition-filter-manifest-entry

Conversation

@zenfenan

@zenfenan zenfenan commented Jul 14, 2026

Copy link
Copy Markdown

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.

@zenfenan
zenfenan force-pushed the fuse-partition-filter-manifest-entry branch from fc4e2f1 to 9492802 Compare July 14, 2026 22:32

@abnobdoss abnobdoss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Returns:
A list of ManifestEntry that matches the provided filters.
"""
return [

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great find! Would it be better to incorporate this fused behavior directly into fetch_manifest_entry by allowing it to accept an entry_filter: Callable[[ManifestEntry], bool] | None arg? That way the optimization is generic and available to all callers that currently materialize the full list just to discard entries immediately.

The call site in _open_manifest would become:

return manifest.fetch_manifest_entry(
    io,
    discard_deleted=True,
    entry_filter=lambda e: partition_filter(e.data_file) and metrics_evaluator(e.data_file),
)

This avoids duplicating the Avro reader setup and _inherit_from_manifest call ordering between fetch_manifest_entry and prune_manifest_entry, and there are 6 other call sites that filter after materializing that could benefit from the same interface:

  1. table/__init__.py - _open_manifest (partition + metrics)
  2. table/inspect.py - _get_files_from_manifest (content type)
  3. table/update/snapshot.py - _OverwriteFiles._build_... (strict + inclusive metrics)
  4. table/update/validate.py - _deleted_data_files (snapshot_id + data_filter + partition_set)
  5. table/update/validate.py - _added_data_files (same)
  6. table/update/validate.py - _added_delete_files (same)

Using Callable[[ManifestEntry], bool] rather than Callable[[DataFile], bool] covers all of these since the validate callers filter on entry-level fields like snapshot_id and status, not just DataFile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fuse partition and metrics filtering into manifest entry deserialization

2 participants