cht42 commented on code in PR #9165:
URL: https://github.com/apache/arrow-rs/pull/9165#discussion_r2691533513
##########
arrow-arith/src/numeric.rs:
##########
@@ -1645,534 +1662,209 @@ mod tests {
}
#[test]
- fn test_date64_to_naive_date_opt_boundary_values() {
+ fn test_date64_to_naive_date_opt_boundaries() {
use arrow_array::types::Date64Type;
- // Date64Type::to_naive_date_opt has boundaries determined by
NaiveDate's supported range.
- // The valid date range is from January 1, -262143 to December 31,
262142 (Gregorian calendar).
-
- let epoch = NaiveDate::from_ymd_opt(1970, 1, 1).unwrap();
- let ms_per_day = 24 * 60 * 60 * 1000i64;
+ const MS_PER_DAY: i64 = 24 * 60 * 60 * 1000;
- // Define the boundary dates using NaiveDate::from_ymd_opt
- let max_valid_date = NaiveDate::from_ymd_opt(262142, 12, 31).unwrap();
- let min_valid_date = NaiveDate::from_ymd_opt(-262143, 1, 1).unwrap();
+ // Verify boundary millisecond values
+ assert_eq!(MAX_VALID_MILLIS, 8210266790400000i64);
+ assert_eq!(MIN_VALID_MILLIS, -8334601228800000i64);
- // Calculate their millisecond values from epoch
- let max_valid_millis = (max_valid_date - epoch).num_milliseconds();
- let min_valid_millis = (min_valid_date - epoch).num_milliseconds();
+ // Valid boundary dates work
+ assert!(Date64Type::to_naive_date_opt(MAX_VALID_MILLIS).is_some());
+ assert!(Date64Type::to_naive_date_opt(MIN_VALID_MILLIS).is_some());
- // Verify these match the expected boundaries in milliseconds
- assert_eq!(
- max_valid_millis, 8210266790400000i64,
- "December 31, 262142 should be 8210266790400000 ms from epoch"
- );
- assert_eq!(
- min_valid_millis, -8334601228800000i64,
- "January 1, -262143 should be -8334601228800000 ms from epoch"
- );
+ // Beyond boundaries fail
+ assert!(Date64Type::to_naive_date_opt(MAX_VALID_MILLIS +
MS_PER_DAY).is_none());
+ assert!(Date64Type::to_naive_date_opt(MIN_VALID_MILLIS -
MS_PER_DAY).is_none());
- // Test that the boundary dates work
- assert!(
- Date64Type::to_naive_date_opt(max_valid_millis).is_some(),
- "December 31, 262142 should return Some"
- );
- assert!(
- Date64Type::to_naive_date_opt(min_valid_millis).is_some(),
- "January 1, -262143 should return Some"
- );
+ // Extreme values fail
+ assert!(Date64Type::to_naive_date_opt(i64::MAX).is_none());
+ assert!(Date64Type::to_naive_date_opt(i64::MIN).is_none());
- // Test that one day beyond the boundaries fails
- assert!(
- Date64Type::to_naive_date_opt(max_valid_millis +
ms_per_day).is_none(),
- "January 1, 262143 should return None"
- );
- assert!(
- Date64Type::to_naive_date_opt(min_valid_millis -
ms_per_day).is_none(),
- "December 31, -262144 should return None"
- );
-
- // Test some values well within the valid range
- assert!(
- Date64Type::to_naive_date_opt(0).is_some(),
- "Epoch (1970-01-01) should return Some"
- );
- let year_2000 = NaiveDate::from_ymd_opt(2000, 1, 1).unwrap();
- let year_2000_millis = (year_2000 - epoch).num_milliseconds();
- assert!(
- Date64Type::to_naive_date_opt(year_2000_millis).is_some(),
- "Year 2000 should return Some"
- );
-
- // Test extreme values that definitely fail due to Duration constraints
- assert!(
- Date64Type::to_naive_date_opt(i64::MAX).is_none(),
- "i64::MAX should return None"
- );
- assert!(
- Date64Type::to_naive_date_opt(i64::MIN).is_none(),
- "i64::MIN should return None"
- );
+ // Common values work
+ assert!(Date64Type::to_naive_date_opt(0).is_some());
+ assert!(Date64Type::to_naive_date_opt(YEAR_2000_MILLIS).is_some());
}
- #[test]
- fn test_date64_add_year_months_opt_boundary_values() {
- use arrow_array::types::Date64Type;
-
- let epoch = NaiveDate::from_ymd_opt(1970, 1, 1).unwrap();
-
- // Test normal case within valid range
- let year_2000 = NaiveDate::from_ymd_opt(2000, 1, 1).unwrap();
- let year_2000_millis = (year_2000 - epoch).num_milliseconds();
- assert!(
- Date64Type::add_year_months_opt(year_2000_millis, 120).is_some(),
- "Adding 10 years to year 2000 should succeed"
- );
-
- // Test with moderate years that are within chrono's safe range
- let large_year = NaiveDate::from_ymd_opt(5000, 1, 1).unwrap();
- let large_year_millis = (large_year - epoch).num_milliseconds();
- assert!(
- Date64Type::add_year_months_opt(large_year_millis, 12).is_some(),
- "Adding 12 months to year 5000 should succeed"
- );
-
- let neg_year = NaiveDate::from_ymd_opt(-5000, 12, 31).unwrap();
- let neg_year_millis = (neg_year - epoch).num_milliseconds();
+ fn test_year_month_op<F>(op: F, op_name: &str)
+ where
+ F: Fn(i64, i32) -> Option<i64>,
+ {
+ // Normal operations succeed
+ assert!(op(YEAR_2000_MILLIS, 120).is_some(), "{op_name}: normal add");
assert!(
- Date64Type::add_year_months_opt(neg_year_millis, -12).is_some(),
- "Subtracting 12 months from year -5000 should succeed"
+ op(YEAR_2000_MILLIS, 0).is_some(),
+ "{op_name}: zero interval"
);
- // Test with extreme input values that would cause overflow
- assert!(
- Date64Type::add_year_months_opt(i64::MAX, 1).is_none(),
- "Adding months to i64::MAX should fail"
- );
- assert!(
- Date64Type::add_year_months_opt(i64::MIN, -1).is_none(),
- "Subtracting months from i64::MIN should fail"
- );
+ // Large but valid years work
+ let large_year = date_to_millis(5000, 1, 1);
+ let neg_year = date_to_millis(-5000, 12, 31);
+ assert!(op(large_year, 12).is_some(), "{op_name}: large year");
+ assert!(op(neg_year, -12).is_some(), "{op_name}: negative year");
- // Test edge case: adding zero should always work for valid dates
- assert!(
- Date64Type::add_year_months_opt(year_2000_millis, 0).is_some(),
- "Adding zero months should always succeed for valid dates"
- );
+ // Extreme base values fail
+ assert!(op(i64::MAX, 1).is_none(), "{op_name}: i64::MAX base");
+ assert!(op(i64::MIN, -1).is_none(), "{op_name}: i64::MIN base");
}
#[test]
- fn test_date64_add_day_time_opt_boundary_values() {
+ fn test_date64_year_month_operations() {
use arrow_array::types::Date64Type;
- use arrow_buffer::IntervalDayTime;
-
- let epoch = NaiveDate::from_ymd_opt(1970, 1, 1).unwrap();
-
- // Test with a date far from the boundary but still testing the
function
- let near_max_date = NaiveDate::from_ymd_opt(200000, 12, 1).unwrap();
- let near_max_millis = (near_max_date - epoch).num_milliseconds();
- // Adding 30 days should succeed
- let interval_30_days = IntervalDayTime::new(30, 0);
- assert!(
- Date64Type::add_day_time_opt(near_max_millis,
interval_30_days).is_some(),
- "Adding 30 days to large year should succeed"
- );
-
- // Adding a very large number of days should fail
- let interval_large_days = IntervalDayTime::new(100000000, 0);
- assert!(
- Date64Type::add_day_time_opt(near_max_millis,
interval_large_days).is_none(),
- "Adding 100M days to large year should fail"
- );
-
- // Test with a date far from the boundary in the negative direction
- let near_min_date = NaiveDate::from_ymd_opt(-200000, 2, 1).unwrap();
- let near_min_millis = (near_min_date - epoch).num_milliseconds();
-
- // Subtracting 30 days should succeed
- let interval_minus_30_days = IntervalDayTime::new(-30, 0);
- assert!(
- Date64Type::add_day_time_opt(near_min_millis,
interval_minus_30_days).is_some(),
- "Subtracting 30 days from large negative year should succeed"
- );
-
- // Subtracting a very large number of days should fail
- let interval_minus_large_days = IntervalDayTime::new(-100000000, 0);
- assert!(
- Date64Type::add_day_time_opt(near_min_millis,
interval_minus_large_days).is_none(),
- "Subtracting 100M days from large negative year should fail"
- );
-
- // Test normal case within valid range
- let year_2000 = NaiveDate::from_ymd_opt(2000, 1, 1).unwrap();
- let year_2000_millis = (year_2000 - epoch).num_milliseconds();
- let interval_1000_days = IntervalDayTime::new(1000, 12345);
- assert!(
- Date64Type::add_day_time_opt(year_2000_millis,
interval_1000_days).is_some(),
- "Adding 1000 days and time to year 2000 should succeed"
- );
-
- // Test with extreme input values that would cause overflow
- let interval_one_day = IntervalDayTime::new(1, 0);
- assert!(
- Date64Type::add_day_time_opt(i64::MAX, interval_one_day).is_none(),
- "Adding interval to i64::MAX should fail"
- );
- assert!(
- Date64Type::add_day_time_opt(i64::MIN, IntervalDayTime::new(-1,
0)).is_none(),
- "Subtracting interval from i64::MIN should fail"
- );
-
- // Test with extreme interval values
- let max_interval = IntervalDayTime::new(i32::MAX, i32::MAX);
- assert!(
- Date64Type::add_day_time_opt(0, max_interval).is_none(),
- "Adding extreme interval should fail"
- );
-
- let min_interval = IntervalDayTime::new(i32::MIN, i32::MIN);
- assert!(
- Date64Type::add_day_time_opt(0, min_interval).is_none(),
- "Adding extreme negative interval should fail"
- );
-
- // Test millisecond overflow within a day
- let large_ms_interval = IntervalDayTime::new(0, i32::MAX);
- assert!(
- Date64Type::add_day_time_opt(year_2000_millis,
large_ms_interval).is_some(),
- "Adding large milliseconds within valid range should succeed"
- );
+ test_year_month_op(Date64Type::add_year_months_opt, "add_year_months");
+ test_year_month_op(Date64Type::subtract_year_months_opt,
"subtract_year_months");
}
- #[test]
- fn test_date64_add_month_day_nano_opt_boundary_values() {
- use arrow_array::types::Date64Type;
- use arrow_buffer::IntervalMonthDayNano;
-
- let epoch = NaiveDate::from_ymd_opt(1970, 1, 1).unwrap();
-
- // Test with a large year that is still within chrono's safe range
- let near_max_date = NaiveDate::from_ymd_opt(5000, 11, 1).unwrap();
- let near_max_millis = (near_max_date - epoch).num_milliseconds();
-
- // Adding 1 month and 30 days should succeed
- let interval_safe = IntervalMonthDayNano::new(1, 30, 0);
- assert!(
- Date64Type::add_month_day_nano_opt(near_max_millis,
interval_safe).is_some(),
- "Adding 1 month 30 days to large year should succeed"
- );
-
- // Test normal case within valid range
- let year_2000 = NaiveDate::from_ymd_opt(2000, 1, 1).unwrap();
- let year_2000_millis = (year_2000 - epoch).num_milliseconds();
-
- // Test edge case: adding zero should always work for valid dates
- let zero_interval = IntervalMonthDayNano::new(0, 0, 0);
- assert!(
- Date64Type::add_month_day_nano_opt(year_2000_millis,
zero_interval).is_some(),
- "Adding zero interval should always succeed for valid dates"
- );
-
- // Test with a negative year that is still within chrono's safe range
- let near_min_date = NaiveDate::from_ymd_opt(-5000, 2, 28).unwrap();
- let near_min_millis = (near_min_date - epoch).num_milliseconds();
-
- // Subtracting 1 month and 30 days should succeed
- let interval_safe_neg = IntervalMonthDayNano::new(-1, -30, 0);
- assert!(
- Date64Type::add_month_day_nano_opt(near_min_millis,
interval_safe_neg).is_some(),
- "Subtracting 1 month 30 days from large negative year should
succeed"
- );
+ fn test_day_time_op<F>(op: F, op_name: &str, is_subtract: bool)
+ where
+ F: Fn(i64, IntervalDayTime) -> Option<i64>,
+ {
+ let near_max = date_to_millis(200000, 12, 1);
+ let near_min = date_to_millis(-200000, 2, 1);
+
+ // Moderate intervals succeed
+ assert!(
+ op(near_max, IntervalDayTime::new(30, 0)).is_some(),
+ "{op_name}: +30 days"
+ );
+ assert!(
+ op(near_min, IntervalDayTime::new(-30, 0)).is_some(),
+ "{op_name}: -30 days"
+ );
+ assert!(
+ op(YEAR_2000_MILLIS, IntervalDayTime::new(1000, 12345)).is_some(),
+ "{op_name}: normal"
+ );
+
+ // Large intervals that would overflow - behavior differs for add vs
subtract
+ // For add: near_max + huge_days overflows, near_min + huge_neg_days
overflows
+ // For subtract: near_min - huge_days overflows, near_max -
huge_neg_days overflows
+ if is_subtract {
Review Comment:
removed, you're right it wasn't useful.
--
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]
