peter-toth commented on code in PR #57227:
URL: https://github.com/apache/spark/pull/57227#discussion_r3604863220
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/MergeIntoTableSuiteBase.scala:
##########
@@ -2851,6 +2853,127 @@ abstract class MergeIntoTableSuiteBase extends
RowLevelOperationSuiteBase
assert(catalog.lastTransaction.isClosed)
}
+ test("SPARK-58007: merge with dynamic options on the target") {
+ withTable(sourceNameAsString) {
+ createAndInitTable("pk INT NOT NULL, salary INT, dep STRING",
+ """{ "pk": 1, "salary": 100, "dep": "hr" }
+ |{ "pk": 2, "salary": 200, "dep": "software" }
+ |""".stripMargin)
+ sql(s"CREATE TABLE $sourceNameAsString (pk INT NOT NULL, salary INT, dep
STRING)")
+ sql(s"INSERT INTO $sourceNameAsString VALUES (1, 150, 'hr'), (3, 300,
'hr')")
+
+ // the target options become the row-level operation options: they reach
the rewritten
+ // DataSourceV2Relation, the RowLevelOperationInfo, and the write
builder's LogicalWriteInfo
+ val Seq(qe) = withQueryExecutionsCaptured(spark) {
+ sql(
+ s"""MERGE INTO $tableNameAsString t WITH (`write.split-size` = 10)
+ |USING $sourceNameAsString s
+ |ON t.pk = s.pk
+ |WHEN MATCHED THEN UPDATE SET t.salary = s.salary
Review Comment:
Every one of the three new tests has a `WHEN MATCHED` action, so they all
take the full row-level rewrite (`ReplaceData`/`WriteDelta`). The MATCHED-free
fast path is untested with options: `RewriteMergeIntoTable` turns a MERGE with
only `WHEN NOT MATCHED THEN INSERT` into an `InsertOnlyMerge`
(`RewriteMergeIntoTable.scala:77,119`), whose write is built from `r.options`
at a separate site (`V2Writes.scala:55`) that these tests never reach. Worth
one more test: target options on a `WHEN NOT MATCHED THEN INSERT *`-only merge,
asserting the option lands on `table.lastWriteInfo`.
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/MergeIntoTableSuiteBase.scala:
##########
@@ -2851,6 +2853,127 @@ abstract class MergeIntoTableSuiteBase extends
RowLevelOperationSuiteBase
assert(catalog.lastTransaction.isClosed)
}
+ test("SPARK-58007: merge with dynamic options on the target") {
+ withTable(sourceNameAsString) {
+ createAndInitTable("pk INT NOT NULL, salary INT, dep STRING",
+ """{ "pk": 1, "salary": 100, "dep": "hr" }
+ |{ "pk": 2, "salary": 200, "dep": "software" }
+ |""".stripMargin)
+ sql(s"CREATE TABLE $sourceNameAsString (pk INT NOT NULL, salary INT, dep
STRING)")
+ sql(s"INSERT INTO $sourceNameAsString VALUES (1, 150, 'hr'), (3, 300,
'hr')")
+
+ // the target options become the row-level operation options: they reach
the rewritten
+ // DataSourceV2Relation, the RowLevelOperationInfo, and the write
builder's LogicalWriteInfo
+ val Seq(qe) = withQueryExecutionsCaptured(spark) {
+ sql(
+ s"""MERGE INTO $tableNameAsString t WITH (`write.split-size` = 10)
+ |USING $sourceNameAsString s
+ |ON t.pk = s.pk
+ |WHEN MATCHED THEN UPDATE SET t.salary = s.salary
+ |WHEN NOT MATCHED THEN INSERT *
+ |""".stripMargin)
+ }
+ val writeRelation = qe.optimizedPlan.collectFirst {
+ case rd: ReplaceData => rd.table
+ case wd: WriteDelta => wd.table
+ }.getOrElse(fail("couldn't find row-level operation in optimized plan"))
+ .asInstanceOf[DataSourceV2Relation]
+ val operation =
writeRelation.table.asInstanceOf[RowLevelOperationTable].operation
+ .asInstanceOf[RowLevelOperationWithOptions]
+ assert(writeRelation.options.get("write.split-size") === "10", "relation
option")
Review Comment:
These three lines are exactly
`UpdateTableSuiteBase.checkRowLevelOperationOptions`
(`UpdateTableSuiteBase.scala:1242`), and the "both" test repeats them again at
2955-2958. Since both `MergeIntoTableSuiteBase` and `UpdateTableSuiteBase`
extend `RowLevelOperationSuiteBase`, consider lifting that helper into the
shared base and calling it here:
```scala
checkRowLevelOperationOptions(
sql(s"""MERGE INTO ... """.stripMargin),
"write.split-size" -> "10")
```
That keeps the target-side verification identical across UPDATE and MERGE
and drops the duplicated block (the helper already wraps the
`collectFirst`/capture).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
