mirror of
https://github.com/GreptimeTeam/greptimedb.git
synced 2026-08-18 12:08:22 +00:00
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 <raphaelroshan@gmail.com>
This commit is contained in:
@@ -198,6 +198,10 @@ impl Date {
|
||||
pub fn negative(&self) -> Self {
|
||||
Self(-self.0)
|
||||
}
|
||||
|
||||
pub fn checked_negative(&self) -> Option<Self> {
|
||||
self.0.checked_neg().map(Self)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -97,6 +97,11 @@ impl Duration {
|
||||
self.value = -self.value;
|
||||
self
|
||||
}
|
||||
|
||||
pub fn checked_negative(mut self) -> Option<Self> {
|
||||
self.value = self.value.checked_neg()?;
|
||||
Some(self)
|
||||
}
|
||||
}
|
||||
|
||||
/// Convert i64 to Duration Type.
|
||||
|
||||
@@ -80,6 +80,10 @@ impl IntervalYearMonth {
|
||||
Self::new(-self.months)
|
||||
}
|
||||
|
||||
pub fn checked_negative(&self) -> Option<Self> {
|
||||
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<Self> {
|
||||
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<Self> {
|
||||
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()
|
||||
}
|
||||
|
||||
@@ -145,6 +145,11 @@ impl Time {
|
||||
self.value = -self.value;
|
||||
self
|
||||
}
|
||||
|
||||
pub fn checked_negative(mut self) -> Option<Self> {
|
||||
self.value = self.value.checked_neg()?;
|
||||
Some(self)
|
||||
}
|
||||
}
|
||||
|
||||
impl From<i64> for Time {
|
||||
|
||||
@@ -484,6 +484,11 @@ impl Timestamp {
|
||||
self.value = -self.value;
|
||||
self
|
||||
}
|
||||
|
||||
pub fn checked_negative(mut self) -> Option<Self> {
|
||||
self.value = self.value.checked_neg()?;
|
||||
Some(self)
|
||||
}
|
||||
}
|
||||
|
||||
impl Timestamp {
|
||||
|
||||
+54
-11
@@ -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),
|
||||
|
||||
Reference in New Issue
Block a user