shardulm94 commented on a change in pull request #35332:
URL: https://github.com/apache/spark/pull/35332#discussion_r793944287
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
##########
@@ -310,13 +310,16 @@ abstract class CastBase extends UnaryExpression with
TimeZoneAwareExpression wit
protected def ansiEnabled: Boolean
+ protected def withDataType(dataType: DataType): CastBase
+
// When this cast involves TimeZone, it's only resolved if the timeZoneId is
set;
// Otherwise behave like Expression.resolved.
override lazy val resolved: Boolean =
childrenResolved && checkInputDataTypes().isSuccess && (!needsTimeZone ||
timeZoneId.isDefined)
override lazy val preCanonicalized: Expression = {
val basic =
withNewChildren(Seq(child.preCanonicalized)).asInstanceOf[CastBase]
+ .withDataType(dataType.asNullable)
Review comment:
I am not sure why `AttributeReference` needs to remove nullability
either, however I don't have much domain knowledge here, so I opted for a safer
change to only fix `Cast`. There can be other `Cast` like expressions which do
not handle removing nullability of their dataType params.
I had made a [test
run](https://github.com/shardulm94/spark/commit/9475fc38edeabf239904447ba6dab5a3286ffefe)
a few days ago for preserving nullability in `AttributeReference` during
canonicalization and the existing tests passed. I don't think making this
change should lead to failures, but it may lead to some lost optimization?
(From what I can see, Canonicalization is used to compare to query plans, and
with `.asNullable`, it is more likely that plans will match)
--
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]
