Skip to content

perf: skip building FilterRemapper when there are no parent filters - #24428

Open
Braedon-Wooding-Displayr wants to merge 2 commits into
apache:mainfrom
Braedon-Wooding-Displayr:perf/skip-identity-projection
Open

perf: skip building FilterRemapper when there are no parent filters#24428
Braedon-Wooding-Displayr wants to merge 2 commits into
apache:mainfrom
Braedon-Wooding-Displayr:perf/skip-identity-projection

Conversation

@Braedon-Wooding-Displayr

@Braedon-Wooding-Displayr Braedon-Wooding-Displayr commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

ChildFilterDescription::from_child (and from_child_with_allowed_indices) built a FilterRemapper before checking whether parent_filters was empty. FilterRemapper::new indexes every column of the child's schema into a HashMap, and with no filters to remap that index is never used: remap_filters returns an empty description regardless of what the remapper contains.

Each plan node produces one ChildFilterDescription per child, so a union pays this cost once per child. On a 250,000-column scan a sampling profile put FilterRemapper::new at 794 of 20,373 samples, almost entirely HashMap insertion and hashing. Return ChildFilterDescription::empty() directly when parent_filters is empty, before the remapper is built.

Which issue does this PR close?

Do I need an issue for this change, it feels like a really minor fix?

ChildFilterDescription::from_child (and from_child_with_allowed_indices)
built a FilterRemapper before checking whether parent_filters was
empty. FilterRemapper::new indexes every column of the child's schema
into a HashMap, and with no filters to remap that index is never used:
remap_filters returns an empty description regardless of what the
remapper contains.

Each plan node produces one ChildFilterDescription per child, so a
union pays this cost once per child. On a 250,000-column scan a
sampling profile put FilterRemapper::new at 794 of 20,373 samples,
almost entirely HashMap insertion and hashing. Return
ChildFilterDescription::empty() directly when parent_filters is empty,
before the remapper is built.
@github-actions github-actions Bot added the physical-plan Changes to the physical-plan crate label Aug 18, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.24%. Comparing base (c7be84a) to head (c145abc).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24428      +/-   ##
==========================================
- Coverage   81.24%   81.24%   -0.01%     
==========================================
  Files        1112     1112              
  Lines      390912   390924      +12     
  Branches   390912   390924      +12     
==========================================
  Hits       317604   317604              
- Misses      54670    54678       +8     
- Partials    18638    18642       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

physical-plan Changes to the physical-plan crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants