feat: Support Spark expression: make_dt_interval#4338
Conversation
|
This patch also help #4150 |
|
|
||
| object CometMakeDate extends CometScalarFunction[MakeDate]("make_date") | ||
|
|
||
| object CometMakeDTInterval extends CometScalarFunction[MakeDTInterval]("make_dt_interval") |
There was a problem hiding this comment.
Could you implement getSupportLevel and mark this as incompatible. Also, we need getIncompatReasons so we generate documentation.
My AI review helper tells me that Spark's IntervalUtils.makeDayTimeInterval uses Math.addExact / Math.multiplyExact and always throws on overflow:
catch {
case _: ArithmeticException =>
throw QueryExecutionErrors.withoutSuggestionIntervalArithmeticOverflowError(context)
}The upstream SparkMakeDtInterval kernel uses checked_mul / checked_add and silently returns NULL on overflow. Inputs that throw INTERVAL_ARITHMETIC_OVERFLOW in Spark will
produce a NULL row in Comet.
Could you verify whether this is true or not? If it is then we can just mark it incompatible for now and file an upstream issue against the datafusion repo.
There was a problem hiding this comment.
Hi @andygrove
Thanks for the review! The statement is true, i've marked it incompatible and filed an issue.
…sion_make_dt_interval # Conflicts: # docs/source/user-guide/latest/expressions.md # native/core/src/execution/planner.rs # native/proto/src/proto/types.proto # spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala # spark/src/main/scala/org/apache/comet/serde/datetime.scala # spark/src/main/scala/org/apache/spark/sql/comet/util/Utils.scala
Which issue does this PR close?
Closes #3098
Rationale for this change
Support expression
make_dt_intervalWhat changes are included in this PR?
QueryPlanSerd,serd.rsandtypes.protojni_api.rs,datatime.scalaandQueryPlanSerdHow are these changes tested?
Add
make_dt_intervalsql and test in spark3.4/3.5/4.0