This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-3323-af1da4c5bc86178c38c1c6db0578bdd9fb7021e3 in repository https://gitbox.apache.org/repos/asf/iceberg-rust.git
commit ecba0b537a1f78ec8125c1a26f6dfab3f95b509e Author: Andy Grove <[email protected]> AuthorDate: Sun Oct 4 21:10:38 2026 +0000 fix(transform): floor day transform for pre-epoch timestamps (#3323) * fix(transform): floor day transform for pre-epoch timestamps `Day::day_timestamp_micro` and `Day::day_timestamp_nano` took the whole seconds of a timestamp with `/`, which truncates toward zero, while the fraction of the second came from `rem_euclid`, which floors. For a negative timestamp that is not on a whole second the seconds came out one too high, so a timestamp in the last second of a day before 1969-12-31 got the next day. Compute the day with `div_euclid` on the length of a day instead, the way `Hour` already does. This gives the same days as Iceberg Java's `DateTimeUtil`, except for a pre-epoch timestamp exactly 999999 microseconds (999999999 nanoseconds) after midnight, which Java puts in the day before. iceberg-rust already returned the calendar day for those, as PyIceberg and the `year`, `month` and `hour` transforms do, and following Java there would make the transform non-monotonic. Files written before this change can hold those rows one day too high. Inclusive projection already widens pre-epoch `day` predicates by one day to cover such files, as Java does, but only for `timestamp` columns. Apply it to every timestamp type. * refactor(transform): list timestamp types for day projection adjustment Match how transform.rs lists the timestamp types elsewhere, and keep the adjustment opt-in if `day` ever gains another source type. No behavior change: `day` accepts `date` and the four timestamp types. Co-authored-by: Copilot App <[email protected]> * chore(transform): trim day transform comments Drop comments that narrate history or restate the test code. The history is in the PR description. Co-authored-by: Copilot App <[email protected]> --------- Co-authored-by: Kevin Liu <[email protected]> Co-authored-by: Copilot App <[email protected]> --- crates/iceberg/src/spec/transform.rs | 8 +- crates/iceberg/src/transform/temporal.rs | 153 ++++++++++++++++++++----------- 2 files changed, 105 insertions(+), 56 deletions(-) diff --git a/crates/iceberg/src/spec/transform.rs b/crates/iceberg/src/spec/transform.rs index c4c915c08..d87054ab7 100644 --- a/crates/iceberg/src/spec/transform.rs +++ b/crates/iceberg/src/spec/transform.rs @@ -873,7 +873,13 @@ impl Transform { transformed: &Datum, ) -> Option<AdjustedProjection> { let should_adjust = match self { - Transform::Day => matches!(original.data_type(), PrimitiveType::Timestamp), + Transform::Day => matches!( + original.data_type(), + PrimitiveType::Timestamp + | PrimitiveType::Timestamptz + | PrimitiveType::TimestampNs + | PrimitiveType::TimestamptzNs + ), Transform::Year | Transform::Month => true, _ => false, }; diff --git a/crates/iceberg/src/transform/temporal.rs b/crates/iceberg/src/transform/temporal.rs index 04a8256f5..02ac2b4b9 100644 --- a/crates/iceberg/src/transform/temporal.rs +++ b/crates/iceberg/src/transform/temporal.rs @@ -24,7 +24,7 @@ use arrow_array::{ Array, ArrayRef, Date32Array, Int32Array, TimestampMicrosecondArray, TimestampNanosecondArray, }; use arrow_schema::{DataType, TimeUnit}; -use chrono::{DateTime, Datelike, Duration}; +use chrono::{DateTime, Datelike}; use super::TransformFunction; use crate::error::invalid_data; @@ -37,10 +37,10 @@ const MICROSECONDS_PER_HOUR: i64 = 3_600_000_000; const NANOSECONDS_PER_HOUR: i64 = 3_600_000_000_000; /// Year of unix epoch. const UNIX_EPOCH_YEAR: i32 = 1970; -/// One second in micros. -const MICROS_PER_SECOND: i64 = 1_000_000; -/// One second in nanos. -const NANOS_PER_SECOND: i64 = 1_000_000_000; +/// Microseconds in one day. +const MICROSECONDS_PER_DAY: i64 = 86_400_000_000; +/// Nanoseconds in one day. +const NANOSECONDS_PER_DAY: i64 = 86_400_000_000_000; /// Extract a date or timestamp year, as years from 1970 #[derive(Debug)] @@ -205,50 +205,13 @@ pub struct Day; impl Day { #[inline] - fn day_timestamp_micro(v: i64) -> Result<i32> { - let secs = v / MICROS_PER_SECOND; - - let (nanos, offset) = if v >= 0 { - let nanos = (v.rem_euclid(MICROS_PER_SECOND) * 1_000) as u32; - let offset = 0i64; - (nanos, offset) - } else { - let v = v + 1; - let nanos = (v.rem_euclid(MICROS_PER_SECOND) * 1_000) as u32; - let offset = 1i64; - (nanos, offset) - }; - - let delta = Duration::new(secs, nanos).ok_or_else(|| { - invalid_data!("Failed to create 'TimeDelta' from seconds {secs} and nanos {nanos}") - })?; - - let days = (delta.num_days() - offset) as i32; - - Ok(days) + fn day_timestamp_micro(v: i64) -> i32 { + v.div_euclid(MICROSECONDS_PER_DAY) as i32 } - fn day_timestamp_nano(v: i64) -> Result<i32> { - let secs = v / NANOS_PER_SECOND; - - let (nanos, offset) = if v >= 0 { - let nanos = (v.rem_euclid(NANOS_PER_SECOND)) as u32; - let offset = 0i64; - (nanos, offset) - } else { - let v = v + 1; - let nanos = (v.rem_euclid(NANOS_PER_SECOND)) as u32; - let offset = 1i64; - (nanos, offset) - }; - - let delta = Duration::new(secs, nanos).ok_or_else(|| { - invalid_data!("Failed to create 'TimeDelta' from seconds {secs} and nanos {nanos}") - })?; - - let days = (delta.num_days() - offset) as i32; - - Ok(days) + #[inline] + fn day_timestamp_nano(v: i64) -> i32 { + v.div_euclid(NANOSECONDS_PER_DAY) as i32 } } @@ -259,12 +222,12 @@ impl TransformFunction for Day { .as_any() .downcast_ref::<TimestampMicrosecondArray>() .unwrap() - .try_unary(|v| -> Result<i32> { Self::day_timestamp_micro(v) })?, + .unary(|v| -> i32 { Self::day_timestamp_micro(v) }), DataType::Timestamp(TimeUnit::Nanosecond, _) => input .as_any() .downcast_ref::<TimestampNanosecondArray>() .unwrap() - .try_unary(|v| -> Result<i32> { Self::day_timestamp_nano(v) })?, + .unary(|v| -> i32 { Self::day_timestamp_nano(v) }), DataType::Date32 => input .as_any() .downcast_ref::<Date32Array>() @@ -286,15 +249,13 @@ impl TransformFunction for Day { fn transform_literal(&self, input: &Datum) -> Result<Option<Datum>> { let val = match (input.data_type(), input.literal()) { (PrimitiveType::Date, PrimitiveLiteral::Int(v)) => *v, - (PrimitiveType::Timestamp, PrimitiveLiteral::Long(v)) => Self::day_timestamp_micro(*v)?, + (PrimitiveType::Timestamp, PrimitiveLiteral::Long(v)) => Self::day_timestamp_micro(*v), (PrimitiveType::Timestamptz, PrimitiveLiteral::Long(v)) => { - Self::day_timestamp_micro(*v)? - } - (PrimitiveType::TimestampNs, PrimitiveLiteral::Long(v)) => { - Self::day_timestamp_nano(*v)? + Self::day_timestamp_micro(*v) } + (PrimitiveType::TimestampNs, PrimitiveLiteral::Long(v)) => Self::day_timestamp_nano(*v), (PrimitiveType::TimestamptzNs, PrimitiveLiteral::Long(v)) => { - Self::day_timestamp_nano(*v)? + Self::day_timestamp_nano(*v) } _ => { return Err(Error::new( @@ -1247,6 +1208,46 @@ mod test { Ok(()) } + #[test] + fn test_projection_timestamp_types_day_negative() -> Result<()> { + // 1969-12-30T23:59:59.5 + let micros = -86_400_500_000; + for (field_type, value) in [ + (Timestamp, Datum::timestamp_micros(micros)), + (Timestamptz, Datum::timestamptz_micros(micros)), + (TimestampNs, Datum::timestamp_nanos(micros * 1_000)), + (TimestamptzNs, Datum::timestamptz_nanos(micros * 1_000)), + ] { + let fixture = TestProjectionFixture::new( + Transform::Day, + "name", + NestedField::required(1, "value", Primitive(field_type)), + ); + + fixture.assert_projection( + &fixture.binary_predicate(PredicateOperator::LessThan, value.clone()), + Some("name <= 1969-12-31"), + )?; + + fixture.assert_projection( + &fixture.binary_predicate(PredicateOperator::LessThanOrEq, value.clone()), + Some("name <= 1969-12-31"), + )?; + + fixture.assert_projection( + &fixture.binary_predicate(PredicateOperator::Eq, value.clone()), + Some("name IN (1969-12-31, 1969-12-30)"), + )?; + + fixture.assert_projection( + &fixture.set_predicate(PredicateOperator::In, vec![value]), + Some("name IN (1969-12-31, 1969-12-30)"), + )?; + } + + Ok(()) + } + #[test] fn test_projection_timestamp_day_upper_bound() -> Result<()> { // 17501 @@ -2650,6 +2651,48 @@ mod test { test_timestamp_ns_and_tz_transform("2017-12-01T10:30:42.123000", &day, Datum::date(17501)); } + #[test] + fn test_transform_days_pre_epoch() { + let day = Box::new(super::Day) as BoxedTransformFunction; + let expected = [-1, -2, -2, -2, -2, -366, -365]; + + let micros = vec![ + -500_000, // 1969-12-31T23:59:59.500000 + -86_400_000_001, // 1969-12-30T23:59:59.999999 + -86_400_000_002, // 1969-12-30T23:59:59.999998 + -86_400_500_000, // 1969-12-30T23:59:59.500000 + -86_401_000_000, // 1969-12-30T23:59:59.000000 + -31_536_000_500_000, // 1968-12-31T23:59:59.500000 + -31_535_999_000_001, // 1969-01-01T00:00:00.999999, Iceberg Java gives -366 + ]; + let res = day + .transform(Arc::new(TimestampMicrosecondArray::from(micros.clone()))) + .unwrap(); + let res = res.as_any().downcast_ref::<Date32Array>().unwrap(); + assert_eq!(res.values(), &expected); + for (v, d) in micros.into_iter().zip(expected) { + test_timestamp_and_tz_transform_using_i64(v, &day, Datum::date(d)); + } + + let nanos = vec![ + -500_000_000, // 1969-12-31T23:59:59.500000000 + -86_400_000_000_001, // 1969-12-30T23:59:59.999999999 + -86_400_000_000_002, // 1969-12-30T23:59:59.999999998 + -86_400_500_000_000, // 1969-12-30T23:59:59.500000000 + -86_401_000_000_000, // 1969-12-30T23:59:59.000000000 + -31_536_000_500_000_000, // 1968-12-31T23:59:59.500000000 + -31_535_999_000_000_001, // 1969-01-01T00:00:00.999999999, Iceberg Java gives -366 + ]; + let res = day + .transform(Arc::new(TimestampNanosecondArray::from(nanos.clone()))) + .unwrap(); + let res = res.as_any().downcast_ref::<Date32Array>().unwrap(); + assert_eq!(res.values(), &expected); + for (v, d) in nanos.into_iter().zip(expected) { + test_timestamp_ns_and_tz_transform_using_i64(v, &day, Datum::date(d)); + } + } + #[test] fn test_transform_hours() { let hour = super::Hour;
