Skip to content

fix: FilterExec should drop projection when apply projection pushdown#21460

Merged
alamb merged 5 commits intoapache:mainfrom
haohuaijin:fix-projection-pushdown
Apr 8, 2026
Merged

fix: FilterExec should drop projection when apply projection pushdown#21460
alamb merged 5 commits intoapache:mainfrom
haohuaijin:fix-projection-pushdown

Conversation

@haohuaijin
Copy link
Copy Markdown
Contributor

@haohuaijin haohuaijin commented Apr 8, 2026

Which issue does this PR close?

Rationale for this change

When a ProjectionExec sits on top of a FilterExec that already carries an explicit projection, the ProjectionPushdown optimizer attempts to swap them via try_swapping_with_projection. The swap replaces the FilterExec's input with the narrower ProjectionExec, but FilterExecBuilder::from(self) carried over the old projection indices (e.g. [0, 1, 2]). After the swap the new input only has the columns selected by the ProjectionExec (e.g. 2 columns), so .build() tries to validate the stale projection against the narrower schema and panics with "project index 2 out of bounds, max field 2".

What changes are included in this PR?

In FilterExec::try_swapping_with_projection, after replacing the input with the narrower ProjectionExec, clear the FilterExec's own projection via .apply_projection(None). The ProjectionExec that is now the input already handles column selection, so the FilterExec no longer needs its own projection.

Are these changes tested?

yes, add test case

Are there any user-facing changes?

@github-actions github-actions Bot added the physical-plan Changes to the physical-plan crate label Apr 8, 2026
@alamb alamb added the regression Something that used to work no longer does label Apr 8, 2026
Copy link
Copy Markdown
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks @haohuaijin

the fix makes sense to me, but I think it would be better to try and add some higher level tests if possible

Comment thread datafusion/physical-plan/src/filter.rs
@github-actions github-actions Bot added the core Core DataFusion crate label Apr 8, 2026
@alamb alamb added this pull request to the merge queue Apr 8, 2026
@alamb
Copy link
Copy Markdown
Contributor

alamb commented Apr 8, 2026

Thanks @haohuaijin

Merged via the queue into apache:main with commit 91c2e04 Apr 8, 2026
35 checks passed
@haohuaijin haohuaijin deleted the fix-projection-pushdown branch April 9, 2026 01:57
@haohuaijin
Copy link
Copy Markdown
Contributor Author

Thanks for your quick reviews @alamb

@alamb
Copy link
Copy Markdown
Contributor

alamb commented Apr 9, 2026

Gotta love the bug fixes. THanks @haohuaijin

Cna you please make a backport for this issue to branch-53 if you want to include it in the 53 release?

@haohuaijin
Copy link
Copy Markdown
Contributor Author

haohuaijin commented Apr 9, 2026

Cna you please make a backport for this issue to branch-53 if you want to include it in the 53 release?

@alamb, thanks @comphead, we already have the backport for this pr #21492, although the pr does not include the tests I would added later in 4937804, but i think it is fine.

@alamb
Copy link
Copy Markdown
Contributor

alamb commented Apr 9, 2026

Cna you please make a backport for this issue to branch-53 if you want to include it in the 53 release?

@alamb, thanks @comphead, we already have the backport for this pr #21492, although the pr does not include the tests I would added later in 4937804, but i think it is fine.

I agree -- it is fine that the test is not included

Rich-T-kid pushed a commit to Rich-T-kid/datafusion that referenced this pull request Apr 21, 2026
…apache#21460)

## Which issue does this PR close?

<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax. For example
`Closes apache#123` indicates that this PR will close issue apache#123.
-->

- Closes apache#21459 

## Rationale for this change

When a `ProjectionExec` sits on top of a `FilterExec` that already
carries an explicit projection, the `ProjectionPushdown` optimizer
attempts to swap them via `try_swapping_with_projection`. The swap
replaces the `FilterExec's` input with the narrower `ProjectionExec`,
but `FilterExecBuilder::from(self)` carried over the old projection
indices (e.g. [0, 1, 2]). After the swap the new input only has the
columns selected by the `ProjectionExec` (e.g. 2 columns), so .build()
tries to validate the stale projection against the narrower schema and
panics with "project index 2 out of bounds, max field 2".

## What changes are included in this PR?

In `FilterExec::try_swapping_with_projection`, after replacing the input
with the narrower ProjectionExec, clear the FilterExec's own projection
via .`apply_projection(None)`. The ProjectionExec that is now the input
already handles column selection, so the FilterExec no longer needs its
own projection.



## Are these changes tested?

yes, add test case

## Are there any user-facing changes?

<!--
If there are user-facing changes then we may require documentation to be
updated before approving the PR.
-->

<!--
If there are any breaking changes to public APIs, please add the `api
change` label.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate physical-plan Changes to the physical-plan crate regression Something that used to work no longer does

Projects

None yet

Development

Successfully merging this pull request may close these issues.

regression: ProjectionPushdown failed with project index 2 out of bounds, max field 2

2 participants