dannycjones opened a new issue, #3322:
URL: https://github.com/apache/iceberg-rust/issues/3322
### Apache Iceberg Rust version
main
### Describe the bug
Imagine there's a table with some data files. You then evolve the schema to
introduce some new int column `x` with some `initial-default` value of 10.
You have some scan like this:
```sql
SELECT * FROM table WHERE x > 5
```
The initial default is not populated, so x will be treated as `null` rather
than 10.
### To Reproduce
Apply this patch to `main`. It fails today.
```diff
diff --git a/crates/iceberg/src/arrow/reader/row_filter.rs
b/crates/iceberg/src/arrow/reader/row_filter.rs
index 34b739113..2a96212ed 100644
--- a/crates/iceberg/src/arrow/reader/row_filter.rs
+++ b/crates/iceberg/src/arrow/reader/row_filter.rs
@@ -217,7 +217,7 @@ mod tests {
use arrow_array::cast::AsArray;
use arrow_array::{
- ArrayRef, Decimal128Array, Float32Array, Int32Array, Int64Array,
LargeStringArray,
+ Array, ArrayRef, Decimal128Array, Float32Array, Int32Array,
Int64Array, LargeStringArray,
RecordBatch, StringArray,
};
use arrow_schema::{DataType, Field, Schema as ArrowSchema};
@@ -236,7 +236,8 @@ mod tests {
use crate::io::FileIO;
use crate::scan::{FileScanTask, FileScanTaskDeleteFile,
FileScanTaskStream};
use crate::spec::{
- DataContentType, DataFileFormat, Datum, NestedField, PrimitiveType,
Schema, SchemaRef, Type,
+ DataContentType, DataFileFormat, Datum, Literal, NestedField,
PrimitiveType, Schema,
+ SchemaRef, Type,
};
async fn test_perform_read(
@@ -1282,6 +1283,107 @@ mod tests {
);
}
+ /// Spec column projection rule #3
(https://iceberg.apache.org/spec/#column-projection):
+ /// a column absent from a data file resolves to the field's
`initial-default`, not to
+ /// null.
+ ///
+ /// The Arrow row filter is built from the Parquet schema alone, so a
field added by
+ /// schema evolution has no leaf column and
`PredicateConverter::bound_reference`
+ /// returns `None`. Every predicate arm then folds to a constant on the
premise that
+ /// the column is null -- for `x > 5` that fold is always-false, so
parquet-rs discards
+ /// every row during decode, before `RecordBatchTransformer` gets the
chance to
+ /// substitute the default of 10. Nothing re-applies the predicate
afterwards.
+ #[tokio::test]
+ async fn test_predicate_on_missing_column_honours_initial_default() {
+ // The data file predates the schema change: it carries `id` only.
+ let file_arrow_schema = Arc::new(ArrowSchema::new(vec![
+ Field::new("id", DataType::Int32,
false).with_metadata(HashMap::from([(
+ PARQUET_FIELD_ID_META_KEY.to_string(),
+ "1".to_string(),
+ )])),
+ ]));
+
+ let tmp_dir = TempDir::new().unwrap();
+ let file_path = format!("{}/old-data.parquet",
tmp_dir.path().to_str().unwrap());
+
+ let batch = RecordBatch::try_new(file_arrow_schema.clone(), vec![
+ Arc::new(Int32Array::from(vec![1, 2, 3])) as ArrayRef,
+ ])
+ .unwrap();
+
+ let file = File::create(&file_path).unwrap();
+ let props = WriterProperties::builder()
+ .set_compression(Compression::SNAPPY)
+ .build();
+ let mut writer = ArrowWriter::try_new(file, file_arrow_schema,
Some(props)).unwrap();
+ writer.write(&batch).unwrap();
+ writer.close().unwrap();
+
+ // The table schema then grew an `x` column with `initial-default =
10`, so every
+ // pre-existing row reads back as x = 10.
+ let table_schema = Arc::new(
+ Schema::builder()
+ .with_schema_id(2)
+ .with_fields(vec![
+ NestedField::required(1, "id",
Type::Primitive(PrimitiveType::Int)).into(),
+ NestedField::optional(2, "x",
Type::Primitive(PrimitiveType::Int))
+ .with_initial_default(Literal::int(10))
+ .into(),
+ ])
+ .build()
+ .unwrap(),
+ );
+
+ // SELECT * FROM t WHERE x > 5 -- 10 > 5 holds, so all three rows
qualify.
+ let predicate = Reference::new("x")
+ .greater_than(Datum::int(5))
+ .bind(table_schema.clone(), false)
+ .unwrap();
+
+ // Default reader settings: the Arrow `RowFilter` is installed
whenever the task
+ // carries a predicate, independent of row selection or row group
filtering.
+ let reader = ArrowReaderBuilder::new(FileIO::new_with_fs(),
Runtime::current()).build();
+
+ let task = FileScanTask::builder()
+
.with_file_size_in_bytes(std::fs::metadata(&file_path).unwrap().len())
+ .with_start(0)
+ .with_length(0)
+ .with_data_file_path(file_path)
+ .with_data_file_format(DataFileFormat::Parquet)
+ .with_schema(table_schema)
+ .with_project_field_ids(vec![1, 2])
+ .with_predicate(Some(predicate))
+ .with_case_sensitive(false)
+ .build()
+ .unwrap();
+
+ let tasks = Box::pin(futures::stream::iter(vec![Ok(task)])) as
FileScanTaskStream;
+ let batches: Vec<RecordBatch> = reader
+ .read(tasks)
+ .unwrap()
+ .stream()
+ .try_collect()
+ .await
+ .unwrap();
+
+ let rows: Vec<(i32, Option<i32>)> = batches
+ .iter()
+ .flat_map(|b| {
+ let ids =
b.column(0).as_primitive::<arrow_array::types::Int32Type>();
+ let xs =
b.column(1).as_primitive::<arrow_array::types::Int32Type>();
+ (0..b.num_rows())
+ .map(|i| (ids.value(i), (!xs.is_null(i)).then(||
xs.value(i))))
+ .collect::<Vec<_>>()
+ })
+ .collect();
+
+ assert_eq!(
+ rows,
+ vec![(1, Some(10)), (2, Some(10)), (3, Some(10))],
+ "`x > 5` must be evaluated against the initial-default of 10,
not against null"
+ );
+ }
+
// Bloom filter pushdown: on-vs-off equivalence
// Pushdown must never change results. An encoding bug in the probe
shows up as
// rows the bloom filter drops and the row filter keeps, so every case
below
```
### Expected behavior
The initial default should be populated in predicates, not just projections.
In the above test, x > 5 must return true where x's initial-default
specifies 10 and the column is missing from the data file.
### Willingness to contribute
I can contribute a fix for this bug independently
--
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]
