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