fix: update comment on FilterPushdownPropagation #20040
Open
+1
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
masonh22
approved these changes
Jan 28, 2026
Contributor
masonh22
left a comment
masonh22
left a comment
There was a problem hiding this comment.
Looks good, I just have one small comment
| #[derive(Debug, Clone)] | ||
| pub struct FilterPushdownPropagation<T> { | ||
| /// What filters were pushed into the parent node. | ||
| /// Which parent filters were pushed down into this node's children. |
Contributor
There was a problem hiding this comment.
I'm not super familiar with the physical optimizer filter pushdown rule, but push_down_filters() in physical-optimizer/src/filter_pushdown.rs specifically distinguishes between "parent" filters and "self" filters, which are both contained in FilterPushdownPropagation::filters.
So I think the comment should be "Which filters were pushed down..." instead of "Which parent filters were pushed down..."
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
No
Rationale for this change
I think the original comment is misleading as we actually want to express if a parent filter was pushed down to any child successfully.
What changes are included in this PR?
Updated the comment about the
filtersin theFilterPushdownPropagationstruct.Are these changes tested?
No need to test as it only modifies comments.
Are there any user-facing changes?
Yes, but only touches the comment.