timsaucer opened a new issue, #23348: URL: https://github.com/apache/datafusion/issues/23348
### Is your feature request related to a problem or challenge? When using `datafusion-ffi` I want to create a table function that returns a view on an existing table. I can do this outside of an FFI context because I can take the `&dyn Session` within the table function arguments and downcast it to a `SessionState`. I can then access the catalog. In an FFI context, I cannot downcast the `Session` to `SessionState` because of the well known unstable ABI issue. What I would like to be able to do is to access the catalog via `Session` trait methods. I believe this is consistent with the path the project wishes to take (see section below on additional context). ### Describe the solution you'd like We currently have a circular dependency if we were to try exposing any catalog methods, because `datafusion-catalog` depends on `datafusion-session` both for the fact that we need a `&dyn Session` for `scan()` in a table provider as well as we need `&dyn Session` for table functions. We would need to break this circular dependency. We could do this by moving catalog methods to `trait Session` and have them return `Arc<&dyn Any>` and then rely on the downstream users to downcast them to `CatalogProviderList`. This seems awkward to me. I would prefer if we were to hoist these trait definitions into the `datafusion-session` crate. ### Describe alternatives you've considered Right now the work around is complex. Either you have to be able to downcast to `SessionState`, which is done frequently or you need to do some other more complex way to get the same catalogs out. The `datafusion-python` project can do some things like calling into the python code to access `.catalog()` but this is not ideal at all. ### Additional context ### Pre-existing code comment justification There is the following comment [in the code](https://github.com/apache/datafusion/blob/0365d3c2631eea73f2889c197506fdfb3895fd3e/datafusion/session/src/session.rs#L50) that suggests the best path forward is to expose more of `SessionState` on the trait. ``` /// # Migration from `SessionState` /// /// Using trait methods is preferred, as the implementation may change in future /// versions. However, you can downcast a `Session` to a `SessionState` as shown /// in the example below. If you find yourself needing to do this, please open /// an issue on the DataFusion repository so we can extend the trait to provide /// the required information. ``` ### Relation to Function Registry I believe this topic is also a blocker for https://github.com/apache/datafusion/issues/15095 . If we want to support that instead then we'd need to hoist the trait definitions up even higher into `datafusion-expr` or maybe even consider a crate that simply has trait definitions. That feels not ideal either, where it's nice to have the traits live closer to their implementations. ### Prior conversation This topic has been addressed in a similar way in https://github.com/apache/datafusion/issues/11420 where there was discussion of passing `SessionState` around. Some of the comments in that issue are stale, but the general gist I think is along the same lines. -- 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]
