anoopj commented on code in PR #3247:
URL: https://github.com/apache/iceberg-rust/pull/3247#discussion_r4076315391
##########
crates/iceberg/src/expr/visitors/page_index_evaluator.rs:
##########
@@ -339,46 +339,62 @@ impl<'a> PageIndexEvaluator<'a> {
)
})
.collect(),
- ColumnIndexMetaData::BYTE_ARRAY(idx) => idx
- .min_values_iter()
- .zip(idx.max_values_iter())
- .enumerate()
- .zip(row_counts.iter())
- .map(|((i, (min, max)), &row_count)| {
- predicate(
- min.map(|val| {
- Datum::new(
- field_type.clone(),
-
PrimitiveLiteral::String(String::from_utf8(val.to_vec()).unwrap()),
- )
- }),
- max.map(|val| {
- Datum::new(
- field_type.clone(),
-
PrimitiveLiteral::String(String::from_utf8(val.to_vec()).unwrap()),
- )
- }),
- PageNullCount::from_row_and_null_counts(row_count,
idx.null_count(i)),
- )
- })
- .collect(),
- ColumnIndexMetaData::FIXED_LEN_BYTE_ARRAY(_) => {
- return Err(Error::new(
- ErrorKind::FeatureUnsupported,
- "unsupported 'FIXED_LEN_BYTE_ARRAY' index type in
column_index",
- ));
+ ColumnIndexMetaData::BYTE_ARRAY(idx) => {
+ // Parquet stores Iceberg string and binary bounds as
BYTE_ARRAY.
+ // Any other field type on a BYTE_ARRAY column (e.g. a non-spec
+ // BYTE_ARRAY decimal) can't be decoded safely, so skip page
+ // pruning for it rather than risk pruning pages that match.
+ if !matches!(field_type, PrimitiveType::String |
PrimitiveType::Binary) {
+ return Ok(None);
+ }
+
+ idx.min_values_iter()
+ .zip(idx.max_values_iter())
+ .enumerate()
+ .zip(row_counts.iter())
+ .map(|((i, (min, max)), &row_count)| {
+ predicate(
+ min.map(|val|
Self::byte_array_bound_to_datum(field_type, val))
+ .transpose()?,
+ max.map(|val|
Self::byte_array_bound_to_datum(field_type, val))
+ .transpose()?,
+ PageNullCount::from_row_and_null_counts(row_count,
idx.null_count(i)),
+ )
+ })
+ .collect()
}
- ColumnIndexMetaData::INT96(_) => {
- return Err(Error::new(
- ErrorKind::FeatureUnsupported,
- "unsupported 'INT96' index type in column_index",
- ));
+ // Column index types we can't interpret: skip page pruning rather
+ // than abort the scan. Row-group filtering and the Arrow row
filter
+ // still apply the predicate.
+ ColumnIndexMetaData::FIXED_LEN_BYTE_ARRAY(_) |
ColumnIndexMetaData::INT96(_) => {
+ return Ok(None);
}
};
Ok(Some(result?))
}
+ /// Converts a `BYTE_ARRAY` page bound into a [`Datum`] according to the
+ /// field's primitive type. Parquet stores Iceberg `string` and `binary`
+ /// bounds as `BYTE_ARRAY`.
+ fn byte_array_bound_to_datum(field_type: &PrimitiveType, bytes: &[u8]) ->
Result<Datum> {
+ match field_type {
+ PrimitiveType::String => {
+ let value = std::str::from_utf8(bytes).map_err(|err| {
+ Error::new(ErrorKind::DataInvalid, "Invalid UTF-8 in
string page bound")
+ .with_source(err)
+ })?;
+ Ok(Datum::string(value))
+ }
+ PrimitiveType::Binary => Ok(Datum::binary(bytes.to_vec())),
+ // Unreachable: callers gate BYTE_ARRAY decoding to string and
binary.
Review Comment:
Reworded
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
