cloud-fan commented on code in PR #58244:
URL: https://github.com/apache/spark/pull/58244#discussion_r3854074685
##########
sql/core/src/test/scala/org/apache/spark/sql/connector/RowLevelOperationSuiteBase.scala:
##########
@@ -127,6 +127,15 @@ abstract class RowLevelOperationSuiteBase
catalog.createTable(ident, tableInfo)
}
+ /** Declares a session variable for the duration of `f`, dropping it
afterwards. */
Review Comment:
**Non-blocking:**
Make this helper reject or preserve an existing same-named variable.
`DECLARE OR REPLACE` overwrites the outer entry, and the inner `finally` then
drops it, so a nested call returns to an outer body where the variable is gone.
The inherited `QueryTest.withSessionVariable(name)` already provides
exception-safe cleanup; wrapping a plain `DECLARE VARIABLE` with it avoids both
clobbering and cleanup masking.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveReferencesInUpdate.scala:
##########
@@ -67,7 +67,8 @@ class ResolveReferencesInUpdate(val catalogManager:
CatalogManager)
val newUpdate = u.copy(
assignments = newAssignments,
- condition = u.condition.map(resolveExpressionByPlanChildren(_, u)))
+ condition = u.condition.map(
+ resolveExpressionByPlanChildren(_, u, includeLastResort = true)))
Review Comment:
**Non-blocking:**
Update the class Scaladoc with this condition path. It currently presents a
complete three-step `UpdateTable` resolution order, but `includeLastResort =
true` adds outer-reference and SQL-variable resolution for conditions.
Separating the assignment and condition orders would keep the contract accurate.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -1894,7 +1895,8 @@ class Analyzer(
case o => o
}
- val resolvedMergeCondition =
resolveExpressionByPlanChildren(m.mergeCondition, m)
+ val resolvedMergeCondition =
+ resolveExpressionByPlanChildren(m.mergeCondition, m,
includeLastResort = true)
Review Comment:
Confirmed: both `WHEN NOT MATCHED` condition branches now enable last-resort
resolution, and the focused insert and insert-star tests assert the filtered
result. Thanks.
--
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]
