MaxGekk commented on code in PR #56199:
URL: https://github.com/apache/spark/pull/56199#discussion_r3334740123
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/literals.scala:
##########
@@ -540,11 +540,15 @@ case class Literal (value: Any, dataType: DataType)
extends LeafExpression {
}
case ByteType | ShortType =>
ExprCode.forNonNullValue(JavaCode.expression(s"($javaType)$value",
dataType))
- case TimestampType | TimestampNTZType | LongType | _:
DayTimeIntervalType | _: TimeType =>
+ case TimestampType | TimestampNTZType | LongType | _:
DayTimeIntervalType =>
Review Comment:
Good catch, thanks - you're right. Removing `_: TimeType` from the explicit
`Long` case means that with `spark.sql.types.framework.enabled=false`,
`TypeOps(TimeType)` returns `None` and we fall through to
`ctx.addReferenceObj("literal", value, javaType)` with `javaType = "long"`,
generating `((long) references[i])`, which doesn't compile against the
`Object[] references` array.
Rather than restoring the `TimeType` special-case, I fixed the fallback
itself so it's correct for any primitive-typed value that reaches it: when
`javaType` is primitive it now casts to the boxed type (e.g. `(Long)
references[i]`) and relies on Java auto-unboxing in primitive contexts.
Reference-typed values (such as the nanos timestamp types) are unaffected.
I also added a regression test in `LiteralExpressionSuite` that compiles
`TimeType` (and nanos) literal codegen with the framework flag off - it
reproduced the compile failure before the fix and passes now. Fixed in 4a4e50a.
--
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]
