-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Set catalog_list
from outside for SessionState
.
#5277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set catalog_list
from outside for SessionState
.
#5277
Conversation
It appears there is a clippy failure on this PR https://github.com/apache/arrow-datafusion/actions/runs/4173309478/jobs/7225595775 |
Fixed, PTAL @alamb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution @MichaelScofield
I can't help but think the management of catalogs is somewhat in need of assistance in DataFusion at the moment.
DataFusion is fairly heavily focused on being a query engine and it grown some basic in memory catalog support as needed, and has a bunch of extension points to support other catalog implementations
However, I think the APIs for how to implement an external catalog are not well defined and there isn't a good example of doing so. I think it is fine to merge this PR as is to make things better for you and your project, but longer term we should really have better story for catalogs.
I will file a follow on ticket for discussion
I filed #5291 to track improvement in this area |
Benchmark runs are scheduled for baseline = 2b73119 and contender = d05647c. d05647c is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
* Set `catalog_list` from outside for `SessionState`. * fix: resolve PR comments
Which issue does this PR close?
Closes #.
Rationale for this change
catalog_list
inSessionState
was made private in #4764. That makescatalog_list
fixed toMemoryCatalogList
inside:https://github.com/apache/arrow-datafusion/blob/c0defeb7a074d286976240a3965600a77038459a/datafusion/core/src/execution/context.rs#L1475-L1480
If we are to provide another implementation for
catalog_list
, we can't.What changes are included in this PR?
Adds a new constructor for
SessionState
for injecting custom implementor forcatalog_list
.Are these changes tested?
Are there any user-facing changes?