diff --git a/crates/iceberg/src/expr/visitors/strict_projection.rs b/crates/iceberg/src/expr/visitors/strict_projection.rs index ebc6212c76..2188e6df08 100644 --- a/crates/iceberg/src/expr/visitors/strict_projection.rs +++ b/crates/iceberg/src/expr/visitors/strict_projection.rs @@ -2761,7 +2761,7 @@ mod tests { let result = strict_projection.strict_project(&predicate).unwrap(); assert_eq!( result.to_string(), - "((pcol1 < 100) AND (pcol2 < 100)) AND (pcol3 < 10000)".to_string() + "((pcol1 < 100) AND (pcol2 < 100)) AND (pcol3 < 100.00)".to_string() ); // test less or eq @@ -2780,7 +2780,7 @@ mod tests { let result = strict_projection.strict_project(&predicate).unwrap(); assert_eq!( result.to_string(), - "((pcol1 < 100) AND (pcol2 < 100)) AND (pcol3 < 10000)".to_string() + "((pcol1 < 100) AND (pcol2 < 100)) AND (pcol3 < 100.00)".to_string() ); // test greater @@ -2799,7 +2799,7 @@ mod tests { let result = strict_projection.strict_project(&predicate).unwrap(); assert_eq!( result.to_string(), - "((pcol1 > 100) AND (pcol2 > 100)) AND (pcol3 > 10000)".to_string() + "((pcol1 > 100) AND (pcol2 > 100)) AND (pcol3 > 100.00)".to_string() ); // test greater or eq @@ -2818,7 +2818,7 @@ mod tests { let result = strict_projection.strict_project(&predicate).unwrap(); assert_eq!( result.to_string(), - "((pcol1 > 90) AND (pcol2 > 90)) AND (pcol3 > 9990)".to_string() + "((pcol1 > 90) AND (pcol2 > 90)) AND (pcol3 > 99.90)".to_string() ); // test not eq @@ -2837,7 +2837,7 @@ mod tests { let result = strict_projection.strict_project(&predicate).unwrap(); assert_eq!( result.to_string(), - "((pcol1 != 100) AND (pcol2 != 100)) AND (pcol3 != 10000)".to_string() + "((pcol1 != 100) AND (pcol2 != 100)) AND (pcol3 != 100.00)".to_string() ); // test not in @@ -2880,7 +2880,7 @@ mod tests { let result = strict_projection.strict_project(&predicate).unwrap(); assert_eq!( result.to_string(), - "((pcol1 NOT IN (100, 90)) AND (pcol2 NOT IN (100, 90))) AND (pcol3 NOT IN (10000, 10100, 9900))" + "((pcol1 NOT IN (100, 90)) AND (pcol2 NOT IN (100, 90))) AND (pcol3 NOT IN (100.00, 99.00, 101.00))" .to_string() ); @@ -2984,7 +2984,7 @@ mod tests { let result = strict_projection.strict_project(&predicate).unwrap(); assert_eq!( result.to_string(), - "((pcol1 < 90) AND (pcol2 < 90)) AND (pcol3 < 9990)".to_string() + "((pcol1 < 90) AND (pcol2 < 90)) AND (pcol3 < 99.90)".to_string() ); // test less or eq @@ -3003,7 +3003,7 @@ mod tests { let result = strict_projection.strict_project(&predicate).unwrap(); assert_eq!( result.to_string(), - "((pcol1 < 100) AND (pcol2 < 100)) AND (pcol3 < 10000)".to_string() + "((pcol1 < 100) AND (pcol2 < 100)) AND (pcol3 < 100.00)".to_string() ); // test greater @@ -3022,7 +3022,7 @@ mod tests { let result = strict_projection.strict_project(&predicate).unwrap(); assert_eq!( result.to_string(), - "((pcol1 > 90) AND (pcol2 > 90)) AND (pcol3 > 9990)".to_string() + "((pcol1 > 90) AND (pcol2 > 90)) AND (pcol3 > 99.90)".to_string() ); // test greater or eq @@ -3041,7 +3041,7 @@ mod tests { let result = strict_projection.strict_project(&predicate).unwrap(); assert_eq!( result.to_string(), - "((pcol1 > 90) AND (pcol2 > 90)) AND (pcol3 > 9990)".to_string() + "((pcol1 > 90) AND (pcol2 > 90)) AND (pcol3 > 99.90)".to_string() ); // test not eq @@ -3060,7 +3060,7 @@ mod tests { let result = strict_projection.strict_project(&predicate).unwrap(); assert_eq!( result.to_string(), - "((pcol1 != 90) AND (pcol2 != 90)) AND (pcol3 != 9990)".to_string() + "((pcol1 != 90) AND (pcol2 != 90)) AND (pcol3 != 99.90)".to_string() ); // test not in @@ -3103,7 +3103,7 @@ mod tests { let result = strict_projection.strict_project(&predicate).unwrap(); assert_eq!( result.to_string(), - "((pcol1 NOT IN (100, 90)) AND (pcol2 NOT IN (100, 90))) AND (pcol3 NOT IN (9890, 9990, 10090))" + "((pcol1 NOT IN (100, 90)) AND (pcol2 NOT IN (100, 90))) AND (pcol3 NOT IN (98.90, 100.90, 99.90))" .to_string() ); diff --git a/crates/iceberg/src/spec/transform.rs b/crates/iceberg/src/spec/transform.rs index 97ab638e79..587832de5a 100644 --- a/crates/iceberg/src/spec/transform.rs +++ b/crates/iceberg/src/spec/transform.rs @@ -24,7 +24,6 @@ use std::str::FromStr; use fnv::FnvHashSet; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use super::values::decimal_utils::decimal_from_i128_with_scale; use super::{Datum, PrimitiveLiteral}; use crate::ErrorKind; use crate::error::{Error, Result}; @@ -660,8 +659,8 @@ impl Transform { PredicateOperator::LessThan => match (datum.data_type(), datum.literal()) { (PrimitiveType::Int, PrimitiveLiteral::Int(v)) => Some(Datum::int(v - 1)), (PrimitiveType::Long, PrimitiveLiteral::Long(v)) => Some(Datum::long(v - 1)), - (PrimitiveType::Decimal { .. }, PrimitiveLiteral::Int128(v)) => { - Some(Datum::decimal(decimal_from_i128_with_scale(v - 1, 0))?) + (PrimitiveType::Decimal { precision, scale }, PrimitiveLiteral::Int128(v)) => { + Some(Datum::decimal_from_mantissa(v - 1, *precision, *scale)?) } (PrimitiveType::Date, PrimitiveLiteral::Int(v)) => Some(Datum::date(v - 1)), (PrimitiveType::Timestamp, PrimitiveLiteral::Long(v)) => { @@ -681,8 +680,8 @@ impl Transform { PredicateOperator::GreaterThan => match (datum.data_type(), datum.literal()) { (PrimitiveType::Int, PrimitiveLiteral::Int(v)) => Some(Datum::int(v + 1)), (PrimitiveType::Long, PrimitiveLiteral::Long(v)) => Some(Datum::long(v + 1)), - (PrimitiveType::Decimal { .. }, PrimitiveLiteral::Int128(v)) => { - Some(Datum::decimal(decimal_from_i128_with_scale(v + 1, 0))?) + (PrimitiveType::Decimal { precision, scale }, PrimitiveLiteral::Int128(v)) => { + Some(Datum::decimal_from_mantissa(v + 1, *precision, *scale)?) } (PrimitiveType::Date, PrimitiveLiteral::Int(v)) => Some(Datum::date(v + 1)), (PrimitiveType::Timestamp, PrimitiveLiteral::Long(v)) => { @@ -825,8 +824,8 @@ impl Transform { match (datum.data_type(), datum.literal()) { (PrimitiveType::Int, PrimitiveLiteral::Int(v)) => Ok(Datum::int(v + 1)), (PrimitiveType::Long, PrimitiveLiteral::Long(v)) => Ok(Datum::long(v + 1)), - (PrimitiveType::Decimal { .. }, PrimitiveLiteral::Int128(v)) => { - Datum::decimal(decimal_from_i128_with_scale(v + 1, 0)) + (PrimitiveType::Decimal { precision, scale }, PrimitiveLiteral::Int128(v)) => { + Datum::decimal_from_mantissa(v + 1, *precision, *scale) } (PrimitiveType::Date, PrimitiveLiteral::Int(v)) => Ok(Datum::date(v + 1)), (PrimitiveType::Timestamp, PrimitiveLiteral::Long(v)) => { @@ -863,8 +862,8 @@ impl Transform { match (datum.data_type(), datum.literal()) { (PrimitiveType::Int, PrimitiveLiteral::Int(v)) => Ok(Datum::int(v - 1)), (PrimitiveType::Long, PrimitiveLiteral::Long(v)) => Ok(Datum::long(v - 1)), - (PrimitiveType::Decimal { .. }, PrimitiveLiteral::Int128(v)) => { - Datum::decimal(decimal_from_i128_with_scale(v - 1, 0)) + (PrimitiveType::Decimal { precision, scale }, PrimitiveLiteral::Int128(v)) => { + Datum::decimal_from_mantissa(v - 1, *precision, *scale) } (PrimitiveType::Date, PrimitiveLiteral::Int(v)) => Ok(Datum::date(v - 1)), (PrimitiveType::Timestamp, PrimitiveLiteral::Long(v)) => { @@ -1119,4 +1118,20 @@ mod tests { check_boundary(PredicateOperator::GreaterThanOrEq, datum.clone(), datum); } } + + #[test] + fn test_adjust_boundary_preserves_decimal_type() { + let datum = Datum::decimal_from_mantissa(10_000, 9, 2).unwrap(); + + check_boundary( + PredicateOperator::LessThan, + datum.clone(), + Datum::decimal_from_mantissa(9_999, 9, 2).unwrap(), + ); + check_boundary( + PredicateOperator::GreaterThan, + datum, + Datum::decimal_from_mantissa(10_001, 9, 2).unwrap(), + ); + } } diff --git a/crates/iceberg/src/spec/values/datum.rs b/crates/iceberg/src/spec/values/datum.rs index f170a09df5..dceb3f1f5a 100644 --- a/crates/iceberg/src/spec/values/datum.rs +++ b/crates/iceberg/src/spec/values/datum.rs @@ -1102,7 +1102,11 @@ impl Datum { }) } - fn decimal_from_mantissa(mantissa: i128, precision: u32, scale: u32) -> Result { + pub(crate) fn decimal_from_mantissa( + mantissa: i128, + precision: u32, + scale: u32, + ) -> Result { let r#type = Type::decimal(precision, scale)?; if decimal_precision(mantissa) > precision { let value = decimal_from_i128_with_scale(mantissa, scale); diff --git a/crates/iceberg/src/transform/truncate.rs b/crates/iceberg/src/transform/truncate.rs index f60f8e4939..63c6f8ec5c 100644 --- a/crates/iceberg/src/transform/truncate.rs +++ b/crates/iceberg/src/transform/truncate.rs @@ -22,8 +22,7 @@ use arrow_schema::DataType; use super::TransformFunction; use crate::Error; -use crate::spec::decimal_utils::decimal_from_i128_with_scale; -use crate::spec::{Datum, PrimitiveLiteral}; +use crate::spec::{Datum, PrimitiveLiteral, PrimitiveType}; #[derive(Debug)] pub struct Truncate { @@ -163,11 +162,21 @@ impl TransformFunction for Truncate { Datum::long(Self::truncate_i64(*v, width)) })), PrimitiveLiteral::Int128(v) => Ok(Some({ + let PrimitiveType::Decimal { precision, scale } = input.data_type() else { + return Err(Error::new( + crate::ErrorKind::DataInvalid, + format!( + "Expected decimal type for decimal literal, got: {:?}", + input.data_type() + ), + )); + }; let width = self.width as i128; - Datum::decimal(decimal_from_i128_with_scale( + Datum::decimal_from_mantissa( Self::truncate_decimal_i128(*v, width), - 0, - ))? + *precision, + *scale, + )? })), PrimitiveLiteral::String(v) => Ok(Some({ let len = self.width as usize; @@ -343,6 +352,17 @@ mod test { Ok(()) } + #[test] + fn test_truncate_decimal_literal_preserves_type() -> Result<()> { + let input = Datum::decimal_with_precision(decimal_new(9_999, 2), 9)?; + let transformed = super::Truncate::new(10).transform_literal(&input)?.unwrap(); + + assert_eq!(transformed.data_type(), input.data_type()); + assert_eq!(transformed.to_string(), "99.90"); + + Ok(()) + } + #[test] fn test_projection_truncate_upper_bound_decimal() -> Result<()> { let prev = "98.99"; @@ -364,7 +384,7 @@ mod test { fixture.assert_projection( &fixture.binary_predicate(PredicateOperator::LessThan, Datum::decimal_from_str(curr)?), - Some("name <= 9990"), + Some("name <= 99.90"), )?; fixture.assert_projection( @@ -372,7 +392,7 @@ mod test { PredicateOperator::LessThanOrEq, Datum::decimal_from_str(curr)?, ), - Some("name <= 9990"), + Some("name <= 99.90"), )?; fixture.assert_projection( @@ -380,12 +400,12 @@ mod test { PredicateOperator::GreaterThanOrEq, Datum::decimal_from_str(curr)?, ), - Some("name >= 9990"), + Some("name >= 99.90"), )?; fixture.assert_projection( &fixture.binary_predicate(PredicateOperator::Eq, Datum::decimal_from_str(curr)?), - Some("name = 9990"), + Some("name = 99.90"), )?; fixture.assert_projection( @@ -399,7 +419,7 @@ mod test { Datum::decimal_from_str(curr)?, Datum::decimal_from_str(next)?, ]), - Some("name IN (9890, 9990, 10090)"), + Some("name IN (99.90, 100.90, 98.90)"), )?; fixture.assert_projection( @@ -434,7 +454,7 @@ mod test { fixture.assert_projection( &fixture.binary_predicate(PredicateOperator::LessThan, Datum::decimal_from_str(curr)?), - Some("name <= 9990"), + Some("name <= 99.90"), )?; fixture.assert_projection( @@ -442,7 +462,7 @@ mod test { PredicateOperator::LessThanOrEq, Datum::decimal_from_str(curr)?, ), - Some("name <= 10000"), + Some("name <= 100.00"), )?; fixture.assert_projection( @@ -450,12 +470,12 @@ mod test { PredicateOperator::GreaterThanOrEq, Datum::decimal_from_str(curr)?, ), - Some("name >= 10000"), + Some("name >= 100.00"), )?; fixture.assert_projection( &fixture.binary_predicate(PredicateOperator::Eq, Datum::decimal_from_str(curr)?), - Some("name = 10000"), + Some("name = 100.00"), )?; fixture.assert_projection( @@ -469,7 +489,7 @@ mod test { Datum::decimal_from_str(curr)?, Datum::decimal_from_str(next)?, ]), - Some("name IN (10000, 10100, 9900)"), + Some("name IN (99.00, 100.00, 101.00)"), )?; fixture.assert_projection(