adriangb commented on code in PR #17336:
URL: https://github.com/apache/datafusion/pull/17336#discussion_r2325523600
##########
datafusion/core/src/datasource/listing/table.rs:
##########
@@ -1166,6 +1166,22 @@ impl TableProvider for ListingTable {
filters: &[Expr],
limit: Option<usize>,
) -> Result<Arc<dyn ExecutionPlan>> {
+ let options = ScanArgs::default()
+ .with_projection(projection.cloned())
+ .with_filters(Some(filters.to_vec()))
+ .with_limit(limit);
+ Ok(self.scan_with_args(state, options).await?.plan())
+ }
Review Comment:
I think the biggest question here is how to handle backwards compatibility.
I implemented both methods in terms of the other which is nice because:
1. Existing users are not forced to implement `scan_with_args`
2. We can start using `scan_with_args` in the physical planner.
But if you implement neither I think you'll get a recursion error.
I also chose not to deprecate `scan()` to (1) avoid a bunch of code churn
mostly for us and (2) until we are confident that `scan_with_args` is a
workable evolution
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
