You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "yadavay-amzn (via GitHub)" <gi...@apache.org> on 2026/06/25 06:19:11 UTC
[PR] [SPARK-57685][SQL] Support non-deterministic expressions in MERGE INTO action assignments on DSv2 tables [spark]
yadavay-amzn opened a new pull request, #56767:
URL: https://github.com/apache/spark/pull/56767
### What changes were proposed in this pull request?
Make `MergeRows` implement `SupportsNonDeterministicExpression` with `allowNonDeterministicExpression = true`. This allows non-deterministic expressions (e.g. `uuid()`, `rand()`) in MERGE INTO action assignments (`UPDATE SET col = uuid()`, `INSERT ... VALUES (..., rand())`) to pass analysis and execute correctly on DSv2 row-level operation tables.
### Why are the changes needed?
This is a follow-up to SPARK-56729 (PR #55858) which handled non-deterministic expressions in MERGE *source* queries. However, non-deterministic expressions in MERGE *action assignments* still fail with `INVALID_NON_DETERMINISTIC_EXPRESSIONS` because the rewritten plan places these expressions inside `MergeRows`, which was not in CheckAnalysis's allowlist.
`MergeRows` evaluates each WHEN-clause output projection exactly once per produced output row:
- Interpreted path (MergeRowsExec lines 579-590): first matching instruction evaluates `projection.apply(row)` then returns.
- Codegen path (MergeRowsExec lines 237-249): condition checked, if true: consume projection then return.
This satisfies the safety condition for `SupportsNonDeterministicExpression` (same rationale as operators already in the allowlist, e.g. `NearestByJoin`).
The single change covers both the group-based (copy-on-write: MergeRows -> ReplaceData) and delta-based (merge-on-read: MergeRows -> WriteDelta) MERGE paths, since both route action expressions through `MergeRows`.
### Does this PR introduce _any_ user-facing change?
Yes. MERGE INTO statements with non-deterministic expressions in action assignments (e.g. `WHEN MATCHED THEN UPDATE SET id = uuid()`) now succeed on DSv2 tables instead of failing with `INVALID_NON_DETERMINISTIC_EXPRESSIONS`.
### How was this patch tested?
- Converted existing failure-intercept tests into success tests covering both group-based and delta-based paths.
- Tests verify that `uuid()` in UPDATE SET produces non-null 36-char UUID strings.
- Tests verify that `rand()` in INSERT VALUES produces values in expected range.
- All existing CheckAnalysis, MergeIntoTable, GroupBasedMerge, and DeltaBasedMerge test suites pass.
- Scalastyle passes with 0 errors.
### Was this patch authored or co-authored using generative AI tooling?
Yes.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57685][SQL] Support non-deterministic expressions in MERGE INTO action assignments on DSv2 tables [spark]
Posted by "yadavay-amzn (via GitHub)" <gi...@apache.org>.
yadavay-amzn commented on code in PR #56767:
URL: https://github.com/apache/spark/pull/56767#discussion_r3483330101
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/MergeNonDeterministicReproSuite.scala:
##########
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connector
+
+/**
+ * Tests for SPARK-57685: MERGE INTO with non-deterministic expressions in action
+ * assignments should pass analysis and execute correctly on DSv2 row-level tables.
+ *
+ * MergeRows now implements SupportsNonDeterministicExpression, allowing non-deterministic
+ * expressions like uuid() and rand() in MERGE action assignments.
+ */
+class MergeNonDeterministicReproSuite extends MergeIntoTableSuiteBase {
Review Comment:
Renamed to `MergeIntoNonDeterministicSuite`. Thanks for the review and for verifying the once-per-row safety.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57685][SQL] Support non-deterministic expressions in MERGE INTO action assignments on DSv2 tables [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #56767: [SPARK-57685][SQL] Support non-deterministic expressions in MERGE INTO action assignments on DSv2 tables
URL: https://github.com/apache/spark/pull/56767
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57685][SQL] Support non-deterministic expressions in MERGE INTO action assignments on DSv2 tables [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #56767:
URL: https://github.com/apache/spark/pull/56767#discussion_r3478195510
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/MergeNonDeterministicReproSuite.scala:
##########
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.connector
+
+/**
+ * Tests for SPARK-57685: MERGE INTO with non-deterministic expressions in action
+ * assignments should pass analysis and execute correctly on DSv2 row-level tables.
+ *
+ * MergeRows now implements SupportsNonDeterministicExpression, allowing non-deterministic
+ * expressions like uuid() and rand() in MERGE action assignments.
+ */
+class MergeNonDeterministicReproSuite extends MergeIntoTableSuiteBase {
Review Comment:
Minor: this is a permanent regression suite, but the name `MergeNonDeterministicReproSuite` ("Repro") reads like a throwaway reproduction. Consider something like `MergeIntoNonDeterministicSuite` so future maintainers don't mistake it for a temporary repro. Non-blocking.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-57685][SQL] Support non-deterministic expressions in MERGE INTO action assignments on DSv2 tables [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #56767:
URL: https://github.com/apache/spark/pull/56767#issuecomment-5986238667
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org