Skip to content

Commit b154603

Browse files
advancedxyadvancedxy
authored
fix(transform): correct decimal and binary truncate literals (#3243)
* fix(transform): correct decimal and binary truncate literals The Arrow array path already preserves decimal precision and scale and supports binary truncation. The literal path instead reset decimal scale to zero and rejected binary values. For example, truncate[50](10.65) returned 1050 instead of 10.50. Align literal truncation with the Arrow array path: preserve the source decimal precision and scale, and return the first W bytes of binary values, or the entire value when shorter. Decimal projection boundary adjustments must also preserve precision and scale to keep their values correct before truncation. * fix(transform): address truncate literal review feedback
1 parent b55cda1 commit b154603

4 files changed

Lines changed: 251 additions & 43 deletions

File tree

‎crates/iceberg/src/expr/visitors/strict_projection.rs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2761,7 +2761,7 @@ mod tests {
27612761
let result = strict_projection.strict_project(&predicate).unwrap();
27622762
assert_eq!(
27632763
result.to_string(),
2764-
"((pcol1 < 100) AND (pcol2 < 100)) AND (pcol3 < 10000)".to_string()
2764+
"((pcol1 < 100) AND (pcol2 < 100)) AND (pcol3 < 100.00)".to_string()
27652765
);
27662766

27672767
// test less or eq
@@ -2780,7 +2780,7 @@ mod tests {
27802780
let result = strict_projection.strict_project(&predicate).unwrap();
27812781
assert_eq!(
27822782
result.to_string(),
2783-
"((pcol1 < 100) AND (pcol2 < 100)) AND (pcol3 < 10000)".to_string()
2783+
"((pcol1 < 100) AND (pcol2 < 100)) AND (pcol3 < 100.00)".to_string()
27842784
);
27852785

27862786
// test greater
@@ -2799,7 +2799,7 @@ mod tests {
27992799
let result = strict_projection.strict_project(&predicate).unwrap();
28002800
assert_eq!(
28012801
result.to_string(),
2802-
"((pcol1 > 100) AND (pcol2 > 100)) AND (pcol3 > 10000)".to_string()
2802+
"((pcol1 > 100) AND (pcol2 > 100)) AND (pcol3 > 100.00)".to_string()
28032803
);
28042804

28052805
// test greater or eq
@@ -2818,7 +2818,7 @@ mod tests {
28182818
let result = strict_projection.strict_project(&predicate).unwrap();
28192819
assert_eq!(
28202820
result.to_string(),
2821-
"((pcol1 > 90) AND (pcol2 > 90)) AND (pcol3 > 9990)".to_string()
2821+
"((pcol1 > 90) AND (pcol2 > 90)) AND (pcol3 > 99.90)".to_string()
28222822
);
28232823

28242824
// test not eq
@@ -2837,7 +2837,7 @@ mod tests {
28372837
let result = strict_projection.strict_project(&predicate).unwrap();
28382838
assert_eq!(
28392839
result.to_string(),
2840-
"((pcol1 != 100) AND (pcol2 != 100)) AND (pcol3 != 10000)".to_string()
2840+
"((pcol1 != 100) AND (pcol2 != 100)) AND (pcol3 != 100.00)".to_string()
28412841
);
28422842

28432843
// test not in
@@ -2880,7 +2880,7 @@ mod tests {
28802880
let result = strict_projection.strict_project(&predicate).unwrap();
28812881
assert_eq!(
28822882
result.to_string(),
2883-
"((pcol1 NOT IN (100, 90)) AND (pcol2 NOT IN (100, 90))) AND (pcol3 NOT IN (10000, 10100, 9900))"
2883+
"((pcol1 NOT IN (100, 90)) AND (pcol2 NOT IN (100, 90))) AND (pcol3 NOT IN (100.00, 99.00, 101.00))"
28842884
.to_string()
28852885
);
28862886

@@ -2984,7 +2984,7 @@ mod tests {
29842984
let result = strict_projection.strict_project(&predicate).unwrap();
29852985
assert_eq!(
29862986
result.to_string(),
2987-
"((pcol1 < 90) AND (pcol2 < 90)) AND (pcol3 < 9990)".to_string()
2987+
"((pcol1 < 90) AND (pcol2 < 90)) AND (pcol3 < 99.90)".to_string()
29882988
);
29892989

29902990
// test less or eq
@@ -3003,7 +3003,7 @@ mod tests {
30033003
let result = strict_projection.strict_project(&predicate).unwrap();
30043004
assert_eq!(
30053005
result.to_string(),
3006-
"((pcol1 < 100) AND (pcol2 < 100)) AND (pcol3 < 10000)".to_string()
3006+
"((pcol1 < 100) AND (pcol2 < 100)) AND (pcol3 < 100.00)".to_string()
30073007
);
30083008

30093009
// test greater
@@ -3022,7 +3022,7 @@ mod tests {
30223022
let result = strict_projection.strict_project(&predicate).unwrap();
30233023
assert_eq!(
30243024
result.to_string(),
3025-
"((pcol1 > 90) AND (pcol2 > 90)) AND (pcol3 > 9990)".to_string()
3025+
"((pcol1 > 90) AND (pcol2 > 90)) AND (pcol3 > 99.90)".to_string()
30263026
);
30273027

30283028
// test greater or eq
@@ -3041,7 +3041,7 @@ mod tests {
30413041
let result = strict_projection.strict_project(&predicate).unwrap();
30423042
assert_eq!(
30433043
result.to_string(),
3044-
"((pcol1 > 90) AND (pcol2 > 90)) AND (pcol3 > 9990)".to_string()
3044+
"((pcol1 > 90) AND (pcol2 > 90)) AND (pcol3 > 99.90)".to_string()
30453045
);
30463046

30473047
// test not eq
@@ -3060,7 +3060,7 @@ mod tests {
30603060
let result = strict_projection.strict_project(&predicate).unwrap();
30613061
assert_eq!(
30623062
result.to_string(),
3063-
"((pcol1 != 90) AND (pcol2 != 90)) AND (pcol3 != 9990)".to_string()
3063+
"((pcol1 != 90) AND (pcol2 != 90)) AND (pcol3 != 99.90)".to_string()
30643064
);
30653065

30663066
// test not in
@@ -3103,7 +3103,7 @@ mod tests {
31033103
let result = strict_projection.strict_project(&predicate).unwrap();
31043104
assert_eq!(
31053105
result.to_string(),
3106-
"((pcol1 NOT IN (100, 90)) AND (pcol2 NOT IN (100, 90))) AND (pcol3 NOT IN (9890, 9990, 10090))"
3106+
"((pcol1 NOT IN (100, 90)) AND (pcol2 NOT IN (100, 90))) AND (pcol3 NOT IN (98.90, 100.90, 99.90))"
31073107
.to_string()
31083108
);
31093109

‎crates/iceberg/src/spec/transform.rs‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ use chrono::Datelike;
2525
use fnv::FnvHashSet;
2626
use serde::{Deserialize, Deserializer, Serialize, Serializer};
2727

28-
use super::values::decimal_utils::decimal_from_i128_with_scale;
2928
use super::values::temporal::date;
3029
use super::{Datum, PrimitiveLiteral};
3130
use crate::ErrorKind;
@@ -762,8 +761,8 @@ impl Transform {
762761
PredicateOperator::LessThan => match (datum.data_type(), datum.literal()) {
763762
(PrimitiveType::Int, PrimitiveLiteral::Int(v)) => Some(Datum::int(v - 1)),
764763
(PrimitiveType::Long, PrimitiveLiteral::Long(v)) => Some(Datum::long(v - 1)),
765-
(PrimitiveType::Decimal { .. }, PrimitiveLiteral::Int128(v)) => {
766-
Some(Datum::decimal(decimal_from_i128_with_scale(v - 1, 0))?)
764+
(PrimitiveType::Decimal { .. }, PrimitiveLiteral::Int128(_)) => {
765+
Some(Self::try_decrement_number(datum)?)
767766
}
768767
(PrimitiveType::Date, PrimitiveLiteral::Int(v)) => Some(Datum::date(v - 1)),
769768
(PrimitiveType::Timestamp, PrimitiveLiteral::Long(v)) => {
@@ -783,8 +782,8 @@ impl Transform {
783782
PredicateOperator::GreaterThan => match (datum.data_type(), datum.literal()) {
784783
(PrimitiveType::Int, PrimitiveLiteral::Int(v)) => Some(Datum::int(v + 1)),
785784
(PrimitiveType::Long, PrimitiveLiteral::Long(v)) => Some(Datum::long(v + 1)),
786-
(PrimitiveType::Decimal { .. }, PrimitiveLiteral::Int128(v)) => {
787-
Some(Datum::decimal(decimal_from_i128_with_scale(v + 1, 0))?)
785+
(PrimitiveType::Decimal { .. }, PrimitiveLiteral::Int128(_)) => {
786+
Some(Self::try_increment_number(datum)?)
788787
}
789788
(PrimitiveType::Date, PrimitiveLiteral::Int(v)) => Some(Datum::date(v + 1)),
790789
(PrimitiveType::Timestamp, PrimitiveLiteral::Long(v)) => {
@@ -934,7 +933,7 @@ impl Transform {
934933
(PrimitiveType::Int, PrimitiveLiteral::Int(v)) => Ok(Datum::int(v + 1)),
935934
(PrimitiveType::Long, PrimitiveLiteral::Long(v)) => Ok(Datum::long(v + 1)),
936935
(PrimitiveType::Decimal { .. }, PrimitiveLiteral::Int128(v)) => {
937-
Datum::decimal(decimal_from_i128_with_scale(v + 1, 0))
936+
datum.with_decimal_mantissa(v + 1)
938937
}
939938
(PrimitiveType::Date, PrimitiveLiteral::Int(v)) => Ok(Datum::date(v + 1)),
940939
(PrimitiveType::Timestamp, PrimitiveLiteral::Long(v)) => {
@@ -972,7 +971,7 @@ impl Transform {
972971
(PrimitiveType::Int, PrimitiveLiteral::Int(v)) => Ok(Datum::int(v - 1)),
973972
(PrimitiveType::Long, PrimitiveLiteral::Long(v)) => Ok(Datum::long(v - 1)),
974973
(PrimitiveType::Decimal { .. }, PrimitiveLiteral::Int128(v)) => {
975-
Datum::decimal(decimal_from_i128_with_scale(v - 1, 0))
974+
datum.with_decimal_mantissa(v - 1)
976975
}
977976
(PrimitiveType::Date, PrimitiveLiteral::Int(v)) => Ok(Datum::date(v - 1)),
978977
(PrimitiveType::Timestamp, PrimitiveLiteral::Long(v)) => {

‎crates/iceberg/src/spec/values/datum.rs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,6 +1099,22 @@ impl Datum {
10991099
}
11001100
}
11011101

1102+
/// Replaces a decimal's unscaled value while preserving its precision and scale.
1103+
///
1104+
/// The value is not checked against precision because transform projections
1105+
/// may use boundaries outside the source type's range.
1106+
pub(crate) fn with_decimal_mantissa(&self, mantissa: i128) -> Result<Self> {
1107+
ensure_data_valid!(
1108+
matches!(self.r#type, PrimitiveType::Decimal { .. }),
1109+
"Expected a decimal datum, got: {:?}",
1110+
self.r#type
1111+
);
1112+
Ok(Self::new(
1113+
self.r#type.clone(),
1114+
PrimitiveLiteral::Int128(mantissa),
1115+
))
1116+
}
1117+
11021118
/// Convert the datum to `target_type`.
11031119
pub fn to(self, target_type: &Type) -> Result<Datum> {
11041120
match target_type {

0 commit comments

Comments
 (0)