cloud-fan commented on code in PR #56575:
URL: https://github.com/apache/spark/pull/56575#discussion_r3503087052
##########
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:
Fixed in `2f4260d6507`. `normalizeExpressions` now applies its full rule
(random seed + `CommonExpressionId`, alongside the subquery case) to a
`DelegateExpression`'s `inputs` as well, since `transformAllExpressions` never
reaches those non-child nodes. So the `Rand` in `inputs` is normalized to seed
0 just like the one in `definition`, and `right(rand(), 1)` no longer trips a
false `HYBRID_ANALYZER_LOGICAL_PLAN_COMPARISON_MISMATCH`. Added a
`NormalizePlanSuite` regression asserting two delegates with different `inputs`
seeds / common-expr ids normalize equal.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/package.scala:
##########
@@ -127,6 +127,16 @@ package object util extends Logging {
),
dataType = r.dataType
)
+ case d: DelegateExpression =>
+ // `inputs` are display-only metadata, not children, so `transform`
never descends into them.
+ // Render the high-level call with each input prettified (qualifiers
stripped, string literals
+ // unquoted, ...) so generated column names match the pre-delegate
function, e.g. the column
+ // name for `right(c7, 2)` stays `right(c7, 2)` rather than
`right(spark_catalog....c7, 2)`.
+ PrettyAttribute(
+ name = s"${d.name}(${d.inputs
+ .map(i => toPrettySQL(i,
shouldTrimTempResolvedColumn)).mkString(", ")})",
Review Comment:
Fixed in `2f4260d6507`. Mirrored the `InheritAnalysisRules` handling right
above it: when `shouldTrimTempResolvedColumn` is set, the delegate `inputs` are
pre-trimmed with `trimTempResolvedColumn` before `toPrettySQL` (which has no
marker case of its own). An aggregate/HAVING alias over a delegate now
generates `right(v, 1)` instead of `right(tempresolvedcolumn(v), 1)`. Added a
SPARK-52385-style HAVING regression using `right` in
`DelegateExpressionQuerySuite`.
--
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]
