You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "kosiew (via GitHub)" <gi...@apache.org> on 2025/05/26 02:13:32 UTC
[I] Simplify Filter Pushdown APIs for Better Maintainability and Developer Experience [datafusion]
kosiew opened a new issue, #16188:
URL: https://github.com/apache/datafusion/issues/16188
### Is your feature request related to a problem or challenge?
The current filter pushdown APIs in DataFusion (FilterPushdownPropagation, PredicateSupports, etc.) have grown organically but now appear convoluted and redundant. The complex layering of abstractions makes the filter pushdown mechanism difficult to understand, maintain, and extend.
Specific issues include:
- Multiple overlapping abstraction layers (PredicateSupport, PredicateSupports, FilterDescription, etc.)
- Redundant helper methods with inconsistent naming patterns (.unsupported(), .transparent(), .with_filters(), .with_updated_node(), .new_with_supported_check(), .collect_supported(), .is_all_supported(), etc.)
- Complex mental model requiring developers to track multiple states and transformations
- Lack of clear documentation about the conceptual model and flow
- Inconsistent naming conventions (e.g., all_supported creates new objects while make_supported transforms existing ones)
-
These issues increase the learning curve for new contributors and make maintenance more challenging for all developers.
### Describe the solution you'd like
Redesign the filter pushdown APIs with a focus on simplicity, consistency, and clarity:
1. Reduce abstraction layers: Consolidate the multiple wrappers into fewer, more focused data structures.
2. Consistent API patterns: Use clear naming conventions:
- with_* for non-mutating methods that return new objects
- mark_* for transformations
- collect_* for extraction methods
3. Simplified core data structures:
```rust
/// A predicate with its support status for pushdown
enum PredicateWithSupport {
Supported(Arc<dyn PhysicalExpr>),
Unsupported(Arc<dyn PhysicalExpr>),
}
/// Collection of predicates with clearly defined operations
struct Predicates {
// Core operations that are intuitive to use
// ...
}
/// Clear result type for pushdown operations
struct FilterPushdownResult<T> {
pushed_predicates: Vec<Arc<dyn PhysicalExpr>>,
retained_predicates: Vec<Arc<dyn PhysicalExpr>>,
updated_plan: Option<T>,
}
```
4. More declarative approach: Let execution plan nodes declare which predicates they support rather than relying on complex negotiation.
5. Better documentation: Add clear documentation about the mental model, flow, and expected usage patterns.
6. Test coverage: Ensure robust test coverage for the new APIs to prevent regressions.
This redesign should aim to reduce cognitive load for developers while maintaining all current functionality. It should also make future extensions to the filter pushdown system more straightforward.
### Describe alternatives you've considered
_No response_
### Additional context
_No response_
--
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.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: [I] Simplify Filter Pushdown APIs for Better Maintainability and Developer Experience [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on issue #16188:
URL: https://github.com/apache/datafusion/issues/16188#issuecomment-2991542430
> Is there a duplication with push_down_filter in the logical optimizer?
I don't think so. The logical optimizer pushes things down into the table scan phase, the physical optimizer rule pushes them down into the actual physical scan, e.g. parquet accepts pushdown but CSV does not (I think).
But I hear you!
Thinking about it a bit I think what would make the most sense is that the logical optimizer pushes down into the TableScan and then the TableProvider attaches them to the DataSourceExec that it creates. That's kind of how it was before, but it created a lot of duplication and special casing. See https://github.com/apache/datafusion/pull/15769. The main con of this approach is that implementing it correctly is quite cumbersome and makes custom TableProvider's more complex. Having that happen as an optimizer rule keeps the complexity out of the TableProvider. But maybe we should have made it work with a refactor to TableProvider e.g. to return not only an ExecutionPlan but also filter pushdown results or something.
--
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: [I] Simplify Filter Pushdown APIs for Better Maintainability and Developer Experience [datafusion]
Posted by "xudong963 (via GitHub)" <gi...@apache.org>.
xudong963 commented on issue #16188:
URL: https://github.com/apache/datafusion/issues/16188#issuecomment-2994977304
@adriangb @alamb Thank you, now I understand the background about the filter pushdown in logical & physical phases.
> The main con of having TableProvider attach the filters to the scan is that implementing it correctly is quite cumbersome and makes custom TableProvider's more complex, with TableProvider being one of the most common hooks in DataFusion. Having that happen as an optimizer rule keeps the complexity out of the TableProvider.
I see, we should keep custom TableProviders easy to implement and reduce the users' burden. It's fair to keep the current status. Thanks for your detailed explanation.
--
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: [I] Simplify Filter Pushdown APIs for Better Maintainability and Developer Experience [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on issue #16188:
URL: https://github.com/apache/datafusion/issues/16188#issuecomment-2917056155
Thank you @kosiew.
Clearly what we have now needs work but I think I'd like to defer cleaning this up until some other folks try to implement more things with these APIs (e.g. join filter pushdown) which will give us both:
1. More ideas / brains behind API design.
2. More use cases to validate that the design is correct.
@alamb does that sound reasonable? It means any changes are more of a breaking change but since these APIs are very new and complex I think it's reasonable to have a bit of churn before they stabilize.
--
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: [I] Simplify Filter Pushdown APIs for Better Maintainability and Developer Experience [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #16188:
URL: https://github.com/apache/datafusion/issues/16188#issuecomment-2998026890
> FYI, I added more doc for the related code based on my understanding: [#16504](https://github.com/apache/datafusion/pull/16504)
That writeup is 🧑🍳 👌 really nice
--
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: [I] Simplify Filter Pushdown APIs for Better Maintainability and Developer Experience [datafusion]
Posted by "xudong963 (via GitHub)" <gi...@apache.org>.
xudong963 commented on issue #16188:
URL: https://github.com/apache/datafusion/issues/16188#issuecomment-2990200877
@kosiew 's suggestions look good to me, I have one another question:
**FilterPushdownPhase::Pre**
IMO, `FilterPushdownPhase::Pre` runs before most other physical optimizations, It only handles "static filters that do not reference any ExecutionPlans", this type of static filter pushdown should logically belong in the logical optimization phase.
Is there a duplication with `push_down_filter` in the logical optimizer?
--
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: [I] Simplify Filter Pushdown APIs for Better Maintainability and Developer Experience [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #16188:
URL: https://github.com/apache/datafusion/issues/16188#issuecomment-2917159985
In general I agree with the premise that making the filter pushdown APIs easier to use / understand would be very valuable to DataFusion -- the goals @kosiew describe all sound wonderful to me
I think the key question would be exactly what the new API would look like and how much API churn it would entail
I do think getting more ideas would be valuable and I would enjoy reviewing proposals. As @adriangb mentions, one thing that might help drive / force these API changes is trying to add new features.
--
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: [I] Simplify Filter Pushdown APIs for Better Maintainability and Developer Experience [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew closed issue #16188: Simplify Filter Pushdown APIs for Better Maintainability and Developer Experience
URL: https://github.com/apache/datafusion/issues/16188
--
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: [I] Simplify Filter Pushdown APIs for Better Maintainability and Developer Experience [datafusion]
Posted by "xudong963 (via GitHub)" <gi...@apache.org>.
xudong963 commented on issue #16188:
URL: https://github.com/apache/datafusion/issues/16188#issuecomment-2995081666
FYI, I added more doc for the related code based on my understanding: https://github.com/apache/datafusion/pull/16504/
--
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: [I] Simplify Filter Pushdown APIs for Better Maintainability and Developer Experience [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #16188:
URL: https://github.com/apache/datafusion/issues/16188#issuecomment-2992278637
> Is there a duplication with push_down_filter in the logical optimizer?
In my opinion there is duplication in the sense that they both do similar things (push filters down in the plan) but for the reasons that @adriangb mentions we can't combine them
It is interesting that InfluxData also ended up with its own physical rule to push filters down (in our case for other reasons)
Thus I think it is inevitable that we'll have similar functonality in both the Logical and Physical plan layers
--
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: [I] Simplify Filter Pushdown APIs for Better Maintainability and Developer Experience [datafusion]
Posted by "xudong963 (via GitHub)" <gi...@apache.org>.
xudong963 commented on issue #16188:
URL: https://github.com/apache/datafusion/issues/16188#issuecomment-2919458473
I still don't get a chance to read the whole Filter pushdown APIs, but will do in a week, then will give some feedback.
--
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