You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by gi...@apache.org on 2026/10/12 08:49:03 UTC
(iceberg-rust) branch gh-readonly-queue/main/pr-3364-acd610997de78f986ece9f89c3bf476de5e1f5a5 created (now 693339072)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-3364-acd610997de78f986ece9f89c3bf476de5e1f5a5
in repository https://gitbox.apache.org/repos/asf/iceberg-rust.git
at 693339072 fix(arrow): decode short BYTE_ARRAY decimal row-group stats (#3364)
This branch includes the following new commits:
new 693339072 fix(arrow): decode short BYTE_ARRAY decimal row-group stats (#3364)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(iceberg-rust) 01/01: fix(arrow): decode short BYTE_ARRAY decimal row-group stats (#3364)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-3364-acd610997de78f986ece9f89c3bf476de5e1f5a5
in repository https://gitbox.apache.org/repos/asf/iceberg-rust.git
commit 693339072b87eebf075479393a8ef55182f27aca
Author: DeviousCardi <11...@users.noreply.github.com>
AuthorDate: Mon Oct 12 08:48:39 2026 +0000
fix(arrow): decode short BYTE_ARRAY decimal row-group stats (#3364)
get_parquet_stat_min_as_datum / get_parquet_stat_max_as_datum decoded
BYTE_ARRAY decimal bounds with a strict 16-byte conversion. Parquet
stores these as minimal-length big-endian two's-complement values, so
any bound shorter than 16 bytes failed to convert and aborted the
row-group read.
Use the variable-length, sign-extending i128_from_be_bytes helper, as
the FIXED_LEN_BYTE_ARRAY arm already does. Bounds longer than 16 bytes
still return DataInvalid. Empty bounds also return DataInvalid instead
of decoding as zero, which could otherwise prune matching row groups.
Closes #3280
---
crates/iceberg/src/arrow/schema.rs | 95 +++++++++++++++++++++++++++++++++++++-
1 file changed, 93 insertions(+), 2 deletions(-)
diff --git a/crates/iceberg/src/arrow/schema.rs b/crates/iceberg/src/arrow/schema.rs
index 996ced09e..0c673c6bc 100644
--- a/crates/iceberg/src/arrow/schema.rs
+++ b/crates/iceberg/src/arrow/schema.rs
@@ -893,7 +893,13 @@ pub(crate) fn get_parquet_stat_min_as_datum(
};
Some(Datum::new(
primitive_type.clone(),
- PrimitiveLiteral::Int128(i128::from_be_bytes(bytes.try_into()?)),
+ PrimitiveLiteral::Int128(
+ // An empty bound carries no value; reject it rather than
+ // decode it as zero, which could prune matching row groups.
+ i128_from_be_bytes(bytes)
+ .filter(|_| !bytes.is_empty())
+ .ok_or_else(|| invalid_data!("Can't convert bytes to i128: {bytes:?}"))?,
+ ),
))
}
(
@@ -1037,7 +1043,13 @@ pub(crate) fn get_parquet_stat_max_as_datum(
};
Some(Datum::new(
primitive_type.clone(),
- PrimitiveLiteral::Int128(i128::from_be_bytes(bytes.try_into()?)),
+ PrimitiveLiteral::Int128(
+ // An empty bound carries no value; reject it rather than
+ // decode it as zero, which could prune matching row groups.
+ i128_from_be_bytes(bytes)
+ .filter(|_| !bytes.is_empty())
+ .ok_or_else(|| invalid_data!("Can't convert bytes to i128: {bytes:?}"))?,
+ ),
))
}
(
@@ -2625,6 +2637,85 @@ mod tests {
assert_eq!(schema.highest_field_id(), 17);
}
+ fn decimal_byte_array_stats(min: Vec<u8>, max: Vec<u8>) -> Statistics {
+ Statistics::byte_array(
+ Some(parquet::data_type::ByteArray::from(min)),
+ Some(parquet::data_type::ByteArray::from(max)),
+ None,
+ Some(0),
+ false,
+ )
+ }
+
+ fn decimal_bounds(stats: &Statistics) -> Result<(Option<Datum>, Option<Datum>)> {
+ let ty = PrimitiveType::Decimal {
+ precision: 38,
+ scale: 2,
+ };
+ Ok((
+ get_parquet_stat_min_as_datum(&ty, stats)?,
+ get_parquet_stat_max_as_datum(&ty, stats)?,
+ ))
+ }
+
+ fn decimal_datum(value: i128) -> Datum {
+ Datum::new(
+ PrimitiveType::Decimal {
+ precision: 38,
+ scale: 2,
+ },
+ PrimitiveLiteral::Int128(value),
+ )
+ }
+
+ #[test]
+ fn test_byte_array_decimal_stats_short_bounds() {
+ // Minimal-length big-endian two's-complement: -1234 and 1234.
+ let stats = decimal_byte_array_stats(vec![0xFB, 0x2E], vec![0x04, 0xD2]);
+ let (min, max) = decimal_bounds(&stats).unwrap();
+ assert_eq!(min, Some(decimal_datum(-1234)));
+ assert_eq!(max, Some(decimal_datum(1234)));
+
+ // Single-byte bounds: sign bit set must sign-extend, not zero-extend.
+ let stats = decimal_byte_array_stats(vec![0x80], vec![0x7F]);
+ let (min, max) = decimal_bounds(&stats).unwrap();
+ assert_eq!(min, Some(decimal_datum(-128)));
+ assert_eq!(max, Some(decimal_datum(127)));
+ }
+
+ #[test]
+ fn test_byte_array_decimal_stats_full_width_bounds() {
+ let stats = decimal_byte_array_stats(
+ i128::MIN.to_be_bytes().to_vec(),
+ i128::MAX.to_be_bytes().to_vec(),
+ );
+ let (min, max) = decimal_bounds(&stats).unwrap();
+ assert_eq!(min, Some(decimal_datum(i128::MIN)));
+ assert_eq!(max, Some(decimal_datum(i128::MAX)));
+ }
+
+ #[test]
+ fn test_byte_array_decimal_stats_empty_bound_errors() {
+ let stats = decimal_byte_array_stats(vec![], vec![0x01]);
+ let err = decimal_bounds(&stats).unwrap_err();
+ assert_eq!(err.kind(), ErrorKind::DataInvalid);
+
+ let stats = decimal_byte_array_stats(vec![0x01], vec![]);
+ let err = decimal_bounds(&stats).unwrap_err();
+ assert_eq!(err.kind(), ErrorKind::DataInvalid);
+ }
+
+ #[test]
+ fn test_byte_array_decimal_stats_oversized_bound_errors() {
+ let stats = decimal_byte_array_stats(vec![0; 17], vec![0x01]);
+ let err = decimal_bounds(&stats).unwrap_err();
+ assert_eq!(err.kind(), ErrorKind::DataInvalid);
+
+ let stats = decimal_byte_array_stats(vec![0x01], vec![0; 17]);
+ let err = decimal_bounds(&stats).unwrap_err();
+ assert_eq!(err.kind(), ErrorKind::DataInvalid);
+ }
+
/// Every field is rebuilt without metadata, at any depth and inside any
/// container. Fields are rebuilt in nesting order rather than looked up by
/// name, so a path that repeats names (`a.b.a.b`) keeps each level's name