sunchao commented on code in PR #58779:
URL: https://github.com/apache/spark/pull/58779#discussion_r4018751683
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/nullExpressions.scala:
##########
@@ -187,8 +187,18 @@ 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 object TypedNullLiteral {
+ def create(child: Expression): Expression = {
+ if (child.resolved) {
+ Literal.create(null, child.dataType)
Review Comment:
Confirmed fixed at e0e60fd2b38d1c58a3e12eec478c93c7a11d67f6: the typed
marker is synchronized with the coerced return branch, and With refreshes its
references before body coercion. The new regression covering both ANSI and
inlining settings passed in the exact-commit CI reports. Resolving this thread;
I reported a separate reproduced single-pass view-collation issue in the new
review.
##########
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:
[P2] Preserve the generated null's collation during view resolution
This generated `Literal` is revisited by the single-pass resolver and
receives the view's default collation, even though its type came from the
operand. With the default `spark.sql.alwaysInlineCommonExpr=false`, this
reproduces on the current compiled build:
```sql
SET spark.sql.analyzer.singlePassResolver.enabled=false;
CREATE TABLE t (c STRING) USING parquet;
INSERT INTO t VALUES ('a');
CREATE VIEW v DEFAULT COLLATION UTF8_LCASE AS
SELECT lower(nullif(c, c)) AS n FROM t;
SET spark.sql.analyzer.singlePassResolver.enabled=true;
SELECT * FROM v;
```
The last query fails with `INDETERMINATE_COLLATION_IN_EXPRESSION` for
`lower(nullif(c, c))`; the base-code control returns null.
`DefaultCollationTypeCoercion` recolors the generated null to UTF8_LCASE while
the false branch's `CommonExpressionRef` remains UTF8_BINARY. Collation
coercion then wraps the marker in a cast, so the new `IfTypeCoercion` case
cannot recognize and repair it. Please keep this generated null's
operand-derived collation intact, or synchronize it before collation coercion
wraps it, and add a single-pass view 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]
