Add DataFrame::into_view instead of implementing TableProvider (#2659)#4778
Add DataFrame::into_view instead of implementing TableProvider (#2659)#4778tustvold merged 2 commits intoapache:masterfrom
Conversation
github-actions
bot
added
core
|
I originally thought about resolving this by moving CatalogList off SessionState and onto SessionContext, but this created a huge amount of churn, this seems relatively harmless by comparison. |
|
|
||
| #[async_trait] | ||
| impl TableProvider for DataFrame { | ||
| impl TableProvider for DataFrameTableProvider { |
There was a problem hiding this comment.
Where is the ref cycle that this change fixing? I think I am missing something here
I see the cycle happens when the dataframe is registered as a table provider on the same context
I think this makes sense to me
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
cc @Dandandan as I think he was the one who added this API at first
|
What is the current thinking about this PR? It has conflicts. Shall we polish it up and merge? It seems like the bug is real enough |
|
Benchmark runs are scheduled for baseline = 4bea81b and contender = c5e2594. c5e2594 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
3 participants
Which issue does this PR close?
Closes #2659
Rationale for this change
The previous version introduced a ref-cycle as the
DataFrameowns a copy of theCatalogListthat in turn owns the registeredDataFrameWhat changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?