From 1a476f76e235f0c5a311f670fe5544edc98ca77e Mon Sep 17 00:00:00 2001 From: raphaelroshan <49832307+raphaelroshan@users.noreply.github.com> Date: Wed, 15 Jul 2026 09:49:34 +0800 Subject: [PATCH] fix: avoid panic when negating MIN-valued literals (#8484) Value::try_negative and the temporal negative() helpers negated with raw unary minus, which panics (debug) or wraps (release) on MIN values such as -i64::MIN. try_negative already returns None for the unsigned arms; make the signed and temporal arms honor that contract via checked_neg / checked_negative so a MIN literal produces a clean error instead. Signed-off-by: raphaelroshan --- src/common/time/src/date.rs | 4 ++ src/common/time/src/duration.rs | 5 +++ src/common/time/src/interval.rs | 19 ++++++++++ src/common/time/src/time.rs | 5 +++ src/common/time/src/timestamp.rs | 5 +++ src/datatypes/src/value.rs | 65 ++++++++++++++++++++++++++------ 6 files changed, 92 insertions(+), 11 deletions(-) diff --git a/src/common/time/src/date.rs b/src/common/time/src/date.rs index bda9105f55..38667abbca 100644 --- a/src/common/time/src/date.rs +++ b/src/common/time/src/date.rs @@ -198,6 +198,10 @@ impl Date { pub fn negative(&self) -> Self { Self(-self.0) } + + pub fn checked_negative(&self) -> Option { + self.0.checked_neg().map(Self) + } } #[cfg(test)] diff --git a/src/common/time/src/duration.rs b/src/common/time/src/duration.rs index a1e582ca82..b34ced336b 100644 --- a/src/common/time/src/duration.rs +++ b/src/common/time/src/duration.rs @@ -97,6 +97,11 @@ impl Duration { self.value = -self.value; self } + + pub fn checked_negative(mut self) -> Option { + self.value = self.value.checked_neg()?; + Some(self) + } } /// Convert i64 to Duration Type. diff --git a/src/common/time/src/interval.rs b/src/common/time/src/interval.rs index 4cb8c45c55..21bdc656d8 100644 --- a/src/common/time/src/interval.rs +++ b/src/common/time/src/interval.rs @@ -80,6 +80,10 @@ impl IntervalYearMonth { Self::new(-self.months) } + pub fn checked_negative(&self) -> Option { + self.months.checked_neg().map(Self::new) + } + pub fn to_iso8601_string(&self) -> String { IntervalFormat::from(*self).to_iso8601_string() } @@ -157,6 +161,13 @@ impl IntervalDayTime { Self::new(-self.days, -self.milliseconds) } + pub fn checked_negative(&self) -> Option { + Some(Self::new( + self.days.checked_neg()?, + self.milliseconds.checked_neg()?, + )) + } + pub fn to_iso8601_string(&self) -> String { IntervalFormat::from(*self).to_iso8601_string() } @@ -278,6 +289,14 @@ impl IntervalMonthDayNano { Self::new(-self.months, -self.days, -self.nanoseconds) } + pub fn checked_negative(&self) -> Option { + Some(Self::new( + self.months.checked_neg()?, + self.days.checked_neg()?, + self.nanoseconds.checked_neg()?, + )) + } + pub fn to_iso8601_string(&self) -> String { IntervalFormat::from(*self).to_iso8601_string() } diff --git a/src/common/time/src/time.rs b/src/common/time/src/time.rs index 00b123f660..a715e2efec 100644 --- a/src/common/time/src/time.rs +++ b/src/common/time/src/time.rs @@ -145,6 +145,11 @@ impl Time { self.value = -self.value; self } + + pub fn checked_negative(mut self) -> Option { + self.value = self.value.checked_neg()?; + Some(self) + } } impl From for Time { diff --git a/src/common/time/src/timestamp.rs b/src/common/time/src/timestamp.rs index 120dc8e1e9..8ad38e280f 100644 --- a/src/common/time/src/timestamp.rs +++ b/src/common/time/src/timestamp.rs @@ -484,6 +484,11 @@ impl Timestamp { self.value = -self.value; self } + + pub fn checked_negative(mut self) -> Option { + self.value = self.value.checked_neg()?; + Some(self) + } } impl Timestamp { diff --git a/src/datatypes/src/value.rs b/src/datatypes/src/value.rs index 5d703480d3..9d50ac78af 100644 --- a/src/datatypes/src/value.rs +++ b/src/datatypes/src/value.rs @@ -543,20 +543,20 @@ impl Value { None } } - Value::Int8(x) => Some(Value::Int8(-*x)), - Value::Int16(x) => Some(Value::Int16(-*x)), - Value::Int32(x) => Some(Value::Int32(-*x)), - Value::Int64(x) => Some(Value::Int64(-*x)), + Value::Int8(x) => x.checked_neg().map(Value::Int8), + Value::Int16(x) => x.checked_neg().map(Value::Int16), + Value::Int32(x) => x.checked_neg().map(Value::Int32), + Value::Int64(x) => x.checked_neg().map(Value::Int64), Value::Float32(x) => Some(Value::Float32(-*x)), Value::Float64(x) => Some(Value::Float64(-*x)), Value::Decimal128(x) => Some(Value::Decimal128(x.negative())), - Value::Date(x) => Some(Value::Date(x.negative())), - Value::Timestamp(x) => Some(Value::Timestamp(x.negative())), - Value::Time(x) => Some(Value::Time(x.negative())), - Value::Duration(x) => Some(Value::Duration(x.negative())), - Value::IntervalYearMonth(x) => Some(Value::IntervalYearMonth(x.negative())), - Value::IntervalDayTime(x) => Some(Value::IntervalDayTime(x.negative())), - Value::IntervalMonthDayNano(x) => Some(Value::IntervalMonthDayNano(x.negative())), + Value::Date(x) => x.checked_negative().map(Value::Date), + Value::Timestamp(x) => x.checked_negative().map(Value::Timestamp), + Value::Time(x) => x.checked_negative().map(Value::Time), + Value::Duration(x) => x.checked_negative().map(Value::Duration), + Value::IntervalYearMonth(x) => x.checked_negative().map(Value::IntervalYearMonth), + Value::IntervalDayTime(x) => x.checked_negative().map(Value::IntervalDayTime), + Value::IntervalMonthDayNano(x) => x.checked_negative().map(Value::IntervalMonthDayNano), Value::Binary(_) | Value::String(_) @@ -1744,6 +1744,49 @@ pub(crate) mod tests { use crate::types::json_type::{JsonNativeType, JsonObjectType}; use crate::vectors::ListVectorBuilder; + #[test] + fn test_try_negative_overflow() { + // Negating a MIN value overflows, so try_negative returns None instead + // of panicking (consistent with the unsigned arms). + assert_eq!(Value::Int8(i8::MIN).try_negative(), None); + assert_eq!(Value::Int16(i16::MIN).try_negative(), None); + assert_eq!(Value::Int32(i32::MIN).try_negative(), None); + assert_eq!(Value::Int64(i64::MIN).try_negative(), None); + assert_eq!( + Value::Timestamp(Timestamp::new_nanosecond(i64::MIN)).try_negative(), + None + ); + assert_eq!(Value::Date(Date::new(i32::MIN)).try_negative(), None); + assert_eq!( + Value::Time(Time::new_nanosecond(i64::MIN)).try_negative(), + None + ); + assert_eq!( + Value::Duration(Duration::new_nanosecond(i64::MIN)).try_negative(), + None + ); + assert_eq!( + Value::IntervalYearMonth(IntervalYearMonth::new(i32::MIN)).try_negative(), + None + ); + assert_eq!( + Value::IntervalDayTime(IntervalDayTime::new(i32::MIN, i32::MIN)).try_negative(), + None + ); + assert_eq!( + Value::IntervalMonthDayNano(IntervalMonthDayNano::new(i32::MIN, i32::MIN, i64::MIN)) + .try_negative(), + None + ); + + // Non-MIN values still negate. + assert_eq!(Value::Int64(5).try_negative(), Some(Value::Int64(-5))); + assert_eq!( + Value::Timestamp(Timestamp::new_nanosecond(5)).try_negative(), + Some(Value::Timestamp(Timestamp::new_nanosecond(-5))) + ); + } + pub(crate) fn build_struct_type() -> StructType { StructType::new(Arc::new(vec![ StructField::new("id".to_string(), ConcreteDataType::int32_datatype(), false),