sunchao commented on code in PR #58779:
URL: https://github.com/apache/spark/pull/58779#discussion_r4049840611
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/nullExpressions.scala:
##########
@@ -187,8 +187,17 @@ private case class TypedNullLiteral(child: Expression)
override lazy val replacement: Expression = Literal.create(null,
child.dataType)
- override protected def withNewChildInternal(newChild: Expression):
TypedNullLiteral =
- copy(child = newChild)
+ override protected def withNewChildInternal(newChild: Expression):
Expression =
+ TypedNullLiteral.create(newChild)
+}
+
+private[sql] object TypedNullLiteral {
+ /**
+ * Replaces a resolved child with a typed null marker to avoid 3^n growth in
nested `NullIf`
+ * plans.
+ */
+ def create(child: Expression): TypedNullLiteral = TypedNullLiteral(
+ if (child.resolved) Literal.create(null, child.dataType) else child)
Review Comment:
Confirmed fixed at d1f174c4624df6652fd7063ab414dca6a204ffec. I reran the
view reproduction using the revised production classes: it returns the expected
null result under legacy, single-pass, and dual analysis with both
common-expression settings. NullIfResult keeps the generated null out of
analysis, so it no longer receives the view's default collation independently.
Resolving this thread.
##########
sql/core/src/test/scala/org/apache/spark/sql/DataFrameFunctionsSuite.scala:
##########
@@ -363,6 +363,23 @@ class DataFrameFunctionsSuite extends SharedSparkSession {
}
}
+ test("nullif keeps the coerced type of its first argument") {
+ Seq(true, false).foreach { alwaysInlineCommonExpr =>
+ Seq(true, false).foreach { ansiEnabled =>
+ withSQLConf(
+ SQLConf.ALWAYS_INLINE_COMMON_EXPR.key ->
alwaysInlineCommonExpr.toString,
+ SQLConf.ANSI_ENABLED.key -> ansiEnabled.toString,
+ SQLConf.CONCAT_BINARY_AS_STRING.key -> "true") {
+ checkAnswer(sql("SELECT nullif(concat(X'61', X'62'), 'z')"),
Row("ab"))
Review Comment:
[P2] Make the added regression pass under the suite's analyzer validation
This new test currently fails in [the exact-head CI
run](https://github.com/stefankandic/spark/runs/105709484058) with
`HYBRID_ANALYZER_EXCEPTION.LOGICAL_PLAN_COMPARISON_MISMATCH`. With
`alwaysInlineCommonExpr=false`, fixed-point analysis retains binary casts in
the equality predicate, while single-pass analysis compares strings directly. I
reproduced the mismatch with both ANSI settings; each analyzer individually
returns the expected string `ab`.
The same mismatch reproduces with the original production classes, so this
is an existing analyzer discrepancy exposed by the new test, rather than new
result corruption from `NullIfResult`. Nevertheless, the added test leaves this
PR's SQL suite failing. Please reconcile the plans, or explicitly run this
result-typing regression against each analyzer independently while tracking the
pre-existing dual-run discrepancy separately.
--
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]
