sunchao commented on code in PR #56575:
URL: https://github.com/apache/spark/pull/56575#discussion_r3530908533
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/NormalizePlan.scala:
##########
@@ -86,7 +86,15 @@ object NormalizePlan extends PredicateHelper {
* we must normalize them to check if two different queries are identical.
*/
def normalizeExprIds(plan: LogicalPlan): LogicalPlan = {
- plan.transformAllExpressions {
+ // Defined as a named rule (rather than inline) so it can also be applied
to a
+ // `DelegateExpression`'s `inputs`, which are display-only metadata -- not
children -- and so
+ // are never reached by `transformAllExpressions`. Normalizing them
explicitly keeps the
+ // informational call deterministic across runs (e.g. `right(g#0, g#0)` in
EXPLAIN), since expr
+ // ids come from a process-global counter; the `definition` child is
reached by the normal
+ // traversal.
+ lazy val rule: PartialFunction[Expression, Expression] = {
+ case d: DelegateExpression =>
+ d.copy(inputs = d.inputs.map(_.transform(rule)))
Review Comment:
[P2] `RuntimeReplaceable` inputs still bypass this normalization
The follow-up covers random seeds and common-expression IDs, but
`normalizeRuntimeReplaceable(plan)` runs before this delegate-specific
transform and only follows real expression children, so it never reaches
`DelegateExpression.inputs`. A nested `RuntimeReplaceable` there can therefore
retain analyzer-specific state and make otherwise equivalent plans compare
unequal.
With dual-run analysis enabled and the sample rate set to 1, this valid
query deterministically fails before execution with
`HYBRID_ANALYZER_EXCEPTION.LOGICAL_PLAN_COMPARISON_MISMATCH`:
```sql
SELECT right(assert_true(CAST(1 AS BOOLEAN)), 1)
```
Both analyzers succeed and produce equivalent executable `definition` trees;
the mismatch is only the hidden time-zone state inside the display-only
`AssertTrue` input, so the printed plans even look identical. I reproduced this
on the current head; the same query analyzes successfully with dual-run
disabled, and the unchanged base-style `left(assert_true(CAST(1 AS BOOLEAN)),
1)` succeeds under dual-run. Could we apply the `RuntimeReplaceable`
normalization to these informational inputs as well (or exclude them from
structural comparison) and add this dual-run regression?
--
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]
