[I] Howard Hinnant civil-date algorithms hand-written 7 times across 3 crates [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove opened a new issue, #6997:
URL: https://github.com/apache/datafusion-comet/issues/6997

   ### Describe the bug
   
   ## Problem
   
   Location: native/spark-expr/src/kernels/temporal.rs:402
   Symbol: civil_from_days / days_from_civil
   
   Grep finds `days_from_civil` and `civil_from_days` as separate private copies in: (1) native/spark-expr/src/kernels/temporal.rs:407 (civil_from_days) and :425 (days_from_civil); (2) native/spark-expr/src/conversion_funcs/string.rs:1300 (days_from_civil), plus a test copy at :3440; (3) native/core/src/parquet/datetime_rebase.rs:162 (days_from_civil); (4) native/spark-expr/src/iceberg_funcs/temporal.rs:132 (civil_from_days, i32 in, returns (year, month0)); (5) native/core/src/execution/operators/iceberg_partition_path.rs:278 (civil_from_days, i64 in, (i64,u32,u32) out). The copies have already drifted in how they do floor division: string.rs uses `if y >= 0 { y / 400 } else { (y - 399) / 400 }`, iceberg_partition_path.rs uses a manual `shifted - 146_096` floor plus a cast to u64, and the others use `div_euclid`. Month indexing also differs (`(m + 9) % 12` vs. if/else). Each copy is a separately maintained implementation of the same correctness-critical calendar math that Spark compa
 tibility depends on.
   
   ## Suggested fix:
   
   Keep the `div_euclid` i64 versions from kernels/temporal.rs as `pub fn days_from_civil(y, m, d) -> i64` and `pub fn civil_from_days(days) -> (i64, i64, i64)` in a crate that both spark-expr and core depend on (for example native/common, or a `calendar` module in spark-expr). Replace the other copies with thin adapters, such as `(year as i32, month - 1)` for Iceberg, and add one shared property test that checks the pair round-trips across the full i32 epoch-day range.
   
   ## Code context
   
   ```rust
   // Integer proleptic Gregorian calendar conversion, adapted from DataFusion 55.1.0's
   // datetime/date_trunc.rs (Howard Hinnant's algorithms):
   fn civil_from_days(days: i64) -> (i64, i64, i64) {
       let z = days + 719_468;
       let era = z.div_euclid(146_097);
       ...
   }
   
   fn days_from_civil(year: i64, month: i64, day: i64) -> i64 {
       let year = year - i64::from(month <= 2);
       let era = year.div_euclid(400);
       ...
       era * 146_097 + day_of_era - 719_468
   }
   // other copies: conversion_funcs/string.rs:1300, core/parquet/datetime_rebase.rs:162,
   // iceberg_funcs/temporal.rs:132, core/.../iceberg_partition_path.rs:278
   ```
   
   ### Steps to reproduce
   
   _No response_
   
   ### Expected behavior
   
   _No response_
   
   ### Additional context
   
   _No response_
   
   ### Willingness to contribute
   
   None


-- 
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: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org