anoopj commented on code in PR #3247:
URL: https://github.com/apache/iceberg-rust/pull/3247#discussion_r4076312377
##########
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);
Review Comment:
Added `tracing::debug!` with field_id/type at all three skip sites
--
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]
