0lai0 commented on code in PR #3618:
URL: https://github.com/apache/datafusion-comet/pull/3618#discussion_r2877808331
##########
spark/src/main/scala/org/apache/comet/serde/datetime.scala:
##########
@@ -541,3 +541,52 @@ object CometDateFormat extends
CometExpressionSerde[DateFormatClass] {
}
}
}
+
+trait CommonDateTimeExprs {
+
+ def secondsOfTimeToProto(
+ expr: Expression,
+ inputs: Seq[Attribute],
+ binding: Boolean): Option[Expr] = {
+ val childOpt = expr.children.headOption.orElse {
+ withInfo(expr, "SecondsOfTime has no child expression")
+ None
+ }
+
+ childOpt.flatMap { child =>
+ val timeZoneId = {
+ val exprClass = expr.getClass
+ try {
+ val timeZoneIdMethod = exprClass.getMethod("timeZoneId")
Review Comment:
I originally implemented this reflection to handle potential future
scenarios where Spark might add a timeZoneId to SecondsOfTime. However, since
this field doesn't exist in any currently supported Spark versions, it is
effectively dead code. To reduce maintenance overhead, I’ll remove the
reflection logic and explicitly default it to 'UTC', consistent with other
datetime expressions.
--
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]
