andygrove opened a new pull request, #4784: URL: https://github.com/apache/datafusion-comet/pull/4784
## Which issue does this PR close? Part of #4553. ## Rationale for this change Spark's `window()` and `session_window()` time-window grouping functions are usable in ordinary batch DataFrame/SQL queries, not just Structured Streaming. When the analyzer resolves a batch tumbling or sliding `window()` grouping, it rewrites the expression into a plain projection that builds a `named_struct(start, end)` from modulo arithmetic over `PreciseTimestampConversion`, feeding a normal aggregate. Every operator in that plan (`Project`, `HashAggregate`, `Exchange`, and `Expand` for sliding windows) is already supported by Comet, and every expression except `PreciseTimestampConversion` (and the `KnownNullable` tag wrapped around the window bounds) is already supported. As a result these queries previously fell back to Spark solely because of those two expressions. `PreciseTimestampConversion` is a pure reinterpret between the timestamp types and `LongType` that preserves microsecond precision (Spark's `nullSafeEval` is the identity function). The underlying Arrow representations (`Timestamp(microsecond)` and `Int64`) are bit-identical, so no data transformation is required. ## What changes are included in this PR? - Add a `PreciseTimestampConversion` protobuf message and `CometPreciseTimestampConversion` serde. Only the timestamp/long reinterpret is offloaded; any other type combination falls back to Spark. - On the native side, map it to DataFusion's `CastExpr` (Arrow cast semantics, a zero-cost reinterpret between `Timestamp(microsecond)` and `Int64`), rather than Comet's Spark-compatible `Cast`, which would scale by 1,000,000. - Add a `CometKnownNullable` serde for the `KnownNullable` tagging expression that the window resolution wraps around window bounds. It is a runtime no-op, so the serde unwraps and serializes the child. - Update the expression support reference: `window` and `window_time` now run natively for batch grouping; `session_window` still falls back because its batch `UpdatingSessionsExec` operator is not yet native. This work was scaffolded using the `implement-comet-expression` skill. ## How are these changes tested? New tests in `CometTemporalExpressionSuite` verify that batch tumbling, sliding, and `window_time` aggregations produce Spark-identical results and run natively (`checkSparkAnswerAndOperator`), across both `TimestampType` and `TimestampNTZType` (the latter across multiple session timezones). A `session_window` case confirms results still match Spark while that stage falls back. -- 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]
