manuzhang commented on code in PR #4821:
URL: https://github.com/apache/datafusion-comet/pull/4821#discussion_r3556156814
##########
spark/src/main/spark-4.1+/org/apache/comet/shims/CometExprShim.scala:
##########
@@ -46,6 +47,22 @@ trait CometExprShim extends Spark4xCometExprShim {
}
}
+ // Function names on `DateTimeUtils` reached via `StaticInvoke` from the
Spark 4.1 `TIME` family of
+ // `RuntimeReplaceable` expressions (`HoursOfTime`, `MinutesOfTime`,
`SecondsOfTime`,
+ // `SecondsOfTimeWithFraction`, `TimeAddInterval`, `SubtractTimes`,
`TimeDiff`, `TimeTrunc`).
+ // Each of these is codegen-safe (the replacement itself is a `StaticInvoke`
with a proper
+ // `doGenCode`) but has no native lowering yet, so we route them through the
JVM codegen
+ // dispatcher to keep the enclosing projection native.
+ private val timeCodegenDispatchFunctions: Set[String] = Set(
+ "getHoursOfTime",
+ "getMinutesOfTime",
+ "getSecondsOfTime",
+ "getSecondsOfTimeWithFraction",
+ "timeAddInterval",
Review Comment:
This dispatches `timeAddInterval` even when its interval argument is a
non-foldable `DayTimeIntervalType` attribute. Spark preserves that argument as
a bound reference, and `canHandle` accepts it, but execution maps it to
`DurationVector`, which `CometScalaUDFCodegen.specFor` does not support. The
result is an execution-time `UnsupportedOperationException` rather than
fallback.
Could we add `DurationVector` as a long-backed dispatcher input? The current
tests only cover interval literals, so a test with a materialized
`make_dt_interval(...)` column would be useful.
--
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]
