alamb commented on code in PR #16750:
URL: https://github.com/apache/datafusion/pull/16750#discussion_r2205627323


##########
datafusion/catalog/src/default_table_source.rs:
##########
@@ -45,6 +45,26 @@ impl DefaultTableSource {
     pub fn new(table_provider: Arc<dyn TableProvider>) -> Self {
         Self { table_provider }
     }
+
+    /// Attempt to downcast a TableSource to DefaultTableSource and access the
+    /// TableProvider. This will only work with a TableSource created by 
DataFusion.

Review Comment:
   I think we should try to keep everything consistent as the dichotomy between 
TableSource and TableProvider is already confusing enough
   
   How about this for a proposal:
   1. We keep the new DefaultTableSource method
   2. We add the upgrade guide note on the changes to API
   3. We file a follow on ticket to improve the API (I can help here, and I am 
sure someone would do the code change if we file the ticket -- it doesn't have 
to be you)
   
   
   As a follow on PR we can then discuss consolidating / removing 
`source_as_provider`
   
   
   
   > Get rid of source_as_provider (it's only used in one place, where an 
explicit error might be better - trying to convert a logical scan of a 
TableSource into a physical plan should be an error)
   
   Can we please deprecate it instead like 
https://datafusion.apache.org/contributor-guide/api-health.html#deprecation-guidelines
   
   > Change calls to provider_as_source(x) to 
Arc::new(DefaultTableSource::new(x))
   
   Sounds good -- or maybe we could make a method on `DefaultTableSource` like 
`DefaultTableSource::provider_as_source` to people didn't have to wrap it in an 
arc 🤔 
   
   Likewise, if you make this change please rename `provider_as_source`
   
   > Document both with testable examples.
   
   Yes please
   



-- 
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-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to