uros-db commented on code in PR #56355:
URL: https://github.com/apache/spark/pull/56355#discussion_r3372331148
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuiteBase.scala:
##########
@@ -1622,8 +1622,11 @@ abstract class CastSuiteBase extends SparkFunSuite with
ExpressionEvalHelper {
checkEvaluation(Cast(Literal(time), StringType), expectedStr)
}
+ // Cast to string now routes through the zone-aware Types Framework hook
(SPARK-57285), which
+ // evaluates the (zone-independent for TimeType) session zone, so resolve
a time zone like a
+ // real cast rather than leaving the TimeZoneAwareExpression unresolved.
checkConsistencyBetweenInterpretedAndCodegen(
- (child: Expression) => Cast(child, StringType), TimeType())
+ (child: Expression) => Cast(child, StringType, UTC_OPT), TimeType())
Review Comment:
The accompanying comment says the cast "now routes through the zone-aware
Types Framework hook ... which evaluates the (zone-independent for TimeType)
session zone." But TypeApiOps.apply's TimeType branch never forces the by-name
zoneId, and TimeType codegen still uses timeFormatter.
So as far as I can tell, TimeType never evaluates zoneId, and this test
should pass without UTC_OPT? Either the change is unnecessary/defensive (in
which case the comment is misleading), or there's a zone-forcing path for
zone-independent types that I'm missing...
--
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]
