[SPARK-32030][SPARK-32127][SQL] Support unlimited MATCHED and NOT MATCHED clauses in MERGE INTO #28875

xy-xin · 2020-06-20T01:11:43Z

What changes were proposed in this pull request?

This PR add unlimited MATCHED and NOT MATCHED clauses in MERGE INTO statement.

Why are the changes needed?

Now the MERGE INTO syntax is,

MERGE INTO [db_name.]target_table [AS target_alias]
 USING [db_name.]source_table [<time_travel_version>] [AS source_alias]
 ON <merge_condition>
 [ WHEN MATCHED [ AND <condition> ] THEN <matched_action> ]
 [ WHEN MATCHED [ AND <condition> ] THEN <matched_action> ]
 [ WHEN NOT MATCHED [ AND <condition> ] THEN <not_matched_action> ]

It would be nice if we support unlimited MATCHED and NOT MATCHED clauses in MERGE INTO statement, because users may want to deal with different "AND "s, the result of which just like a series of "CASE WHEN"s. The expected syntax looks like

MERGE INTO [db_name.]target_table [AS target_alias]
 USING [db_name.]source_table [<time_travel_version>] [AS source_alias]
 ON <merge_condition>
 [when_matched_clause [, ...]]
 [when_not_matched_clause [, ...]]

where when_matched_clause is

WHEN MATCHED [ AND <condition> ] THEN <matched_action>

and when_not_matched_clause is

WHEN NOT MATCHED [ AND <condition> ] THEN <not_matched_action>

matched_action can be one of

DELETE
UPDATE SET * or
UPDATE SET col1 = value1 [, col2 = value2, ...]

and not_matched_action can be one of

INSERT *
INSERT (col1 [, col2, ...]) VALUES (value1 [, value2, ...])

Does this PR introduce any user-facing change?

Yes. The SQL command changes, but it is backward compatible.

How was this patch tested?

New tests added.

…in MERGE INTO

xy-xin · 2020-06-20T01:26:11Z

@cloud-fan @brkyvz , pls take a look.

SparkQA · 2020-06-20T06:34:41Z

Test build #124306 has finished for PR 28875 at commit e18a7a5.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class DeleteAction(override val condition: Option[Expression]) extends MergeAction(condition)

sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

cloud-fan · 2020-06-23T04:17:12Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

-    }
-    if (matchedActions.groupBy(_.getClass).mapValues(_.size).exists(_._2 > 1)) {
+    val matchedActionSize = matchedActions.length
+    if (matchedActionSize >= 2 && !matchedActions.init.forall(_.condition.nonEmpty)) {


we can still write .children.isEmpty, then we don't need to change v2Commands.scala

I don't think so, because the children of InsertAction and UpdateAction actually include condition and assignments. There may be cases where there're assignments and condition being ignored but children is nonEmpty.

then this is an existing bug?

Yes, it was a bug.

can you send a new PR against branch 3.0 to fix this bug?

Submitted a pr at #28943.

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala

SparkQA · 2020-06-23T07:05:02Z

Test build #124396 has finished for PR 28875 at commit ab97e31.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-06-23T07:05:02Z

Test build #124379 has finished for PR 28875 at commit a6ac363.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-06-23T07:05:02Z

Test build #124381 has finished for PR 28875 at commit 1d39c92.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2020-06-23T11:41:32Z

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala

  }

-  test("merge into table: the first matched clause must have a condition if there's a second") {
+  test("merge into table: only the last matched clause can omit the condition") {


shall we test the same thing for not matched clause?

cloud-fan · 2020-06-23T11:42:40Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala


 sealed abstract class MergeAction(
-    condition: Option[Expression]) extends Expression with Unevaluable {
+    val condition: Option[Expression]) extends Expression with Unevaluable {


to simplify the code a little bit:

sealed trait MergeAction extends Expression with Unevaluable { def condition: Option[Expression] ... }

Then we can just write

case class DeleteAction(condition: Option[Expression]) extends MergeAction

SparkQA · 2020-06-28T07:05:02Z

Test build #124585 has finished for PR 28875 at commit d5edef3.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds the following public classes (experimental):
sealed abstract class MergeAction extends Expression with Unevaluable
case class DeleteAction(condition: Option[Expression]) extends MergeAction

xy-xin · 2020-06-28T07:07:45Z

retest it please.

cloud-fan · 2020-06-29T02:23:29Z

retest it please

cloud-fan · 2020-06-29T03:06:17Z

ok to test

SparkQA · 2020-06-29T05:36:54Z

Test build #5046 has finished for PR 28875 at commit d5edef3.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
sealed abstract class MergeAction extends Expression with Unevaluable
case class DeleteAction(condition: Option[Expression]) extends MergeAction

SparkQA · 2020-06-29T06:15:48Z

Test build #5047 has finished for PR 28875 at commit d5edef3.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
sealed abstract class MergeAction extends Expression with Unevaluable
case class DeleteAction(condition: Option[Expression]) extends MergeAction

SparkQA · 2020-06-29T12:53:13Z

Test build #124625 has finished for PR 28875 at commit d5edef3.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
sealed abstract class MergeAction extends Expression with Unevaluable
case class DeleteAction(condition: Option[Expression]) extends MergeAction

cloud-fan · 2020-06-29T13:13:38Z

thanks, merging to master!

xy-xin · 2020-06-30T01:34:27Z

Thanks @cloud-fan !

[SPARK-32030][SQL] Support unlimited MATCHED and NOT MATCHED clauses …

e18a7a5

…in MERGE INTO

probot-autolabeler bot added the SQL label Jun 20, 2020

cloud-fan reviewed Jun 22, 2020

View reviewed changes

sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 Outdated Show resolved Hide resolved

xy_xin added 2 commits June 23, 2020 10:53

Preserve the order of the MATCHED and NOT MATCHED clauses

a6ac363

Optimize the code

1d39c92

cloud-fan reviewed Jun 23, 2020

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Jun 23, 2020

View reviewed changes

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/DDLParserSuite.scala Outdated Show resolved Hide resolved

cloud-fan reviewed Jun 23, 2020

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala Outdated Show resolved Hide resolved

Update according to review comments

ab97e31

cloud-fan reviewed Jun 23, 2020

View reviewed changes

Tagar mentioned this pull request Jun 26, 2020

Allow multiple UPDATE actions in Delta Lake MERGE INTO statement delta-io/delta#268

Closed

Update accordding to comments

d5edef3

cloud-fan approved these changes Jun 29, 2020

View reviewed changes

cloud-fan closed this in 20cd47e Jun 29, 2020

cloud-fan changed the title ~~[SPARK-32030][SQL] Support unlimited MATCHED and NOT MATCHED clauses in MERGE INTO~~ [SPARK-32030][SPARK-32127][SQL] Support unlimited MATCHED and NOT MATCHED clauses in MERGE INTO Jun 29, 2020

[SPARK-32030][SPARK-32127][SQL] Support unlimited MATCHED and NOT MATCHED clauses in MERGE INTO #28875

[SPARK-32030][SPARK-32127][SQL] Support unlimited MATCHED and NOT MATCHED clauses in MERGE INTO #28875

Uh oh!

Conversation

xy-xin commented Jun 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

xy-xin commented Jun 20, 2020

Uh oh!

SparkQA commented Jun 20, 2020

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan Jun 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Jun 23, 2020

Uh oh!

SparkQA commented Jun 23, 2020

Uh oh!

SparkQA commented Jun 23, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jun 28, 2020

Uh oh!

xy-xin commented Jun 28, 2020

Uh oh!

cloud-fan commented Jun 29, 2020

Uh oh!

cloud-fan commented Jun 29, 2020

Uh oh!

SparkQA commented Jun 29, 2020

Uh oh!

SparkQA commented Jun 29, 2020

Uh oh!

SparkQA commented Jun 29, 2020

Uh oh!

cloud-fan commented Jun 29, 2020

Uh oh!

xy-xin commented Jun 30, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xy-xin commented Jun 20, 2020 •

edited

Loading

cloud-fan Jun 29, 2020 •

edited

Loading