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 f2409e9f5 fix: fill schema-added nested columns with typed NULL arrays
on read (#2635)
f2409e9f5 is described below
commit f2409e9f5eaff1c643985013d927e17fdf16e6ec
Author: Liang-Chi Hsieh <[email protected]>
AuthorDate: Tue Oct 6 17:48:48 2026 +0000
fix: fill schema-added nested columns with typed NULL arrays on read (#2635)
* fix: fill schema-added nested columns with typed NULL arrays on read
When a column of a nested type (list, map, or a struct with nested
children) is added to the table schema after data files were written,
reading those older files failed with "unexpected target column type":
the helpers that materialize missing columns only handled primitive
types plus structs with primitive-only children, via hand-written
per-type NULL branches.
Build the all-NULL column with arrow's new_null_array instead, which
supports every Arrow type (including arbitrarily nested ones), and drop
the per-type NULL branches from create_primitive_array_repeated and
create_primitive_array_single_element.
Closes #2618
* test(arrow): restructure #2618 nested-null test; trim comments
Address review feedback on #2635:
- Extract the evolved schema into `schema_with_added_nested_columns` and
reorder the test to read top-to-bottom as a story (old file -> evolved
schema -> expect typed NULLs), per @kevinjqliu.
- Assert the all-NULL struct still carries its full nested shape (child
fields `a` and the nested list `ys`), locking in that the type-preserving
new_null_array fill keeps nested children.
- Trim the over-explanatory comments on the new_null_array NULL-fill paths
and
note that the literal `1` is new_null_array's row count.
* test: cover typed null helpers and clarify struct default support
* test: compare full evolved schema and link default follow-up
---
.../iceberg/src/arrow/record_batch_transformer.rs | 207 ++++++++++++++++++++-
crates/iceberg/src/arrow/value.rs | 201 +++++++-------------
2 files changed, 269 insertions(+), 139 deletions(-)
diff --git a/crates/iceberg/src/arrow/record_batch_transformer.rs
b/crates/iceberg/src/arrow/record_batch_transformer.rs
index 19976a81c..fe17d336c 100644
--- a/crates/iceberg/src/arrow/record_batch_transformer.rs
+++ b/crates/iceberg/src/arrow/record_batch_transformer.rs
@@ -864,13 +864,24 @@ impl RecordBatchTransformer {
return Err(invalid_data!("Missing required field: {}",
iceberg_field.name));
}
- let default_value =
iceberg_field.initial_default.as_ref().and_then(|lit| {
- if let Literal::Primitive(prim) = lit {
- Some(prim.clone())
- } else {
- None
+ // TODO: Support the spec-defined non-null struct initial
default `{}` by
+ // creating a non-null struct and applying each child's
initial default.
+ // Tracked in
https://github.com/apache/iceberg-rust/issues/3261.
+ let default_value = match
iceberg_field.initial_default.as_ref() {
+ None => None,
+ Some(Literal::Primitive(prim)) => Some(prim.clone()),
+ Some(_) => {
+ return Err(Error::new(
+ ErrorKind::FeatureUnsupported,
+ format!(
+ "Cannot read field {} that is absent from
the data file: \
+ applying a non-primitive initial-default
for {} is not yet \
+ supported",
+ iceberg_field.name,
iceberg_field.field_type
+ ),
+ ));
}
- });
+ };
ColumnSource::Add {
value: default_value,
@@ -1143,12 +1154,14 @@ mod test {
use arrow_cast::cast;
use arrow_schema::{DataType, Field, Schema as ArrowSchema};
- use super::field_with_id;
+ use super::{field_with_id, schema_to_arrow_schema};
use crate::arrow::build_partition_constant;
use crate::arrow::record_batch_transformer::{
RecordBatchTransformer, RecordBatchTransformerBuilder,
};
- use crate::spec::{Literal, NestedField, PrimitiveType, Schema, Struct,
Type};
+ use crate::spec::{
+ ListType, Literal, MapType, NestedField, PrimitiveType, Schema,
Struct, Type,
+ };
/// Helper to extract string values from either StringArray or
RunEndEncoded<StringArray>
/// Returns empty string for null values
@@ -1360,6 +1373,63 @@ mod test {
);
}
+ #[test]
+ fn schema_evolution_absent_struct_with_initial_default_errors() {
+ for required in [false, true] {
+ let added_field = NestedField::new(
+ 2,
+ "added_struct",
+ Type::Struct(crate::spec::StructType::new(vec![
+ NestedField::optional(3, "child",
Type::Primitive(PrimitiveType::Int))
+ .with_initial_default(Literal::int(42))
+ .into(),
+ ])),
+ required,
+ )
+
.with_initial_default(Literal::Struct(Struct::from_iter(vec![None])));
+ let snapshot_schema = Arc::new(
+ Schema::builder()
+ .with_fields(vec![
+ NestedField::required(1, "id",
Type::Primitive(PrimitiveType::Int)).into(),
+ added_field.into(),
+ ])
+ .build()
+ .unwrap(),
+ );
+ let mut transformer =
+ RecordBatchTransformerBuilder::new(snapshot_schema, &[1,
2]).build();
+ let file_schema = Arc::new(ArrowSchema::new(vec![field_with_id(
+ "id",
+ DataType::Int32,
+ false,
+ 1,
+ )]));
+ let file_batch =
+ RecordBatch::try_new(file_schema,
vec![Arc::new(Int32Array::from(vec![1, 2, 3]))])
+ .unwrap();
+
+ let err = transformer
+ .process_record_batch(file_batch)
+ .expect_err(&format!(
+ "required={required}: complex default should be rejected"
+ ));
+ assert_eq!(
+ err.kind(),
+ crate::ErrorKind::FeatureUnsupported,
+ "required={required}"
+ );
+ assert!(
+ err.to_string().contains("Cannot read field added_struct"),
+ "required={required}: {err}"
+ );
+ assert!(
+ err.to_string()
+ .contains("applying a non-primitive initial-default"),
+ "required={required}: {err}"
+ );
+ }
+ }
+
#[test]
fn schema_evolution_adds_struct_column_with_nulls() {
// Test that when a struct column is added after data files are
written,
@@ -1437,6 +1507,127 @@ mod test {
assert!(struct_column.is_null(2));
}
+ /// Evolved table schema for the #2618 regression test: `id` plus three
+ /// later-added optional nested columns — a list, a map, and a struct that
+ /// itself contains a nested list (`ys`). The nested-in-struct list is the
+ /// case a per-type NULL-fill would miss.
+ fn schema_with_added_nested_columns() -> Schema {
+ Schema::builder()
+ .with_schema_id(1)
+ .with_fields(vec![
+ NestedField::required(1, "id",
Type::Primitive(PrimitiveType::Int)).into(),
+ NestedField::optional(
+ 2,
+ "xs",
+ Type::List(ListType {
+ element_field: NestedField::list_element(
+ 3,
+ Type::Primitive(PrimitiveType::Int),
+ false,
+ )
+ .into(),
+ }),
+ )
+ .into(),
+ NestedField::optional(
+ 4,
+ "props",
+ Type::Map(MapType {
+ key_field: NestedField::map_key_element(
+ 5,
+ Type::Primitive(PrimitiveType::String),
+ )
+ .into(),
+ value_field: NestedField::map_value_element(
+ 6,
+ Type::Primitive(PrimitiveType::Int),
+ false,
+ )
+ .into(),
+ }),
+ )
+ .into(),
+ NestedField::optional(
+ 7,
+ "s",
+ Type::Struct(crate::spec::StructType::new(vec![
+ NestedField::optional(8, "a",
Type::Primitive(PrimitiveType::String))
+ .into(),
+ NestedField::optional(
+ 9,
+ "ys",
+ Type::List(ListType {
+ element_field: NestedField::list_element(
+ 10,
+ Type::Primitive(PrimitiveType::Long),
+ false,
+ )
+ .into(),
+ }),
+ )
+ .into(),
+ ])),
+ )
+ .into(),
+ ])
+ .build()
+ .unwrap()
+ }
+
+ #[test]
+ fn schema_evolution_adds_list_map_and_nested_struct_columns_with_nulls() {
+ // Regression test for
https://github.com/apache/iceberg-rust/issues/2618.
+ //
+ // The story the test tells, in order:
+ // 1. An old data file was written with only the `id` column.
+ // 2. The table schema has since evolved, adding optional list / map
/
+ // struct columns (see `schema_with_added_nested_columns`).
+ // 3. Reading the old file against the evolved schema must fill those
+ // absent columns with typed all-NULL arrays — previously this
errored
+ // with "unexpected target column type" for the nested types.
+
+ // (1) The old data file: just `id`.
+ let file_schema = Arc::new(ArrowSchema::new(vec![field_with_id(
+ "id",
+ DataType::Int32,
+ false,
+ 1,
+ )]));
+ let file_batch =
+ RecordBatch::try_new(file_schema,
vec![Arc::new(Int32Array::from(vec![1, 2, 3]))])
+ .unwrap();
+
+ // (2) Read it against the evolved schema, projecting id + the three
added columns.
+ let snapshot_schema = Arc::new(schema_with_added_nested_columns());
+ let projected_iceberg_field_ids = [1, 2, 4, 7];
+ let mut transformer =
+ RecordBatchTransformerBuilder::new(snapshot_schema,
&projected_iceberg_field_ids)
+ .build();
+ let result = transformer.process_record_batch(file_batch).unwrap();
+
+ // (3a) `id` survives unchanged.
+ assert_eq!(result.num_columns(), 4);
+ assert_eq!(result.num_rows(), 3);
+ let id_column = result
+ .column(0)
+ .as_any()
+ .downcast_ref::<Int32Array>()
+ .unwrap();
+ assert_eq!(id_column.values(), &[1, 2, 3]);
+
+ // (3b) The added columns carry the evolved schema's Arrow types,
including nested
+ // field names, nullability, and field IDs, and are all-NULL.
+ let expected_schema =
schema_to_arrow_schema(&schema_with_added_nested_columns()).unwrap();
+ assert_eq!(result.schema().as_ref(), &expected_schema);
+ for (idx, name) in [(1, "xs"), (2, "props"), (3, "s")] {
+ assert_eq!(
+ result.column(idx).null_count(),
+ 3,
+ "added nested column `{name}` should be all-NULL"
+ );
+ }
+ }
+
pub fn source_record_batch() -> RecordBatch {
RecordBatch::try_new(
arrow_schema_promotion_addition_and_renaming_required(),
diff --git a/crates/iceberg/src/arrow/value.rs
b/crates/iceberg/src/arrow/value.rs
index 080d1b99a..7cbebf68c 100644
--- a/crates/iceberg/src/arrow/value.rs
+++ b/crates/iceberg/src/arrow/value.rs
@@ -23,7 +23,7 @@ use arrow_array::{
LargeListArray, LargeStringArray, ListArray, MapArray, StringArray,
StructArray,
Time64MicrosecondArray, TimestampMicrosecondArray,
TimestampNanosecondArray, new_null_array,
};
-use arrow_buffer::{BooleanBuffer, NullBuffer};
+use arrow_buffer::BooleanBuffer;
use arrow_schema::{DataType, FieldRef, TimeUnit};
use uuid::Uuid;
@@ -526,24 +526,23 @@ pub(crate) fn create_primitive_array_single_element(
data_type: &DataType,
prim_lit: Option<&PrimitiveLiteral>,
) -> Result<ArrayRef> {
+ // No value: a single NULL of any (possibly nested) type.
+ if prim_lit.is_none() {
+ return Ok(new_null_array(data_type, 1));
+ }
match (data_type, prim_lit) {
- (DataType::Null, None) => Ok(Arc::new(arrow_array::NullArray::new(1))),
(DataType::Boolean, Some(PrimitiveLiteral::Boolean(v))) => {
Ok(Arc::new(BooleanArray::from(vec![*v])))
}
- (DataType::Boolean, None) =>
Ok(Arc::new(BooleanArray::from(vec![Option::<bool>::None]))),
(DataType::Int32, Some(PrimitiveLiteral::Int(v))) => {
Ok(Arc::new(Int32Array::from(vec![*v])))
}
- (DataType::Int32, None) =>
Ok(Arc::new(Int32Array::from(vec![Option::<i32>::None]))),
(DataType::Date32, Some(PrimitiveLiteral::Int(v))) => {
Ok(Arc::new(Date32Array::from(vec![*v])))
}
- (DataType::Date32, None) =>
Ok(Arc::new(Date32Array::from(vec![Option::<i32>::None]))),
(DataType::Int64, Some(PrimitiveLiteral::Long(v))) => {
Ok(Arc::new(Int64Array::from(vec![*v])))
}
- (DataType::Int64, None) =>
Ok(Arc::new(Int64Array::from(vec![Option::<i64>::None]))),
(DataType::Timestamp(TimeUnit::Microsecond, timezone),
Some(PrimitiveLiteral::Long(v))) => {
let array = TimestampMicrosecondArray::from(vec![*v]);
if let Some(timezone) = timezone {
@@ -552,14 +551,6 @@ pub(crate) fn create_primitive_array_single_element(
Ok(Arc::new(array))
}
}
- (DataType::Timestamp(TimeUnit::Microsecond, timezone), None) => {
- let array =
TimestampMicrosecondArray::from(vec![Option::<i64>::None]);
- if let Some(timezone) = timezone {
- Ok(Arc::new(array.with_timezone(timezone.clone())))
- } else {
- Ok(Arc::new(array))
- }
- }
(DataType::Timestamp(TimeUnit::Nanosecond, timezone),
Some(PrimitiveLiteral::Long(v))) => {
let array = TimestampNanosecondArray::from(vec![*v]);
if let Some(timezone) = timezone {
@@ -568,32 +559,18 @@ pub(crate) fn create_primitive_array_single_element(
Ok(Arc::new(array))
}
}
- (DataType::Timestamp(TimeUnit::Nanosecond, timezone), None) => {
- let array =
TimestampNanosecondArray::from(vec![Option::<i64>::None]);
- if let Some(timezone) = timezone {
- Ok(Arc::new(array.with_timezone(timezone.clone())))
- } else {
- Ok(Arc::new(array))
- }
- }
(DataType::Float32, Some(PrimitiveLiteral::Float(v))) => {
Ok(Arc::new(Float32Array::from(vec![v.0])))
}
- (DataType::Float32, None) =>
Ok(Arc::new(Float32Array::from(vec![Option::<f32>::None]))),
(DataType::Float64, Some(PrimitiveLiteral::Double(v))) => {
Ok(Arc::new(Float64Array::from(vec![v.0])))
}
- (DataType::Float64, None) =>
Ok(Arc::new(Float64Array::from(vec![Option::<f64>::None]))),
(DataType::Utf8, Some(PrimitiveLiteral::String(v))) => {
Ok(Arc::new(StringArray::from(vec![v.as_str()])))
}
- (DataType::Utf8, None) =>
Ok(Arc::new(StringArray::from(vec![Option::<&str>::None]))),
(DataType::Binary, Some(PrimitiveLiteral::Binary(v))) => {
Ok(Arc::new(BinaryArray::from_vec(vec![v.as_slice()])))
}
- (DataType::Binary, None) => Ok(Arc::new(BinaryArray::from_opt_vec(vec![
- Option::<&[u8]>::None,
- ]))),
(DataType::Decimal128(precision, scale),
Some(PrimitiveLiteral::Int128(v))) => {
let array = Decimal128Array::from(vec![{ *v }])
.with_precision_and_scale(*precision, *scale)
@@ -614,78 +591,6 @@ pub(crate) fn create_primitive_array_single_element(
})?;
Ok(Arc::new(array))
}
- (DataType::Decimal128(precision, scale), None) => {
- let array = Decimal128Array::from(vec![Option::<i128>::None])
- .with_precision_and_scale(*precision, *scale)
- .map_err(|e| {
- invalid_data!(
- "Failed to create Decimal128Array with precision
{precision} and scale {scale}: {e}"
- )
- })?;
- Ok(Arc::new(array))
- }
- (DataType::Struct(fields), None) => {
- // Create a single-element StructArray with nulls
- let null_arrays: Vec<ArrayRef> = fields
- .iter()
- .map(|f| {
- // Recursively create null arrays for struct fields
- // For primitive fields in structs, use simple null arrays
(not REE within struct)
- match f.data_type() {
- DataType::Boolean => {
-
Ok(Arc::new(BooleanArray::from(vec![Option::<bool>::None]))
- as ArrayRef)
- }
- DataType::Int32 | DataType::Date32 => {
-
Ok(Arc::new(Int32Array::from(vec![Option::<i32>::None])) as ArrayRef)
- }
- DataType::Int64 => {
-
Ok(Arc::new(Int64Array::from(vec![Option::<i64>::None])) as ArrayRef)
- }
- DataType::Timestamp(TimeUnit::Microsecond, timezone)
=> {
- let array =
TimestampMicrosecondArray::from(vec![Option::<i64>::None]);
- if let Some(timezone) = timezone {
-
Ok(Arc::new(array.with_timezone(timezone.clone())) as ArrayRef)
- } else {
- Ok(Arc::new(array) as ArrayRef)
- }
- }
- DataType::Timestamp(TimeUnit::Nanosecond, timezone) =>
{
- let array =
TimestampNanosecondArray::from(vec![Option::<i64>::None]);
- if let Some(timezone) = timezone {
-
Ok(Arc::new(array.with_timezone(timezone.clone())) as ArrayRef)
- } else {
- Ok(Arc::new(array) as ArrayRef)
- }
- }
- DataType::Float32 => {
-
Ok(Arc::new(Float32Array::from(vec![Option::<f32>::None])) as ArrayRef)
- }
- DataType::Float64 => {
-
Ok(Arc::new(Float64Array::from(vec![Option::<f64>::None])) as ArrayRef)
- }
- DataType::Utf8 => {
-
Ok(Arc::new(StringArray::from(vec![Option::<&str>::None])) as ArrayRef)
- }
- DataType::Binary => {
- Ok(
-
Arc::new(BinaryArray::from_opt_vec(vec![Option::<&[u8]>::None]))
- as ArrayRef,
- )
- }
- _ => Err(Error::new(
- ErrorKind::Unexpected,
- format!("Unsupported struct field type: {:?}",
f.data_type()),
- )),
- }
- })
- .collect::<Result<Vec<_>>>()?;
- Ok(Arc::new(StructArray::new(
- fields.clone(),
- null_arrays,
- Some(NullBuffer::new_null(1)),
- )))
- }
_ => Err(Error::new(
ErrorKind::Unexpected,
format!("Unsupported constant type combination: {data_type:?} with
{prim_lit:?}"),
@@ -702,6 +607,10 @@ pub(crate) fn create_primitive_array_repeated(
prim_lit: Option<&PrimitiveLiteral>,
num_rows: usize,
) -> Result<ArrayRef> {
+ // No value to repeat: an all-NULL column of any (possibly nested) type.
+ if prim_lit.is_none() {
+ return Ok(new_null_array(data_type, num_rows));
+ }
Ok(match (data_type, prim_lit) {
// --- Primitive Some arms ---
(DataType::Boolean, Some(PrimitiveLiteral::Boolean(value))) => {
@@ -805,37 +714,6 @@ pub(crate) fn create_primitive_array_repeated(
)
}
- // --- Special-case None arms ---
- (DataType::Decimal128(precision, scale), None) => {
- let vals: Vec<Option<i128>> = vec![None; num_rows];
- Arc::new(
- Decimal128Array::from(vals)
- .with_precision_and_scale(*precision, *scale)
- .map_err(|e| {
- invalid_data!(
- "Failed to create Decimal128Array with
precision {precision} and scale {scale}: {e}"
- )
- })?,
- )
- }
- (DataType::Struct(fields), None) => {
- // Create a StructArray filled with nulls, recursively creating
null children
- let null_arrays: Vec<ArrayRef> = fields
- .iter()
- .map(|field|
create_primitive_array_repeated(field.data_type(), None, num_rows))
- .collect::<Result<Vec<_>>>()?;
-
- Arc::new(StructArray::new(
- fields.clone(),
- null_arrays,
- Some(NullBuffer::new_null(num_rows)),
- ))
- }
- (DataType::Null, None) =>
Arc::new(arrow_array::NullArray::new(num_rows)),
-
- // --- Catch-all null arm: use arrow-rs new_null_array for any
remaining DataType ---
- (dt, None) => new_null_array(dt, num_rows),
-
(dt, _) => {
return Err(Error::new(
ErrorKind::Unexpected,
@@ -1754,6 +1632,67 @@ mod test {
}
}
+ #[test]
+ fn test_create_null_arrays_preserve_type_and_length() {
+ let data_types = [
+ DataType::Decimal128(10, 2),
+ DataType::Timestamp(TimeUnit::Microsecond, Some("+00:00".into())),
+ DataType::Struct(
+ vec![
+ Field::new("a", DataType::Utf8, true),
+ Field::new(
+ "ys",
+ DataType::List(Arc::new(Field::new("element",
DataType::Int64, true))),
+ true,
+ ),
+ ]
+ .into(),
+ ),
+ DataType::Null,
+ ];
+
+ // NullArray has no validity bitmap; logical_null_count includes its
implicit NULLs.
+ for data_type in data_types {
+ let single = create_primitive_array_single_element(&data_type,
None)
+ .unwrap_or_else(|err| panic!("single, type={data_type:?}:
{err}"));
+ assert_eq!(single.data_type(), &data_type, "single,
type={data_type:?}");
+ assert_eq!(single.len(), 1, "single, type={data_type:?}");
+ if data_type != DataType::Null {
+ assert_eq!(single.null_count(), 1, "single,
type={data_type:?}");
+ }
+ assert_eq!(single.logical_null_count(), 1, "single,
type={data_type:?}");
+
+ for num_rows in [0, 1, 3] {
+ let repeated = create_primitive_array_repeated(&data_type,
None, num_rows)
+ .unwrap_or_else(|err| {
+ panic!("repeated, type={data_type:?}, rows={num_rows}:
{err}")
+ });
+ assert_eq!(
+ repeated.data_type(),
+ &data_type,
+ "repeated, type={data_type:?}, rows={num_rows}"
+ );
+ assert_eq!(
+ repeated.len(),
+ num_rows,
+ "repeated, type={data_type:?}, rows={num_rows}"
+ );
+ if data_type != DataType::Null {
+ assert_eq!(
+ repeated.null_count(),
+ num_rows,
+ "repeated, type={data_type:?}, rows={num_rows}"
+ );
+ }
+ assert_eq!(
+ repeated.logical_null_count(),
+ num_rows,
+ "repeated, type={data_type:?}, rows={num_rows}"
+ );
+ }
+ }
+ }
+
#[test]
fn test_create_null_array_rejects_non_null_literal() {
let literal = Some(PrimitiveLiteral::Int(1));
