feat: support timestampadd and timestampdiff via codegen dispatch#5030
Open
andygrove wants to merge 1 commit into
Open
feat: support timestampadd and timestampdiff via codegen dispatch#5030andygrove wants to merge 1 commit into
andygrove wants to merge 1 commit into
Conversation
Route TimestampAdd and TimestampDiff through the JVM codegen dispatcher so they run inside the native Comet pipeline instead of forcing a full operator fallback to Spark. The dispatcher executes Spark's own generated code, so results match Spark exactly across all supported versions and avoid the datetime incompatibilities that a chrono-based native implementation would risk. make_interval was considered but is left for a follow-up: its output type CalendarIntervalType is not yet supported by Comet's columnar layer, so the dispatcher cannot produce it.
Member
I think you're refer to #3099? I saw the native |
peterxcli
approved these changes
Jul 25, 2026
peterxcli
left a comment
Member
There was a problem hiding this comment.
LGTM, only need to merge upstream and resolve conflict in expressions.md.
ai review: non-blocker review:
- add MILLISECOND to both tests and MICROSECOND to timestampdiff.
- Add one TIMESTAMP_NTZ or DST-boundary case; current tests only exercise timestamp-LTZ away from DST transitions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
There is no dedicated issue. This is part of the ongoing effort to provide native support for expressions that currently force a full fallback to Spark. Related to the expression coverage epic #240.
Rationale for this change
timestampaddandtimestampdiffhave no Comet handler today, so any query using them falls the entire operator back to Spark. Both are regular expressions (present since Spark 3.3), notRuntimeReplaceable, so they reach serde directly.Rather than a native implementation, this PR routes them through the JVM codegen dispatcher. The dispatcher runs Spark's own generated code inside the native Comet pipeline, which keeps the operator native while guaranteeing bit-for-bit Spark compatibility across all supported versions. This avoids the datetime edge-case divergences (timezone and calendar handling) that a
chrono-based native implementation would be prone to.What changes are included in this PR?
CometTimestampAddandCometTimestampDiffcodegen-dispatch serdes indatetime.scala, registered inQueryPlanSerde'stemporalExpressionsmap.timestampadd.sqlandtimestampdiff.sqlcovering all time units, negative quantities, month-end and leap-day rollover, whole-unit truncation, and NULL inputs.make_intervalwas considered but left for a follow-up: its output type isCalendarIntervalType, which Comet's columnar layer does not yet support (the dispatcher reportsunsupported output type CalendarIntervalTypeand falls back). It can be enabled once CalendarIntervalType support lands (#4898).How are these changes tested?
New Comet SQL file tests run each query through both Spark and Comet and verify the results match and that Comet executes the expression natively (through the dispatcher) rather than falling back. Coverage includes every supported time unit, positive and negative quantities, month-end and leap-day boundaries, whole-unit truncation toward zero, and NULL inputs.