kevinjqliu opened a new issue, #18371:
URL: https://github.com/apache/iceberg/issues/18371
### Apache Iceberg version
main (development)
### Query engine
Other
### Please describe the bug 🐞
`DateTimeUtil.convertMicros` and `DateTimeUtil.convertNanos` return the
previous unit for a pre-epoch timestamp in the first second of a unit when its
fraction is `.999999` (`.999999999` for nanos). This affects the `year`,
`month`, `day` and `hour` transforms.
```java
var day = Transforms.day().bind(Types.TimestampType.withoutZone());
long midnight = -86_400_000_000L; // 1969-12-31T00:00:00
day.apply(midnight + 999_998); // -1
day.apply(midnight + 999_999); // -2, expected -1
day.apply(midnight + 1_000_000); // -1
```
The spec defines `day` as days from 1970-01-01, so
`1969-12-31T00:00:00.999999` should be `-1`. The transform also stops being
monotonic, which inclusive projection relies on, so Java's own scan skips a
matching row:
```java
// row 1969-12-31T00:00:00.999999 is written to partition ts_day = -2
Projections.inclusive(spec).project(greaterThanOrEqual("ts", midnight +
500_000));
// -> ts_day >= -1, so the file holding the row is pruned
```
**Cause:** the negative branch adds 1 to the fraction but not to the seconds:
```java
long epochSecond = Math.floorDiv(micros, MICROS_PER_SECOND);
long nanoAdjustment = Math.floorMod(micros + 1, MICROS_PER_SECOND) * 1000;
```
At `.999999`, `micros + 1` crosses into the next second but `epochSecond`
does not, so the timestamp lands almost a second earlier. In the first second
of a unit, that crosses into the previous unit.
**Fix:** `Math.floorDiv(micros + 1, MICROS_PER_SECOND)`, and the same in
`convertNanos` with `NANOS_PER_SECOND`. I checked the micros version against
floor semantics for `year`, `month`, `day` and `hour` across ~47k values,
including every unit boundary from 1900 to 1971: the current code is wrong for
3,446 of them, the fix for none.
iceberg-rust returns the calendar day for these values, see the
[iceberg-rust day transform
fix](https://github.com/apache/iceberg-rust/pull/3323).
Related: #18115 (same class of bug, rounding toward zero instead of down for
pre-epoch values, but in microsecond conversion rather than the transforms),
#9714 (transform rounding for negative values isn't specified). The `+1`/`-1`
adjustment comes from #1981.
### Willingness to contribute
- [ ] I can contribute a fix for this bug independently
- [ ] I would be willing to contribute a fix for this bug with guidance from
the Iceberg community
- [ ] I cannot contribute a fix for this bug at this time
--
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]
