andygrove opened a new issue, #3315:
URL: https://github.com/apache/iceberg-rust/issues/3315
### Apache Iceberg Rust version
main @ `bb1e4a4861f02377489eff818b75138f414c4cb0`
### Describe the bug
`Day::day_timestamp_micro` and `Day::day_timestamp_nano` in
`crates/iceberg/src/transform/temporal.rs` take the whole seconds of the value
with `v / MICROS_PER_SECOND` (and `v / NANOS_PER_SECOND`). Rust's `/` truncates
toward zero, but the fraction of the second comes from `(v +
1).rem_euclid(...)`, which floors. For a negative value that is not on a whole
second, the seconds come out one too high. When that extra second crosses a
midnight, the day count moves to the next day. So a timestamp in the last
second of any day before 1969-12-31 gets the day after it, unless its
microsecond of second is 0 or 999999 (nanosecond of second 0 or 999999999 for
`timestamp_ns`).
`Day::transform` and `Day::transform_literal` both call these functions. So
a table written through iceberg-rust puts these rows in a different `day`
partition from the one iceberg-java writes them to. `Transform::project` also
computes the day of a predicate's literal through the same function.
### To Reproduce
```rust
use std::sync::Arc;
use arrow_array::{ArrayRef, TimestampMicrosecondArray};
use iceberg::spec::{Datum, Transform};
use iceberg::transform::create_transform_function;
let day = create_transform_function(&Transform::Day)?;
let input: ArrayRef = Arc::new(TimestampMicrosecondArray::from(vec![
-86_400_500_000, // 1969-12-30T23:59:59.500000
-86_400_000_002, // 1969-12-30T23:59:59.999998
-31_536_000_500_000, // 1968-12-31T23:59:59.500000
-86_401_000_000, // 1969-12-30T23:59:59.000000
-500_000, // 1969-12-31T23:59:59.500000
]));
let days = day.transform(input)?; // Date32Array [-1, -1, -365, -2, -1]
let literal =
day.transform_literal(&Datum::timestamp_micros(-86_400_500_000))?;
// Some(Datum { type: Date, literal: Int(-1) })
```
| Input (micros) | Timestamp | iceberg-rust `day` |
iceberg-java `day` |
| --------------------- | -------------------------- | ------------------ |
------------------ |
| `-86_400_500_000` | 1969-12-30T23:59:59.500000 | -1 (1969-12-31) |
-2 (1969-12-30) |
| `-86_400_000_002` | 1969-12-30T23:59:59.999998 | -1 (1969-12-31) |
-2 (1969-12-30) |
| `-31_536_000_500_000` | 1968-12-31T23:59:59.500000 | -365 (1969-01-01) |
-366 (1968-12-31) |
| `-86_401_000_000` | 1969-12-30T23:59:59.000000 | -2 |
-2 |
| `-500_000` | 1969-12-31T23:59:59.500000 | -1 |
-1 |
`timestamp_ns` behaves the same way: `-86_400_500_000_000` nanoseconds gives
day -1, where iceberg-java gives -2.
I ran this at `bb1e4a4`. On `main` (`de19a39`) the two functions differ only
in how they build their error.
### Expected behavior
The days in the iceberg-java column above. That column is `Transforms.day()`
bound to `timestamp` and to `timestamptz`, from the 1.5.2 and 1.11.0 runtimes,
and bound to `timestamp_ns` in 1.11.0. They go through
`DateTimeUtil.microsToDays` and `nanosToDays`, which take the seconds with
`Math.floorDiv`:
```java
long epochSecond = Math.floorDiv(micros, MICROS_PER_SECOND);
long nanoAdjustment = Math.floorMod(micros + 1, MICROS_PER_SECOND) * 1000;
return (int) granularity.between(EPOCH, toOffsetDateTime(epochSecond,
nanoAdjustment)) - 1;
```
The partition transforms table in the spec says:
> | **`day`** | Extract a date or timestamp day, as days from 1970-01-01 |
`date`, `timestamp`, `timestamptz`, `timestamp_ns`, `timestamptz_ns` | `date` |
1969-12-30T23:59:59.5 is on 1969-12-30, which is day -2.
**Possible fix.** Floor the seconds as Java does:
`v.div_euclid(MICROS_PER_SECOND)` in `day_timestamp_micro` and
`v.div_euclid(NANOS_PER_SECOND)` in `day_timestamp_nano`. I compiled both
functions with only that change and compared them with
`DateTimeUtil.microsToDays` and `nanosToDays` from Iceberg 1.11.0 on 424,120
values each: offsets around every midnight from 800 days before the epoch, plus
random values. None differed. The current functions differ on 108,041 and
108,938 of them.
Flooring only the seconds, rather than dividing the whole value by the
length of a day, also keeps a quirk of iceberg-java's. Java puts a pre-epoch
timestamp exactly 999999 microseconds after midnight in the day before, so
`1969-01-01T00:00:00.999999` is day 1968-12-31 (-366). iceberg-rust returns
-365 for it today. iceberg-rust's `year`, `month` and `hour` also differ from
iceberg-java on those timestamps, because they floor. That is a separate
question. Comet ran into both while fixing
https://github.com/apache/datafusion-comet/issues/6426.
### Willingness to contribute
I would be willing to contribute a fix for this bug with guidance from the
Iceberg community.
---
**Related upstream work.** #3022 moved `year`, `month` and `hour` to compute
from the epoch, and says "Day already aligns with Java". #3237 (open) says
`Day` and `Hour` already work from the raw epoch. Neither changes
`day_timestamp_micro` or `day_timestamp_nano`.
_This report was drafted with LLM assistance (Claude Code); the behaviour
was verified with a Rust reproducer against iceberg-rust and against Iceberg
Java's `DateTimeUtil` and `Transforms` (1.5.2 and 1.11.0)._
--
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]
