mbutrovich commented on PR #5030:
URL: 
https://github.com/apache/datafusion-comet/pull/5030#issuecomment-5092992895

   The description defers `make_interval` because "the dispatcher reports 
unsupported output type CalendarIntervalType and falls back". That is no longer 
true on main: commit 69df0f12b (#4898) added `CalendarIntervalType => true` to 
`CometBatchKernelCodegen.isSupportedDataType` 
(`spark/src/main/scala/org/apache/comet/codegen/CometBatchKernelCodegen.scala:92`),
 and it is wired end to end, not just in the predicate: `serializeDataType` 
handles it (`QueryPlanSerde.scala:530` and `:571`), the output side maps it to 
`IntervalMonthDayNanoVector` and emits a writer 
(`CometBatchKernelCodegenOutput.scala:178` and `:222`), and the input side 
emits a `getInterval` getter (`CometBatchKernelCodegenInput.scala:610` and 
`:717`). This PR is based on a pre-#4898 main, which is also why it conflicts. 
@peterxcli asked about exactly this. `MakeInterval` is a plain 
`SeptenaryExpression` with `nullIntolerant` and is not RuntimeReplaceable 
(Spark `intervalExpressions.scala:338-348`), so after the rebase `o
 bject CometMakeInterval extends CometCodegenDispatch[MakeInterval]` plus a 
`classOf[MakeInterval]` entry should be all it takes. Either add it here or 
correct the description. Note that `MakeInterval.failOnError` defaults to 
`SQLConf.get.ansiEnabled`, so if you do add it, the ANSI overflow path needs an 
`expect_error` case of its own.


-- 
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]

Reply via email to