andygrove commented on code in PR #5039:
URL: https://github.com/apache/datafusion-comet/pull/5039#discussion_r3659500009
##########
spark/src/main/scala/org/apache/comet/serde/datetime.scala:
##########
@@ -954,6 +954,17 @@ object CometMakeYMInterval extends
CometCodegenDispatch[MakeYMInterval]
object CometMakeDTInterval extends CometCodegenDispatch[MakeDTInterval]
+object CometMakeInterval extends CometExpressionSerde[MakeInterval] {
+ override def convert(
+ expr: MakeInterval,
+ inputs: Seq[Attribute],
+ binding: Boolean): Option[Expr] = {
+ val childExprs = expr.children.map(exprToProtoInternal(_, inputs, binding))
+ val optExpr = scalarFunctionExprToProto("make_interval", expr.failOnError,
childExprs: _*)
+ optExprWithFallbackReason(optExpr, expr, expr.children: _*)
Review Comment:
Could this use `scalarFunctionExprToProtoWithReturnType("make_interval",
CalendarIntervalType, expr.failOnError, childExprs: _*)` instead? The
`make_interval` name collides with the DataFusion UDF registered in
`register_datafusion_spark_function`, and the sibling `CometMakeDate` right
above sets the return type explicitly for the same reason (see the "When to set
the return type explicitly" section of the contributor guide). It currently
works because DataFusion's signature happens to line up, but pinning the return
type keeps this robust against future signature changes.
--
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]
