Remove dependency from LogicalPlan::TableScan to ExecutionPlan
#2284
Conversation
andygrove
changed the title
LogicalPlan::TableScan to ExecutionPlan
|
@matthewmturner @alamb I aborted the large refactor and found a simpler and more practical approach. Let me know what you think. |
|
I plan to review this later today |
|
The last comment from @jimexist during "the great split of datafusion" was is #1762 (comment) -- I think this PR will help |
| } | ||
|
|
||
| /// Extract TableProvider from TableSource | ||
| pub fn source_as_provider( |
There was a problem hiding this comment.
Maybe it is worth a comment on TableSource that it is not intended, initially, to be extended outside of DataFusion.
datafusion/expr/src/table_source.rs
Outdated
| Temporary, | ||
| } | ||
|
|
||
| /// TableSource |
There was a problem hiding this comment.
It is probably worth some comments here about the rationale for this structure existing -- namely so LogicalPlan doesn't directly rely on physical plans
Co-authored-by: Andrew Lamb <[email protected]>
…datafusion into table-provider-refactor
|
Thanks for the review @alamb. I pushed an update with more documentation. Let me know if I missed anything and I can address in one of the following refactor PRs. |
Which issue does this PR close?
Closes #2247
Rationale for this change
We want to remove the dependency from
TableScantoExecutionPlanso that we can move theLogicalPlaninto theexprcrate.What changes are included in this PR?
TableSourcetrait inexprcrate that is referenced fromTableScanDefaultTableSourcestruct in core crate that wrapsTableProviderAre there any user-facing changes?
Yes, at API level