This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-rust.git
The following commit(s) were added to refs/heads/main by this push:
new b154603a7 fix(transform): correct decimal and binary truncate literals
(#3243)
b154603a7 is described below
commit b154603a710d730d2f43f8af4db257cc137237ef
Author: advancedxy <[email protected]>
AuthorDate: Mon Oct 12 19:41:02 2026 +0000
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
---
.../iceberg/src/expr/visitors/strict_projection.rs | 24 +-
crates/iceberg/src/spec/transform.rs | 13 +-
crates/iceberg/src/spec/values/datum.rs | 16 ++
crates/iceberg/src/transform/truncate.rs | 241 +++++++++++++++++++--
4 files changed, 251 insertions(+), 43 deletions(-)
diff --git a/crates/iceberg/src/expr/visitors/strict_projection.rs
b/crates/iceberg/src/expr/visitors/strict_projection.rs
index ebc6212c7..2188e6df0 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 d87054ab7..c28dc28c1 100644
--- a/crates/iceberg/src/spec/transform.rs
+++ b/crates/iceberg/src/spec/transform.rs
@@ -25,7 +25,6 @@ use chrono::Datelike;
use fnv::FnvHashSet;
use serde::{Deserialize, Deserializer, Serialize, Serializer};
-use super::values::decimal_utils::decimal_from_i128_with_scale;
use super::values::temporal::date;
use super::{Datum, PrimitiveLiteral};
use crate::ErrorKind;
@@ -762,8 +761,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 { .. }, PrimitiveLiteral::Int128(_))
=> {
+ Some(Self::try_decrement_number(datum)?)
}
(PrimitiveType::Date, PrimitiveLiteral::Int(v)) =>
Some(Datum::date(v - 1)),
(PrimitiveType::Timestamp, PrimitiveLiteral::Long(v)) => {
@@ -783,8 +782,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 { .. }, PrimitiveLiteral::Int128(_))
=> {
+ Some(Self::try_increment_number(datum)?)
}
(PrimitiveType::Date, PrimitiveLiteral::Int(v)) =>
Some(Datum::date(v + 1)),
(PrimitiveType::Timestamp, PrimitiveLiteral::Long(v)) => {
@@ -934,7 +933,7 @@ impl Transform {
(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))
+ datum.with_decimal_mantissa(v + 1)
}
(PrimitiveType::Date, PrimitiveLiteral::Int(v)) =>
Ok(Datum::date(v + 1)),
(PrimitiveType::Timestamp, PrimitiveLiteral::Long(v)) => {
@@ -972,7 +971,7 @@ impl Transform {
(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))
+ datum.with_decimal_mantissa(v - 1)
}
(PrimitiveType::Date, PrimitiveLiteral::Int(v)) =>
Ok(Datum::date(v - 1)),
(PrimitiveType::Timestamp, PrimitiveLiteral::Long(v)) => {
diff --git a/crates/iceberg/src/spec/values/datum.rs
b/crates/iceberg/src/spec/values/datum.rs
index 57d967573..c4c835f61 100644
--- a/crates/iceberg/src/spec/values/datum.rs
+++ b/crates/iceberg/src/spec/values/datum.rs
@@ -1099,6 +1099,22 @@ impl Datum {
}
}
+ /// Replaces a decimal's unscaled value while preserving its precision and
scale.
+ ///
+ /// The value is not checked against precision because transform
projections
+ /// may use boundaries outside the source type's range.
+ pub(crate) fn with_decimal_mantissa(&self, mantissa: i128) -> Result<Self>
{
+ ensure_data_valid!(
+ matches!(self.r#type, PrimitiveType::Decimal { .. }),
+ "Expected a decimal datum, got: {:?}",
+ self.r#type
+ );
+ Ok(Self::new(
+ self.r#type.clone(),
+ PrimitiveLiteral::Int128(mantissa),
+ ))
+ }
+
/// Convert the datum to `target_type`.
pub fn to(self, target_type: &Type) -> Result<Datum> {
match target_type {
diff --git a/crates/iceberg/src/transform/truncate.rs
b/crates/iceberg/src/transform/truncate.rs
index 0f95854ee..2bb7fdf22 100644
--- a/crates/iceberg/src/transform/truncate.rs
+++ b/crates/iceberg/src/transform/truncate.rs
@@ -23,8 +23,7 @@ use arrow_schema::DataType;
use super::TransformFunction;
use crate::Error;
use crate::error::invalid_data;
-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 {
@@ -159,15 +158,20 @@ impl TransformFunction for Truncate {
})),
PrimitiveLiteral::Int128(v) => Ok(Some({
let width = self.width as i128;
- Datum::decimal(decimal_from_i128_with_scale(
- Self::truncate_decimal_i128(*v, width),
- 0,
- ))?
+ input.with_decimal_mantissa(Self::truncate_decimal_i128(*v,
width))?
})),
PrimitiveLiteral::String(v) => Ok(Some({
let len = self.width as usize;
Datum::string(Self::truncate_str(v, len).to_string())
})),
+ // Fixed shares this literal variant with Binary but does not
support truncate.
+ PrimitiveLiteral::Binary(v) if matches!(input.data_type(),
PrimitiveType::Binary) => {
+ Ok(Some(Datum::binary(
+ Self::truncate_binary(v, self.width as usize)
+ .iter()
+ .copied(),
+ )))
+ }
_ => Err(Error::new(
crate::ErrorKind::FeatureUnsupported,
format!(
@@ -194,8 +198,7 @@ mod test {
TimestampNs, Timestamptz, TimestamptzNs, Uuid,
};
use crate::spec::Type::{Primitive, Struct};
- use crate::spec::decimal_utils::decimal_new;
- use crate::spec::{Datum, NestedField, PrimitiveType, StructType,
Transform};
+ use crate::spec::{Datum, NestedField, PrimitiveLiteral, PrimitiveType,
StructType, Transform};
use crate::transform::TransformFunction;
use crate::transform::test::{TestProjectionFixture, TestTransformFixture};
@@ -250,6 +253,128 @@ mod test {
fixture.assert_transform(trans);
}
+ #[test]
+ fn test_projection_truncate_binary() -> Result<()> {
+ let fixture = TestProjectionFixture::new(
+ Transform::Truncate(3),
+ "name",
+ NestedField::required(1, "value", Primitive(Binary)),
+ );
+
+ for (value, prefix) in [
+ (vec![0x00, 0xff, 0x80, 0x01], "00FF80"),
+ (vec![0x00, 0xff, 0x80], "00FF80"),
+ (vec![0x00, 0xff], "00FF"),
+ ] {
+ for (op, projected_op) in [
+ (PredicateOperator::Eq, "="),
+ (PredicateOperator::LessThanOrEq, "<="),
+ ] {
+ fixture.assert_projection(
+ &fixture.binary_predicate(op,
Datum::binary(value.iter().copied())),
+ Some(&format!("name {projected_op} {prefix}")),
+ )?;
+ }
+ }
+
+ // Both source values share one truncated prefix, so the projected set
has one value.
+ fixture.assert_projection(
+ &fixture.set_predicate(PredicateOperator::In, vec![
+ Datum::binary([0x00, 0xff, 0x80, 0x01]),
+ Datum::binary([0x00, 0xff, 0x80, 0x02]),
+ ]),
+ Some("name IN (00FF80)"),
+ )?;
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_strict_projection_truncate_binary() -> Result<()> {
+ let transform = Transform::Truncate(3);
+ let fixture = TestProjectionFixture::new(
+ transform,
+ "name",
+ NestedField::required(1, "value", Primitive(Binary)),
+ );
+
+ for (op, expected) in [
+ (PredicateOperator::LessThanOrEq, Some("name < 00FF80")),
+ (PredicateOperator::NotEq, Some("name != 00FF80")),
+ (PredicateOperator::Eq, None),
+ ] {
+ let predicate = fixture.binary_predicate(op, Datum::binary([0x00,
0xff, 0x80, 0x01]));
+ let result = transform.strict_project("name", &predicate)?;
+ assert_eq!(result.map(|p| p.to_string()).as_deref(), expected);
+ }
+
+ for (op, expected) in [
+ (PredicateOperator::NotIn, Some("name NOT IN (00FF80)")),
+ (PredicateOperator::In, None),
+ ] {
+ let predicate = fixture.set_predicate(op, vec![
+ Datum::binary([0x00, 0xff, 0x80, 0x01]),
+ Datum::binary([0x00, 0xff, 0x80, 0x02]),
+ ]);
+ let result = transform.strict_project("name", &predicate)?;
+ assert_eq!(result.map(|p| p.to_string()).as_deref(), expected);
+ }
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_projection_truncate_decimal_outside_precision() -> Result<()> {
+ let transform = Transform::Truncate(10);
+ let fixture = TestProjectionFixture::new(
+ transform,
+ "name",
+ NestedField::required(
+ 1,
+ "value",
+ Primitive(Decimal {
+ precision: 2,
+ scale: 1,
+ }),
+ ),
+ );
+
+ for (value, inclusive_op, inclusive_expected, strict_op,
strict_expected) in [
+ (
+ 99,
+ PredicateOperator::GreaterThan,
+ "name >= 10.0",
+ PredicateOperator::LessThanOrEq,
+ "name < 10.0",
+ ),
+ (
+ -99,
+ PredicateOperator::LessThan,
+ "name <= -10.0",
+ PredicateOperator::GreaterThanOrEq,
+ "name > -10.0",
+ ),
+ ] {
+ let datum = Datum::new(
+ Decimal {
+ precision: 2,
+ scale: 1,
+ },
+ PrimitiveLiteral::Int128(value),
+ );
+ fixture.assert_projection(
+ &fixture.binary_predicate(inclusive_op, datum.clone()),
+ Some(inclusive_expected),
+ )?;
+ let result = transform
+ .strict_project("name", &fixture.binary_predicate(strict_op,
datum))?
+ .unwrap();
+ assert_eq!(result.to_string(), strict_expected);
+ }
+
+ Ok(())
+ }
+
#[test]
fn test_projection_truncate_string_rewrite_op() -> Result<()> {
let value = "abcde";
@@ -359,7 +484,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(
@@ -367,7 +492,7 @@ mod test {
PredicateOperator::LessThanOrEq,
Datum::decimal_from_str(curr)?,
),
- Some("name <= 9990"),
+ Some("name <= 99.90"),
)?;
fixture.assert_projection(
@@ -375,12 +500,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(
@@ -394,7 +519,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(
@@ -429,7 +554,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(
@@ -437,7 +562,7 @@ mod test {
PredicateOperator::LessThanOrEq,
Datum::decimal_from_str(curr)?,
),
- Some("name <= 10000"),
+ Some("name <= 100.00"),
)?;
fixture.assert_projection(
@@ -445,12 +570,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(
@@ -464,7 +589,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(
@@ -795,6 +920,34 @@ mod test {
assert_eq!(super::Truncate::truncate_str(test4, 1), test4_1_expected);
}
+ #[test]
+ fn test_literal_spec_examples() -> Result<()> {
+ // https://iceberg.apache.org/spec/#truncate-transform-details
+ let cases = [
+ (10, Datum::int(1), Datum::int(0)),
+ (10, Datum::int(-1), Datum::int(-10)),
+ (10, Datum::long(1), Datum::long(0)),
+ (10, Datum::long(-1), Datum::long(-10)),
+ (
+ 50,
+ Datum::decimal_from_str("10.65")?,
+ Datum::decimal_from_str("10.50")?,
+ ),
+ (3, Datum::string("iceberg"), Datum::string("ice")),
+ (
+ 3,
+ Datum::binary(vec![0x01, 0x02, 0x03, 0x04, 0x05]),
+ Datum::binary(vec![0x01, 0x02, 0x03]),
+ ),
+ ];
+
+ for (width, input, expected) in cases {
+ let result =
super::Truncate::new(width).transform_literal(&input)?;
+ assert_eq!(result, Some(expected), "width: {width}, input:
{input:?}");
+ }
+ Ok(())
+ }
+
#[test]
fn test_literal_int() {
let input = Datum::int(1);
@@ -831,12 +984,52 @@ mod test {
#[test]
fn test_decimal_literal() {
- let input = Datum::decimal(decimal_new(1065, 0)).unwrap();
- let res = super::Truncate::new(50)
- .transform_literal(&input)
- .unwrap()
- .unwrap();
- assert_eq!(res, Datum::decimal(decimal_new(1050, 0)).unwrap(),);
+ for (precision, scale, value, expected) in [
+ (9, 0, 1065, 1050),
+ (9, 2, 1065, 1050),
+ (9, 2, -1065, -1100),
+ (9, 2, 1050, 1050),
+ (9, 2, 0, 0),
+ (9, 5, 1065, 1050),
+ (38, 2, 1065, 1050),
+ ] {
+ let data_type = Decimal { precision, scale };
+ let input = Datum::new(data_type.clone(),
PrimitiveLiteral::Int128(value));
+ let res = super::Truncate::new(50)
+ .transform_literal(&input)
+ .unwrap()
+ .unwrap();
+ assert_eq!(
+ res,
+ Datum::new(data_type, PrimitiveLiteral::Int128(expected)),
+ "input: {input:?}"
+ );
+ }
+ }
+
+ #[test]
+ fn test_binary_literal() {
+ for (value, width, expected) in [
+ (vec![0x00, 0xff, 0x80, 0x01], 2, vec![0x00, 0xff]),
+ (vec![0x00, 0xff], 2, vec![0x00, 0xff]),
+ (vec![0x00, 0xff], 4, vec![0x00, 0xff]),
+ (vec![], 2, vec![]),
+ ] {
+ let input = Datum::binary(value);
+ let res = super::Truncate::new(width)
+ .transform_literal(&input)
+ .unwrap()
+ .unwrap();
+ assert_eq!(res, Datum::binary(expected));
+ }
+ }
+
+ #[test]
+ fn test_fixed_literal_is_unsupported() {
+ let error = super::Truncate::new(2)
+ .transform_literal(&Datum::fixed(vec![0x00, 0xff, 0x80]))
+ .unwrap_err();
+ assert_eq!(error.kind(), crate::ErrorKind::FeatureUnsupported);
}
#[test]
