alamb commented on issue #11420: URL: https://github.com/apache/datafusion/issues/11420#issuecomment-2226253535
In theory I think you are right. However, I am worried about removing SessionState from the `scan` method as it is so widely used and has everything people need. In fact here is at least one request to use SessionState *more*: https://github.com/apache/datafusion/issues/11193 from @cisaacson Here is one alternate proposal https://github.com/apache/datafusion/issues/10782#issuecomment-2226248126 (basically treat SessionState as part of the catalog API). ANother potentially hacky idea is to do something like pass SessionState as an `dyn Any` -- ```rust async fn scan( &self, state: &Any, projection: Option<&Vec<usize>>, _filters: &[Expr], _limit: Option<usize>, ) -> Result<Arc<dyn ExecutionPlan>> { ``` Which would break the explicit dependency but implementors of `scan` could still get it by downcasting ```rust let state = state.as_any().downcast_ref::<SessionState>().unwrap(); ``` 🤔 -- 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
