Uh oh!
There was an error while loading. Please reload this page.
feat: add basic MemTable MERGE INTO support - #24195
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@## main #24195 +/- ##
==========================================
- Coverage 81.23% 81.23% -0.01%
==========================================
Files 1111 1111 Lines 390208 390715 +507 Branches 390208 390715 +507 ==========================================
+ Hits 316990 317383 +393 - Misses 54591 54653 +62 - Partials 18627 18679 +52 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
It looks like this will mutate the table even when we don't execute. If you run these commands in datafusion-cli you'll see:
CREATETABLEdestination(id INT, val VARCHAR);
INSERT INTO destination VALUES (1, 'a'), (2, 'b'), (3, 'c');
CREATETABLEsource(id INT, val VARCHAR);
INSERT INTO source VALUES (2, 'B');
SELECT*FROM destination ORDER BY id;
EXPLAIN MERGE INTO destination USING source ONdestination.id=source.id
WHEN MATCHED THEN UPDATESET val =source.val;
SELECT*FROM destination ORDER BY id;Yields this output:
0 row(s) fetched.
Elapsed 0.024 seconds.
+-------+
| count |
+-------+
| 3 |
+-------+
1 row(s) fetched.
Elapsed 0.018 seconds.
0 row(s) fetched.
Elapsed 0.000 seconds.
+-------+
| count |
+-------+
| 1 |
+-------+
1 row(s) fetched.
Elapsed 0.000 seconds.
+----+-----+
| id | val |
+----+-----+
| 1 | a |
| 2 | b |
| 3 | c |
+----+-----+
3 row(s) fetched.
Elapsed 0.006 seconds.
+---------------+-------------------------------+
| plan_type | plan |
+---------------+-------------------------------+
| physical_plan | ┌───────────────────────────┐ |
| | │ CooperativeExec │ |
| | │ -------------------- │ |
| | │ CooperativeExec │ |
| | └─────────────┬─────────────┘ |
| | ┌─────────────┴─────────────┐ |
| | │ DmlResultExec │ |
| | │ -------------------- │ |
| | │ DmlResultExec: │ |
| | │ rows_affected │ |
| | │ : │ |
| | │ 1 │ |
| | └───────────────────────────┘ |
| | |
+---------------+-------------------------------+
1 row(s) fetched.
Elapsed 0.003 seconds.
+----+-----+
| id | val |
+----+-----+
| 1 | a |
| 2 | B |
| 3 | c |
+----+-----+
3 row(s) fetched.
Elapsed 0.000 seconds.
The big difference I want to point out is in the two select commands. In the first one we have values a, b, c and in the second we have a, B, c so the table was mutated even though we only did an explain plan. I didn't expect explain to actually do the mutation.
Now I think this is a pre-existing issue. In fact if you run an insert command you have similar problems. Starting in a fresh cli:
INSERT INTO t2 VALUES (1), (2);
SELECT*FROM t2;
EXPLAIN UPDATE t2 SET a =99;
SELECT*FROM t2;Yields:
0 row(s) fetched.
Elapsed 0.024 seconds.
+-------+
| count |
+-------+
| 2 |
+-------+
1 row(s) fetched.
Elapsed 0.018 seconds.
+---+
| a |
+---+
| 1 |
| 2 |
+---+
2 row(s) fetched.
Elapsed 0.002 seconds.
+---------------+-------------------------------+
| plan_type | plan |
+---------------+-------------------------------+
| physical_plan | ┌───────────────────────────┐ |
| | │ CooperativeExec │ |
| | │ -------------------- │ |
| | │ CooperativeExec │ |
| | └─────────────┬─────────────┘ |
| | ┌─────────────┴─────────────┐ |
| | │ DmlResultExec │ |
| | │ -------------------- │ |
| | │ DmlResultExec: │ |
| | │ rows_affected │ |
| | │ : │ |
| | │ 2 │ |
| | └───────────────────────────┘ |
| | |
+---------------+-------------------------------+
1 row(s) fetched.
Elapsed 0.003 seconds.
+----+
| a |
+----+
| 99 |
| 99 |
+----+
2 row(s) fetched.
Elapsed 0.000 seconds.
So again we're getting mutation of the data during an explain. I'm not sure if we want to merge as is, make a change, or open an issue. Do others have thoughts? Is this a well known and accepted approach - mutation on explain?
Which issue does this PR close?
Follow-up to the MERGE INTO review in #22988. No separate issue.
Rationale for this change
MERGE INTOcould be planned, but the built-in in-memory table provider could not execute it, andEXPLAINonly showedop=[MergeInto]without theONcondition orWHENactions. This made the new SQL surface hard to exercise end-to-end with the default provider.What changes are included in this PR?
ON/WHEN ... THEN ...details in text and JSON logical plan output.MemTableMERGE INTOexecution for matched update/delete, not-matched insert, and not-matched-by-source update/delete.MERGE INTOsyntax, provider caveats, and currently unsupported planner syntax.Are these changes tested?
Yes:
cargo fmt --allcargo test -p datafusion-exprcargo test -p datafusion-catalogcargo test -p datafusion --test core_integration merge_into -- --nocapturecargo test -p datafusion-sql plan_merge_into_canonicalizes_qualifiers_and_preserves_quoted_columns -- --nocapturecargo test --profile=ci --test sqllogictests -- merge_into.slt --nocapture./ci/scripts/doc_prettier_check.sh --write --allow-dirtycargo clippy --all-targets --all-features -- -D warningsNote: the plan mentioned
cargo test -p datafusion --test sql, but this repository does not currently have asqltest target; the relevant MERGE SQL tests live undercore_integration.Are there any user-facing changes?
Yes.
MemTablenow supports basicMERGE INTO, andEXPLAINoutput forMERGE INTOincludes the merge condition and clauses. There are no public API signature changes.