findepi commented on PR #11516: URL: https://github.com/apache/datafusion/pull/11516#issuecomment-2238870621
> I think this is similar to minimum dependencies what I'm thinking. The only difference is that I propose `struct TableProviderContext`, but you have `trait CatalogSession` > > IMO struct is more sutiable for storing data, unlike trait that mainly used for sharing same behavior A publicly constructible struct is indeed different than a trait. Not sure we want that though, as it would limit evolvability. I thought about a trait, because of it resemblance to interfaces in other languages. To me seems suited for defining contracts -- and here we're defining a contract between core and the table providers. I don't feel strongly though, even less so given my little experience IMO a bigger question is what functionality does this new being provide. If we go into composability, I would want to think about different table providers as add-ons or plugins. Then, maybe FileFormatFactory could be moved our of core, as not inherently part of SQL execution? This question would imply whether FileFormatFactory is exposed in TableProviderContext/CatalogSession. cc @alamb it would be good to hear your opinion, especially that you created the https://github.com/apache/datafusion/issues/10782 issue, which this PR is in response to. -- 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
