diff --git a/crates/iceberg/src/spec/values/datum.rs b/crates/iceberg/src/spec/values/datum.rs index 68ea6b3d46..7236cff2f6 100644 --- a/crates/iceberg/src/spec/values/datum.rs +++ b/crates/iceberg/src/spec/values/datum.rs @@ -29,8 +29,8 @@ use serde::{Deserialize, Serialize}; use serde_bytes::ByteBuf; use super::decimal_utils::{ - Decimal, decimal_from_i128_with_scale, decimal_from_str_exact, decimal_mantissa, decimal_scale, - i128_from_be_bytes, i128_to_be_bytes_min, + Decimal, decimal_from_i128_with_scale, decimal_from_str_exact, decimal_mantissa, + decimal_precision, decimal_scale, i128_from_be_bytes, i128_to_be_bytes_min, }; use super::literal::Literal; use super::primitive::PrimitiveLiteral; @@ -1053,24 +1053,7 @@ impl Datum { let scale = decimal_scale(&value); let mantissa = decimal_mantissa(&value); - let available_bytes = Type::decimal_required_bytes(precision)? as usize; - let actual_bytes = i128_to_be_bytes_min(mantissa); - if actual_bytes.len() > available_bytes { - return Err(Error::new( - ErrorKind::DataInvalid, - format!("Decimal value {value} is too large for precision {precision}"), - )); - } - - let r#type = Type::decimal(precision, scale)?; - if let Type::Primitive(p) = r#type { - Ok(Self { - r#type: p, - literal: PrimitiveLiteral::Int128(mantissa), - }) - } else { - unreachable!("Decimal type must be primitive.") - } + Self::decimal_from_mantissa(mantissa, precision, scale, value) } fn i64_to_i32 + PartialOrd>(val: T) -> Datum { @@ -1109,6 +1092,30 @@ impl Datum { }) } + fn decimal_from_mantissa( + mantissa: i128, + precision: u32, + scale: u32, + value: T, + ) -> Result { + let r#type = Type::decimal(precision, scale)?; + if decimal_precision(mantissa) > precision { + return Err(Error::new( + ErrorKind::DataInvalid, + format!("Decimal value {value} is too large for precision {precision}"), + )); + } + + if let Type::Primitive(p) = r#type { + Ok(Self { + r#type: p, + literal: PrimitiveLiteral::Int128(mantissa), + }) + } else { + unreachable!("Decimal type must be primitive.") + } + } + /// Convert the datum to `target_type`. pub fn to(self, target_type: &Type) -> Result { match target_type { @@ -1130,6 +1137,21 @@ impl Datum { (PrimitiveLiteral::Int128(val), _, PrimitiveType::Long) => { Ok(Datum::i128_to_i64(*val)) } + ( + PrimitiveLiteral::Int128(val), + PrimitiveType::Decimal { + scale: self_scale, .. + }, + PrimitiveType::Decimal { + precision, + scale: target_scale, + }, + ) if self_scale == target_scale => Datum::decimal_from_mantissa( + *val, + *precision, + *target_scale, + self.to_human_string(), + ), (PrimitiveLiteral::String(val), _, PrimitiveType::Boolean) => { Datum::bool_from_str(val) diff --git a/crates/iceberg/src/spec/values/decimal_utils.rs b/crates/iceberg/src/spec/values/decimal_utils.rs index 88e3f72f65..31a8248ef1 100644 --- a/crates/iceberg/src/spec/values/decimal_utils.rs +++ b/crates/iceberg/src/spec/values/decimal_utils.rs @@ -120,6 +120,11 @@ pub fn decimal_mantissa(d: &Decimal) -> i128 { } } +/// Get the decimal digit precision of a mantissa. +pub fn decimal_precision(mantissa: i128) -> u32 { + mantissa.unsigned_abs().to_string().len() as u32 +} + /// Get the scale (number of digits after decimal point). /// /// This is equivalent to rust_decimal's `decimal.scale()`. @@ -232,6 +237,18 @@ mod tests { assert_eq!(decimal_mantissa(&d), -12345); } + #[test] + fn test_decimal_precision() { + assert_eq!(decimal_precision(0), 1); + assert_eq!(decimal_precision(5), 1); + assert_eq!(decimal_precision(42), 2); + assert_eq!(decimal_precision(-42), 2); + assert_eq!( + decimal_precision(99999999999999999999999999999999999999), + 38 + ); + } + #[test] fn test_decimal_scale() { let d = decimal_from_i128_with_scale(12345, 2); diff --git a/crates/iceberg/src/spec/values/tests.rs b/crates/iceberg/src/spec/values/tests.rs index d3e0a455c2..38781753db 100644 --- a/crates/iceberg/src/spec/values/tests.rs +++ b/crates/iceberg/src/spec/values/tests.rs @@ -460,8 +460,8 @@ fn avro_bytes_decimal() { (vec![251u8, 46u8], -1234, 2, 38), (vec![4u8, 210u8], 1234, 3, 38), (vec![251u8, 46u8], -1234, 3, 38), - (vec![42u8], 42, 2, 1), - (vec![214u8], -42, 2, 1), + (vec![42u8], 42, 2, 2), + (vec![214u8], -42, 2, 2), ]; for (input_bytes, decimal_num, expect_scale, expect_precision) in cases { @@ -1424,3 +1424,115 @@ fn test_date_from_json_as_number() { // Both formats should produce the same Literal value } + +#[test] +fn test_datum_to_decimal_narrows_precision_when_scale_matches() { + let target_type = Type::Primitive(PrimitiveType::Decimal { + precision: 9, + scale: 2, + }); + let datum = Datum::decimal_from_str("123.45").unwrap(); + + let converted = datum.to(&target_type).unwrap(); + + assert_eq!(converted.data_type(), &PrimitiveType::Decimal { + precision: 9, + scale: 2, + }); + assert_eq!(converted.literal(), &PrimitiveLiteral::Int128(12345)); +} + +#[test] +fn test_datum_to_decimal_widens_precision_when_scale_matches() { + let target_type = Type::Primitive(PrimitiveType::Decimal { + precision: 38, + scale: 2, + }); + let datum = Datum::decimal_with_precision(decimal_from_i128_with_scale(12345, 2), 9).unwrap(); + + let converted = datum.to(&target_type).unwrap(); + + assert_eq!(converted.data_type(), &PrimitiveType::Decimal { + precision: 38, + scale: 2, + }); + assert_eq!(converted.literal(), &PrimitiveLiteral::Int128(12345)); +} + +#[test] +fn test_datum_to_decimal_rejects_precision_too_narrow() { + let target_type = Type::Primitive(PrimitiveType::Decimal { + precision: 1, + scale: 2, + }); + let datum = Datum::decimal_from_str("123.45").unwrap(); + + let result = datum.to(&target_type); + + assert!(result.is_err(), "expect error but got {result:?}"); + assert_eq!(result.unwrap_err().kind(), ErrorKind::DataInvalid); +} + +#[test] +fn test_datum_to_decimal_rejects_value_that_fits_storage_bytes_but_not_precision() { + let target_type = Type::Primitive(PrimitiveType::Decimal { + precision: 1, + scale: 2, + }); + let datum = Datum::decimal_from_str("0.42").unwrap(); + + let result = datum.to(&target_type); + + assert!(result.is_err(), "expect error but got {result:?}"); + assert_eq!(result.unwrap_err().kind(), ErrorKind::DataInvalid); +} + +#[test] +fn test_datum_to_decimal_accepts_single_digit_mantissa_for_precision_one() { + let target_type = Type::Primitive(PrimitiveType::Decimal { + precision: 1, + scale: 2, + }); + let datum = Datum::decimal_from_str("0.05").unwrap(); + + let converted = datum.to(&target_type).unwrap(); + + assert_eq!(converted.data_type(), &PrimitiveType::Decimal { + precision: 1, + scale: 2, + }); + assert_eq!(converted.literal(), &PrimitiveLiteral::Int128(5)); +} + +#[test] +fn test_datum_decimal_with_precision_rejects_value_that_exceeds_digit_precision() { + let result = Datum::decimal_with_precision(decimal_from_i128_with_scale(42, 2), 1); + + assert!(result.is_err(), "expect error but got {result:?}"); + assert_eq!(result.unwrap_err().kind(), ErrorKind::DataInvalid); +} + +#[test] +fn test_datum_decimal_with_precision_accepts_value_that_fits_digit_precision() { + let datum = Datum::decimal_with_precision(decimal_from_i128_with_scale(5, 2), 1).unwrap(); + + assert_eq!(datum.data_type(), &PrimitiveType::Decimal { + precision: 1, + scale: 2, + }); + assert_eq!(datum.literal(), &PrimitiveLiteral::Int128(5)); +} + +#[test] +fn test_datum_to_decimal_rejects_scale_change() { + let target_type = Type::Primitive(PrimitiveType::Decimal { + precision: 9, + scale: 3, + }); + let datum = Datum::decimal_from_str("123.45").unwrap(); + + let result = datum.to(&target_type); + + assert!(result.is_err(), "expect error but got {result:?}"); + assert_eq!(result.unwrap_err().kind(), ErrorKind::DataInvalid); +}