You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "Mark1626 (via GitHub)" <gi...@apache.org> on 2026/09/06 14:45:51 UTC
[PR] feat: support filter pushdown through UNNEST operator [datafusion]
Mark1626 opened a new pull request, #24984:
URL: https://github.com/apache/datafusion/pull/24984
## Rationale for this change
I have a couple of queries which have Unnest in them, I noticed that the perf of these queries can be improved by pushing down filters to the scan. Filters which are not part of the Unnest operator can be push-down to the scan
## What changes are included in this PR?
Implements `gather_filters_for_pushdown` and `handle_child_pushdown_result` for the Unnest Operator
## What is the testing strategy for this PR?
Unit tests have been added in the PR. I have manually tested this against a couple of queries with `SET datafusion.optimizer.enable_dynamic_filter_pushdown = true;`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support filter pushdown through UNNEST operator [datafusion]
Posted by "ryux1 (via GitHub)" <gi...@apache.org>.
ryux1 commented on PR #24984:
URL: https://github.com/apache/datafusion/pull/24984#issuecomment-5623398395
Nothing else is needed from my review. The index-based mapping and the two collision regressions address the concern I raised. The remaining cancelled hash-collision job is not a code issue introduced by this change; maintainer review is the next step. I would wait for the normal review queue rather than tagging unrelated reviewers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support filter pushdown through UNNEST operator [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #24984:
URL: https://github.com/apache/datafusion/pull/24984#issuecomment-5560137333
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/24984?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `97.43590%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 81.67%. Comparing base ([`33028d5`](https://app.codecov.io/gh/apache/datafusion/commit/33028d5543ed2fd2ad19bd247888cfb3c55cec3b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`5bfdf5e`](https://app.codecov.io/gh/apache/datafusion/commit/5bfdf5e608edca13f127870a50c8590b4f3eeddb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 15 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/24984?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [datafusion/physical-plan/src/unnest.rs](https://app.codecov.io/gh/apache/datafusion/pull/24984?src=pr&el=tree&filepath=datafusion%2Fphysical-plan%2Fsrc%2Funnest.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1wbGFuL3NyYy91bm5lc3QucnM=) | 97.43% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/24984?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #24984 +/- ##
==========================================
+ Coverage 81.61% 81.67% +0.05%
==========================================
Files 1124 1126 +2
Lines 412077 414563 +2486
Branches 412077 414563 +2486
==========================================
+ Hits 336318 338581 +2263
- Misses 55949 56065 +116
- Partials 19810 19917 +107
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/24984?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support filter pushdown through UNNEST operator [datafusion]
Posted by "ryux1 (via GitHub)" <gi...@apache.org>.
ryux1 commented on code in PR #24984:
URL: https://github.com/apache/datafusion/pull/24984#discussion_r3948831996
##########
datafusion/physical-plan/src/unnest.rs:
##########
@@ -328,6 +333,57 @@ impl ExecutionPlan for UnnestExec {
Some(self.metrics.clone_inner())
}
+ fn gather_filters_for_pushdown(
+ &self,
+ _phase: FilterPushdownPhase,
+ parent_filters: Vec<Arc<dyn PhysicalExpr>>,
+ _config: &ConfigOptions,
+ ) -> Result<FilterDescription> {
+ // Filters that reference only non-unnested (passthrough) columns commute
+ // with the unnest. Non-unnest values values are replicated onto every row
+ // produced from an input row, so dropping an input row before the unnest
+ // removes exactly the output rows the filter would have dropped after it.
+ // Filters referencing unnested list/struct columns must stay above this
+ // node.
+ let input_schema = self.input.schema();
+ let unnested_input_indices: HashSet<usize> = self
+ .list_column_indices
+ .iter()
+ .map(|list_unnest| list_unnest.index_in_input_schema)
+ .chain(self.struct_column_indices.iter().copied())
+ .collect();
+
+ // Output indices of passthrough columns, resolved by name the same way
+ // `compute_properties` builds its projection mapping.
+ let allowed_output_indices: std::collections::HashSet<usize> =
+ (0..input_schema.fields().len())
+ .filter(|input_idx| !unnested_input_indices.contains(input_idx))
+ .filter_map(|input_idx| {
+ let name = input_schema.field(input_idx).name();
+ self.schema
+ .fields()
+ .iter()
+ .position(|output_field| output_field.name() == name)
Review Comment:
The positional mapping and the two collision regressions address the issue I raised. Thanks for also fixing the existing equivalence-property mapping.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support filter pushdown through UNNEST operator [datafusion]
Posted by "ryux1 (via GitHub)" <gi...@apache.org>.
ryux1 commented on code in PR #24984:
URL: https://github.com/apache/datafusion/pull/24984#discussion_r3944377733
##########
datafusion/physical-plan/src/unnest.rs:
##########
@@ -328,6 +333,57 @@ impl ExecutionPlan for UnnestExec {
Some(self.metrics.clone_inner())
}
+ fn gather_filters_for_pushdown(
+ &self,
+ _phase: FilterPushdownPhase,
+ parent_filters: Vec<Arc<dyn PhysicalExpr>>,
+ _config: &ConfigOptions,
+ ) -> Result<FilterDescription> {
+ // Filters that reference only non-unnested (passthrough) columns commute
+ // with the unnest. Non-unnest values values are replicated onto every row
+ // produced from an input row, so dropping an input row before the unnest
+ // removes exactly the output rows the filter would have dropped after it.
+ // Filters referencing unnested list/struct columns must stay above this
+ // node.
+ let input_schema = self.input.schema();
+ let unnested_input_indices: HashSet<usize> = self
+ .list_column_indices
+ .iter()
+ .map(|list_unnest| list_unnest.index_in_input_schema)
+ .chain(self.struct_column_indices.iter().copied())
+ .collect();
+
+ // Output indices of passthrough columns, resolved by name the same way
+ // `compute_properties` builds its projection mapping.
+ let allowed_output_indices: std::collections::HashSet<usize> =
+ (0..input_schema.fields().len())
+ .filter(|input_idx| !unnested_input_indices.contains(input_idx))
+ .filter_map(|input_idx| {
+ let name = input_schema.field(input_idx).name();
+ self.schema
+ .fields()
+ .iter()
+ .position(|output_field| output_field.name() == name)
Review Comment:
This name-based lookup can make an unnested output incorrectly eligible when generated and passthrough fields collide. For example, if the input has struct `s { f1 }` before a passthrough field physically named `s.f1`, the output can contain two `s.f1` fields; `position` selects the generated one, so a filter on that generated index is accepted and `FilterRemapper` maps it to the unrelated passthrough input field. Physical expressions are index-based precisely so duplicate names can be represented. Could this mapping be derived positionally from the list/struct expansion (or reject ambiguous matches), with a collision regression test?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support filter pushdown through UNNEST operator [datafusion]
Posted by "Mark1626 (via GitHub)" <gi...@apache.org>.
Mark1626 commented on code in PR #24984:
URL: https://github.com/apache/datafusion/pull/24984#discussion_r3946725666
##########
datafusion/physical-plan/src/unnest.rs:
##########
@@ -328,6 +333,57 @@ impl ExecutionPlan for UnnestExec {
Some(self.metrics.clone_inner())
}
+ fn gather_filters_for_pushdown(
+ &self,
+ _phase: FilterPushdownPhase,
+ parent_filters: Vec<Arc<dyn PhysicalExpr>>,
+ _config: &ConfigOptions,
+ ) -> Result<FilterDescription> {
+ // Filters that reference only non-unnested (passthrough) columns commute
+ // with the unnest. Non-unnest values values are replicated onto every row
+ // produced from an input row, so dropping an input row before the unnest
+ // removes exactly the output rows the filter would have dropped after it.
+ // Filters referencing unnested list/struct columns must stay above this
+ // node.
+ let input_schema = self.input.schema();
+ let unnested_input_indices: HashSet<usize> = self
+ .list_column_indices
+ .iter()
+ .map(|list_unnest| list_unnest.index_in_input_schema)
+ .chain(self.struct_column_indices.iter().copied())
+ .collect();
+
+ // Output indices of passthrough columns, resolved by name the same way
+ // `compute_properties` builds its projection mapping.
+ let allowed_output_indices: std::collections::HashSet<usize> =
+ (0..input_schema.fields().len())
+ .filter(|input_idx| !unnested_input_indices.contains(input_idx))
+ .filter_map(|input_idx| {
+ let name = input_schema.field(input_idx).name();
+ self.schema
+ .fields()
+ .iter()
+ .position(|output_field| output_field.name() == name)
Review Comment:
Thanks, I missed looking at that. I'll modify the logic to use the indices rather than the name, also adding a regression test as suggested
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: support filter pushdown through UNNEST operator [datafusion]
Posted by "Mark1626 (via GitHub)" <gi...@apache.org>.
Mark1626 commented on PR #24984:
URL: https://github.com/apache/datafusion/pull/24984#issuecomment-5613403550
@ryux1 Is there anything else needed in this PR? Should I tag someone else for a review?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org