sunchao opened a new pull request, #5457:
URL: https://github.com/apache/datafusion-comet/pull/5457

   ## Which issue does this PR close?
   
   Closes #5456. Follow-up to #5443.
   
   ## Why are the changes needed?
   
   Native `make_date` now accepts Spark's wide date range, but its timestamp 
consumers still assume narrower ranges. For example, casting `make_date(300000, 
6, 15)` to `TIMESTAMP_NTZ` silently wraps the microsecond count in release 
builds instead of throwing Spark's `ArithmeticException: long overflow`. 
Casting `make_date(262143, 1, 1)` to `TIMESTAMP` in UTC panics inside chrono 
even though the timestamp fits in a signed 64-bit value.
   
   The fix needs to preserve the expanded date range while making both casts 
safe. Restoring the old `make_date` null restriction would lose the 
compatibility improvement from #5443.
   
   ## What changes were proposed in this PR?
   
   Use direct epoch-day arithmetic for NTZ, UTC, and standard `+/-HH:MM` 
fixed-offset casts. Apply the offset in seconds before checked multiplication 
to microseconds, so the final range check includes the timezone adjustment. 
Overflow becomes a plain `ArithmeticException` in both legacy and ANSI modes. 
Nullable scalar `TRY_CAST` returns null for overflowing rows without discarding 
valid rows in the same batch.
   
   Other date-to-timestamp timezone spellings use Spark's existing JVM codegen 
dispatcher, or Spark row execution when the dispatcher is disabled. This keeps 
historical and far-future timezone rules with Spark. This routing applies to 
ordinary dates too, so named-zone casts may be slower; UTC and standard fixed 
offsets retain native execution.
   
   Spark's nullability inference can miss date-to-timestamp overflow in 
`TRY_CAST`. Non-nullable scalar and complex TRY casts containing this 
conversion therefore stay on Spark's row path, avoiding nulls in non-nullable 
Arrow fields or map keys. The codegen eligibility check also catches these 
casts inside a larger dispatched expression.
   
   ## How was this PR tested?
   
   - `make core`.
   - `cargo test --manifest-path native/Cargo.toml -p 
datafusion-comet-spark-expr conversion_funcs::temporal::tests`: 5 passed.
   - `cargo test --manifest-path native/Cargo.toml -p datafusion-comet-common 
test_long_overflow_json`: 1 passed.
   - Focused `CometNativeCastSuite DateType to` and `SparkErrorConverterSuite` 
runs on Spark 3.5.9, 4.0.4, and 4.1.3: 26 passed per profile, using JDK 17 and 
clean JVM builds between profiles.
   - An isolated harness including the actual native cast source passed its 5 
kernel tests with overflow checks disabled, plus a check confirming unchecked 
overflow wraps in that harness.
   - `cargo fmt --manifest-path native/Cargo.toml --all -- --check`, Spotless, 
Scalastyle, and `git diff --check`.
   
   The JVM regressions use Parquet-backed columns and assert native execution, 
JVM dispatcher execution, or full Spark fallback as appropriate. They cover 
wide positive/negative dates, timestamp boundaries, offset-induced overflow, 
nulls, both ANSI settings, and nested TRY casts. These are targeted local 
checks, not full-suite or CI results.
   


-- 
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]

Reply via email to