andygrove commented on PR #5292:
URL:
https://github.com/apache/datafusion-comet/pull/5292#issuecomment-5876415559
This is a light fully automated review since there are so many PRs open.
`CometAdd`, `CometSubtract` and `CometUnaryMinus`
(`spark/src/main/scala/org/apache/comet/serde/arithmetic.scala:152`, `:190` and
`:442`) gate on `mathDataTypeSupportLevel`, which accepts
`CalendarIntervalType`, but their native kernels only understand
`Interval(MonthDayNano)`. With this PR every calendar interval that reaches
native code is the tagged struct, so `SELECT -make_interval(years) FROM t`
hands a `StructArray` to arrow's `neg_wrapping` in `NegativeExpr`
(`native/spark-expr/src/math_funcs/negative.rs:106`) and fails with `Invalid
arithmetic operation`. `make_interval(years) + make_interval(0, months)` and
the matching subtraction reach DataFusion's `BinaryExpr`, which fails with
`Cannot coerce arithmetic expression Struct ... + Struct ...`. On main all
three ran natively on the dispatcher's `Interval(MonthDayNano)` output, and
Spark evaluates them with `IntervalUtils.negate`, `add` and `subtract` (the
`*Exact` variants under ANSI). Could these serdes decline `CalendarIn
tervalType` until there are struct-aware kernels, and could a SQL test cover
negating, adding and subtracting `make_interval` columns?
This PR closes #5279, but
`spark/src/main/scala/org/apache/comet/serde/literals.scala:375` still declines
any folded literal holding a `CalendarInterval` beyond `Long.MaxValue / 1000`
microseconds, and the Scaladoc at `:354` justifies it with the
`Math.multiplyExact(microseconds, 1000L)` conversion into
`IntervalMonthDayNanoVector`. The codegen output now writes `microseconds`
straight into the struct, so a folded `map('k', make_interval(0, 0, 0, 0,
2562048))` still falls back to Spark for a reason that no longer exists. Would
it make sense to remove that arm and `MaxArrowIntervalMicros` together with the
comment?
--
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]
