Skip to content

fix: fill schema-added nested columns with typed NULL arrays on read #2635

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
laskoviymishka merged 7 commits into from
Oct 6, 2026
Merged

fix: fill schema-added nested columns with typed NULL arrays on read #2635

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
31b5186
fix: fill schema-added nested columns with typed NULL arrays on read
viirya Jun 12, 2026
e845ae0
test(arrow): restructure #2618 nested-null test; trim comments
viirya Jun 30, 2026
cc794c8
Merge upstream main and address nested null-fill review feedback
viirya Sep 9, 2026
fbc8f72
Merge upstream main and reject unsupported complex initial defaults
viirya Sep 22, 2026
47b82ae
Merge upstream main and resolve null-fill conflicts
viirya Oct 4, 2026
a8d823a
test: cover typed null helpers and clarify struct default support
viirya Oct 5, 2026
6c79fc7
test: compare full evolved schema and link default follow-up
viirya Oct 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
207 changes: 199 additions & 8 deletions crates/iceberg/src/arrow/record_batch_transformer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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![

@laskoviymishka laskoviymishka Sep 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s is added here with no initial_default, so null-fill is exactly right. The case this PR quietly makes reachable is a struct added with a non-primitive initial_default (e.g. {}) — the and_then in the transformer drops it to None, so we'd null-fill instead of materializing the spec-mandated non-null struct. Not blocking, but a small pin-test here (even #[ignore]'d) documenting that gap would pair well with the follow-up I noted in the summary.

@viirya viirya Sep 9, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — the extraction currently drops non-primitive defaults to None. Added a TODO at that and_then to keep the gap visible. I'd keep the regression test with the complex-default implementation in a follow-up, where we can cover the non-null struct and child-default semantics together.

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(),
Expand Down
Loading
Loading