dwsmith1983 opened a new pull request, #5864: URL: https://github.com/apache/datafusion-comet/pull/5864
## Which issue does this PR close? Closes #3094, closes #3112, closes #3086, closes #3115, closes #3114. ## Rationale for this change Spark's date and timestamp interval arithmetic (`date - date`, `timestamp - timestamp`, `date + interval`, `timestamp + interval`) had no serde at all, so any projection using them fell back to Spark with a columnar-to-row transition. The recent temporal additions (`timestampadd`, `timestampdiff`, `make_interval`) show these run inside the Comet pipeline through the codegen dispatcher with a one-line serde each, and the dispatcher already accepts every interval type on input and output, so nothing native needs to change. ## What changes are included in this PR? - `CometSubtractDates`, `CometSubtractTimestamps`, `CometDateAddInterval`, `CometDateAddYMInterval` and `CometTimestampAddYMInterval` as `CometCodegenDispatch` objects in `serde/datetime.scala`, registered in `temporalExpressions`. - `timestamp + day-time interval` is `TimeAdd` on Spark 3.4, 3.5 and 4.0 and `TimestampAddInterval` on 4.1 and later, so those two go through the version shims' misc expressions: a `CometTimeAdd` serde in the `spark-3.x` and `spark-4.0` source roots (no root covers exactly 3.4 through 4.0) and a `CometTimestampAddInterval` serde in `spark-4.1+`. - The `date - interval` and `timestamp - interval` forms need nothing extra: Spark rewrites `DatetimeSub` into the add form over a negated interval, and the dispatcher binds the whole subtree. - Notes on the `+` and `-` rows of the expressions guide. ## How are these changes tested? Eight SQL-file fixtures under `sql-tests/expressions/datetime/`, all over parquet tables so nothing folds to a literal, with NULL and negative operands, month-end clamping for year-month intervals, rows across both DST transitions in `America/Los_Angeles`, `TIMESTAMP` and `TIMESTAMP_NTZ` inputs, both values of `spark.sql.legacy.interval.enabled` where the result type changes, and one query per file through native shuffle. The ANSI rejection of a date plus an interval with a time part is split by Spark version, since 3.x reports a plain message and 4.x reports `INVALID_INTERVAL_WITH_MICROSECONDS_ADDITION`. Before the serde change, seven of the fixtures failed with `Expected only Comet native operators, but found Project`. After it: 20 of 20 interval fixtures on Spark 3.5, the ANSI fixtures on the Spark 4.0 profile, and `test-compile` on the 3.4, 4.0 and 4.1 profiles, spotless clean. One pre-existing gap surfaced while writing the fixtures: in legacy-interval mode Spark's null propagation folds `CAST(NULL AS DATE) - date'...'` into a bare NULL literal of `CalendarIntervalType`, which `CometLiteral` admits and the native planner rejects at execution. That is independent of this change and will get its own issue. -- 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]
