Add insert/update/delete to LogicalPlan and add SQL planner support #4902

avantgardnerio · 2023-01-13T20:59:16Z

Which issue does this PR close?

Closes #4901.

Rationale for this change

Described in issue.

What changes are included in this PR?

A new LogicalPlan enum variant called WriteRel and modeled after Substrait
Changes to the planner to allow it to generate the new plans
Integration tests for insert/update/delete

Are these changes tested?

Yes

Are there any user-facing changes?

Only if the user is a programmer using DataFusion as a library, in which case more ASTs will convert to LogicalPlans.

avantgardnerio · 2023-01-13T21:00:11Z

@andygrove @alamb I'd love to hear your thoughts on this.

datafusion/expr/src/logical_plan/plan.rs

datafusion/sql/src/statement.rs

datafusion/sql/tests/integration_test.rs

alamb

I think this is looking quite nice @avantgardnerio -- I need to review it when I have a bit more brain power.

My biggest reaction is to the need to start namespacing the LogicalPlan a bit more -- but the WriteRel is a good model to follow (e.g. for DML)

datafusion/core/src/physical_plan/planner.rs

datafusion/expr/src/logical_plan/plan.rs

datafusion/sql/src/statement.rs

avantgardnerio · 2023-01-16T19:49:03Z

datafusion/sql/tests/integration_test.rs

+    let sql = "delete from person where id=1";
+    let plan = r#"
+Write: op=[Delete] table=[person]
+  Filter: id = Int64(1)


No projection needed for delete I assume?

Makes sense

andygrove

LGTM. Thanks @avantgardnerio

alamb · 2023-01-17T20:44:16Z

datafusion/core/src/physical_plan/planner.rs

@@ -1180,6 +1180,12 @@ impl DefaultPhysicalPlanner {
                        "Unsupported logical plan: CreateView".to_string(),
                    ))
                }
+                LogicalPlan::Dml(_) => {


alamb

Looks good to me -- thank you @avantgardnerio

alamb · 2023-01-17T20:45:08Z

datafusion/sql/src/statement.rs

+                        "Insert-returning clause not yet supported".to_owned(),
+                    ))?;
+                }
+                let _ = into; // optional keyword doesn't change behavior


ursabot · 2023-01-17T20:54:26Z

Benchmark runs are scheduled for baseline = 279440b and contender = aa8f139. aa8f139 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

github-actions bot added core Core datafusion crate logical-expr Logical plan and expressions optimizer Optimizer rules sql labels Jan 13, 2023