Skip to content
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

LogicalPlan::TableScan should not depend on the physical plan #2247

Closed
Tracked by #474
andygrove opened this issue Apr 16, 2022 · 0 comments · Fixed by #2284
Closed
Tracked by #474

LogicalPlan::TableScan should not depend on the physical plan #2247

andygrove opened this issue Apr 16, 2022 · 0 comments · Fixed by #2284
Assignees
Labels
enhancement New feature or request

Comments

@andygrove
Copy link
Member

andygrove commented Apr 16, 2022

Is your feature request related to a problem or challenge? Please describe what you are trying to do.
The logical plan currently depends on the physical plan, which prevents us from moving the logical plan into its own crate alongside the logical expressions (which is needed in order to support subqueries, and also to better support the use case where DataFusion is being used just for SQL query planning and then using a different execution engine).

The reason that the logical plan depends on the physical plan is that LogicalPlan::TableScan has a reference to a TableProvider which has a scan method that returns ExecutionPlan.

Describe the solution you'd like
I think the cleanest solution is going to be to have TableScan refer to a TableProvider by name rather than containing a reference to the TableProvider directly. The LogicalPlanBuilder would then need to maintain some state and that would need to get passed along to the physical planner somehow.

Describe alternatives you've considered
Here is the original solution I was pursuing. It did not work out.

Split the TableProvider trait into two separate traits - TableProvider and TableScanProvider (which can extend TableProvider). We have no need to call the scan method when building or optimizing the logical plan.

We will need to add extra information, such as table_scan_provider_name (or maybe listing config?) to the TableScan struct so that the physical planner can lookup or create the TableScanProvider.

Additional context
This came up when attempting to add support for subqueries.

@andygrove andygrove added the enhancement New feature or request label Apr 16, 2022
@andygrove andygrove self-assigned this Apr 16, 2022
@andygrove andygrove changed the title The logical plan should not depend on the physical plan LogicalPlan::TableScan should not depend on the physical plan Apr 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
1 participant