colinmarc commented on code in PR #16750:
URL: https://github.com/apache/datafusion/pull/16750#discussion_r2202889418
##########
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:
It's not strictly necessary to keep it. My original plan was to remove
`source_as_provider` and replace it with this method. I thought that it should
be associated with `DefaultTableSource`, since that's actually what's being
used to wrap/unwrap here.
Then I noticed that doing both downcasts in one function avoided adding an
extra indentation level in the `to_proto` logic, e.g.
```rust
if let Some(provider) = source_as_provider(source) {
if let Some(listing_table) = provider.downcast_ref::<ListingTable>() {
// ...
}
}
```
So I kept both. If you'd like I can do the following:
- 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)
- Move `provider_as_source` to `DefaultTableSource::wrap`
- Document both with testable examples.
--
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
