You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by gi...@apache.org on 2026/10/11 18:29:44 UTC
(iceberg-rust) branch gh-readonly-queue/main/pr-3303-2fddb1601a9af1c97a559baa9865e1494d93756c created (now 9092073f9)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-3303-2fddb1601a9af1c97a559baa9865e1494d93756c
in repository https://gitbox.apache.org/repos/asf/iceberg-rust.git
at 9092073f9 fix(arrow): strip metadata from nested fields and normalize container field names (#3303)
This branch includes the following new commits:
new 9092073f9 fix(arrow): strip metadata from nested fields and normalize container field names (#3303)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(iceberg-rust) 01/01: fix(arrow): strip metadata from nested fields and normalize container field names (#3303)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-3303-2fddb1601a9af1c97a559baa9865e1494d93756c
in repository https://gitbox.apache.org/repos/asf/iceberg-rust.git
commit 9092073f988b3e6401d4ca00e2b50b7a53434ee1
Author: NoahKusaba <97...@users.noreply.github.com>
AuthorDate: Sun Oct 11 18:20:15 2026 +0000
fix(arrow): strip metadata from nested fields and normalize container field names (#3303)
* fix(arrow): strip metadata from list elements and map entries
strip_metadata_from_schema failed with "Field stack underflow in list" on
any schema with a list or map column. MetadataStripVisitor pushed a field
onto its stack only in before_field, which the visitor does not call for
list elements, map keys or map values, so rebuilding those popped from an
empty stack. Push in before_list_element, before_map_key and
before_map_value too.
The stack now holds each field's name and nullability rather than a
Field with a placeholder type, and one pop_field helper rebuilds fields
for struct, list, map and primitive.
Document the two normalizations the function makes besides removing
metadata, which callers comparing stripped schemas rely on: a map's
entries field is renamed to DEFAULT_MAP_FIELD_NAME, and a
dictionary-encoded field becomes its value type.
Closes #3297
* test(arrow): strip metadata from fields with repeated nested names
* refactor(arrow): rename MetadataStripVisitor's field stack helpers to start_field and finish_field
* docs(arrow): clarify visitor field hooks and strip_metadata_from_schema naming
before_field and after_field run for every schema and struct field, not
for list elements or map keys and values. strip_metadata_from_schema
keeps list element names, and renames only a map's entries field.
* normalize list and map field names when stripping metadata
---
crates/iceberg/src/arrow/schema.rs | 388 +++++++++++++++++++++++++++++++------
1 file changed, 325 insertions(+), 63 deletions(-)
diff --git a/crates/iceberg/src/arrow/schema.rs b/crates/iceberg/src/arrow/schema.rs
index 338f1776d..996ced09e 100644
--- a/crates/iceberg/src/arrow/schema.rs
+++ b/crates/iceberg/src/arrow/schema.rs
@@ -37,8 +37,9 @@ use uuid::Uuid;
use crate::error::{Result, invalid_data};
use crate::spec::decimal_utils::i128_from_be_bytes;
use crate::spec::{
- Datum, FIRST_FIELD_ID, ListType, MapType, NestedField, NestedFieldRef, PrimitiveLiteral,
- PrimitiveType, Schema, SchemaVisitor, StructType, Type, VariantType,
+ Datum, FIRST_FIELD_ID, LIST_FIELD_NAME, ListType, MAP_KEY_FIELD_NAME, MAP_VALUE_FIELD_NAME,
+ MapType, NestedField, NestedFieldRef, PrimitiveLiteral, PrimitiveType, Schema, SchemaVisitor,
+ StructType, Type, VariantType,
};
use crate::{Error, ErrorKind};
@@ -110,12 +111,16 @@ pub trait ArrowSchemaVisitor {
/// Return type of this visitor on arrow schema.
type U;
- /// Called before struct/list/map field.
+ /// Called before each field of the schema or of a struct, whatever its type.
+ /// List elements and map keys and values use [`Self::before_list_element`],
+ /// [`Self::before_map_key`] and [`Self::before_map_value`] instead.
fn before_field(&mut self, _field: &FieldRef) -> Result<()> {
Ok(())
}
- /// Called after struct/list/map field.
+ /// Called after each field of the schema or of a struct, whatever its type.
+ /// List elements and map keys and values use [`Self::after_list_element`],
+ /// [`Self::after_map_key`] and [`Self::after_map_value`] instead.
fn after_field(&mut self, _field: &FieldRef) -> Result<()> {
Ok(())
}
@@ -1198,16 +1203,43 @@ pub(crate) fn primitive_type_to_arrow_type_with_ree(primitive_type: &PrimitiveTy
/// including nested struct, list, and map fields. This is useful for schema comparison
/// where metadata differences should be ignored.
struct MetadataStripVisitor {
- /// Stack to track field information during traversal
- field_stack: Vec<Field>,
+ /// Fields whose types are still being visited, innermost last
+ pending_fields: Vec<PendingField>,
+}
+
+/// What `MetadataStripVisitor` keeps of a field until its type is visited
+struct PendingField {
+ name: String,
+ nullable: bool,
}
impl MetadataStripVisitor {
fn new() -> Self {
Self {
- field_stack: Vec::new(),
+ pending_fields: Vec::new(),
}
}
+
+ /// Records a field whose type is about to be visited.
+ fn start_field(&mut self, name: &str, nullable: bool) -> Result<()> {
+ self.pending_fields.push(PendingField {
+ name: name.to_string(),
+ nullable,
+ });
+ Ok(())
+ }
+
+ /// Builds the innermost pending field with `data_type` and no metadata.
+ /// `kind` names the type in the error for an unbalanced stack.
+ fn finish_field(&mut self, data_type: DataType, kind: &str) -> Result<Field> {
+ let PendingField { name, nullable } = self.pending_fields.pop().ok_or_else(|| {
+ Error::new(
+ ErrorKind::Unexpected,
+ format!("Field stack underflow in {kind}"),
+ )
+ })?;
+ Ok(Field::new(name, data_type, nullable))
+ }
}
impl ArrowSchemaVisitor for MetadataStripVisitor {
@@ -1215,46 +1247,40 @@ impl ArrowSchemaVisitor for MetadataStripVisitor {
type U = ArrowSchema;
fn before_field(&mut self, field: &FieldRef) -> Result<()> {
- // Store field name and nullability for later reconstruction
- self.field_stack.push(Field::new(
- field.name(),
- DataType::Null, // Placeholder, will be replaced
- field.is_nullable(),
- ));
- Ok(())
+ self.start_field(field.name(), field.is_nullable())
}
- fn after_field(&mut self, _field: &FieldRef) -> Result<()> {
- Ok(())
+ fn before_list_element(&mut self, field: &FieldRef) -> Result<()> {
+ self.start_field(LIST_FIELD_NAME, field.is_nullable())
+ }
+
+ fn before_map_key(&mut self, _field: &FieldRef) -> Result<()> {
+ // Map keys are always required, as `schema_to_arrow_schema` builds them.
+ self.start_field(MAP_KEY_FIELD_NAME, false)
+ }
+
+ fn before_map_value(&mut self, field: &FieldRef) -> Result<()> {
+ self.start_field(MAP_VALUE_FIELD_NAME, field.is_nullable())
}
fn schema(&mut self, _schema: &ArrowSchema, values: Vec<Self::T>) -> Result<Self::U> {
+ if !self.pending_fields.is_empty() {
+ return Err(Error::new(
+ ErrorKind::Unexpected,
+ format!(
+ "{} fields left on the field stack after traversal",
+ self.pending_fields.len()
+ ),
+ ));
+ }
Ok(ArrowSchema::new(values))
}
fn r#struct(&mut self, _fields: &Fields, results: Vec<Self::T>) -> Result<Self::T> {
- // Pop the struct field from the stack
- let field_info = self
- .field_stack
- .pop()
- .ok_or_else(|| Error::new(ErrorKind::Unexpected, "Field stack underflow in struct"))?;
-
- // Reconstruct struct field without metadata
- Ok(Field::new(
- field_info.name(),
- DataType::Struct(Fields::from(results)),
- field_info.is_nullable(),
- ))
+ self.finish_field(DataType::Struct(Fields::from(results)), "struct")
}
fn list(&mut self, list: &DataType, value: Self::T) -> Result<Self::T> {
- // Pop the list field from the stack
- let field_info = self
- .field_stack
- .pop()
- .ok_or_else(|| Error::new(ErrorKind::Unexpected, "Field stack underflow in list"))?;
-
- // Reconstruct list field without metadata
let list_type = match list {
DataType::List(_) => DataType::List(Arc::new(value)),
DataType::LargeList(_) => DataType::LargeList(Arc::new(value)),
@@ -1267,20 +1293,10 @@ impl ArrowSchemaVisitor for MetadataStripVisitor {
}
};
- Ok(Field::new(
- field_info.name(),
- list_type,
- field_info.is_nullable(),
- ))
+ self.finish_field(list_type, "list")
}
fn map(&mut self, map: &DataType, key_value: Self::T, value: Self::T) -> Result<Self::T> {
- // Pop the map field from the stack
- let field_info = self
- .field_stack
- .pop()
- .ok_or_else(|| Error::new(ErrorKind::Unexpected, "Field stack underflow in map"))?;
-
// Reconstruct the map's struct field (contains key and value)
let struct_field = Field::new(
DEFAULT_MAP_FIELD_NAME,
@@ -1299,26 +1315,11 @@ impl ArrowSchemaVisitor for MetadataStripVisitor {
}
};
- // Reconstruct map field without metadata
- Ok(Field::new(
- field_info.name(),
- DataType::Map(Arc::new(struct_field), sorted),
- field_info.is_nullable(),
- ))
+ self.finish_field(DataType::Map(Arc::new(struct_field), sorted), "map")
}
fn primitive(&mut self, p: &DataType) -> Result<Self::T> {
- // Pop the primitive field from the stack
- let field_info = self.field_stack.pop().ok_or_else(|| {
- Error::new(ErrorKind::Unexpected, "Field stack underflow in primitive")
- })?;
-
- // Return field without metadata
- Ok(Field::new(
- field_info.name(),
- p.clone(),
- field_info.is_nullable(),
- ))
+ self.finish_field(p.clone(), "primitive")
}
}
@@ -1328,6 +1329,21 @@ impl ArrowSchemaVisitor for MetadataStripVisitor {
/// including nested struct, list, and map fields. This is useful for schema comparison
/// where metadata differences should be ignored.
///
+/// Some other differences are normalized away too, so that schemas that differ only in
+/// them compare equal once stripped. The fields inside lists and maps are renamed as
+/// [`schema_to_arrow_schema`] names them, and dictionaries are unwrapped:
+/// - A list's element field is renamed to [`LIST_FIELD_NAME`], so `item` (the arrow-rs
+/// default) becomes `element`.
+/// - A map's entries field is renamed to [`DEFAULT_MAP_FIELD_NAME`] and made non-nullable.
+/// - A map's key and value fields are renamed to [`MAP_KEY_FIELD_NAME`] and
+/// [`MAP_VALUE_FIELD_NAME`], so `keys` and `values` (the arrow-rs `MapBuilder` default)
+/// become `key` and `value`, and the key is made non-nullable.
+/// - A dictionary-encoded field becomes a field of the dictionary's value type.
+///
+/// The names of the schema's fields and of struct fields are kept. Extension type
+/// metadata (`ARROW:extension:*`) is dropped with the rest, so a variant field becomes a
+/// plain struct of its `metadata` and `value` fields.
+///
/// # Arguments
/// * `schema` - The Arrow schema to strip metadata from
///
@@ -2608,4 +2624,250 @@ mod tests {
pretty_assertions::assert_eq!(schema, expected);
assert_eq!(schema.highest_field_id(), 17);
}
+
+ /// Every field is rebuilt without metadata, at any depth and inside any
+ /// container. Fields are rebuilt in nesting order rather than looked up by
+ /// name, so a path that repeats names (`a.b.a.b`) keeps each level's name
+ /// and nullability.
+ #[test]
+ fn test_strip_metadata_from_nested_schema() {
+ // The same schema, built with and without metadata on the schema and every field.
+ let schema = |with_metadata: bool| {
+ let metadata = |value: &str| {
+ if with_metadata {
+ HashMap::from([("k".to_string(), value.to_string())])
+ } else {
+ HashMap::new()
+ }
+ };
+ let field = |name: &str, ty: DataType, nullable: bool| {
+ Field::new(name, ty, nullable).with_metadata(metadata(name))
+ };
+ let element = |ty: DataType| Arc::new(field(LIST_FIELD_NAME, ty, false));
+ let ints = || DataType::List(element(DataType::Int32));
+ let map = |value: DataType, sorted: bool| {
+ let entries = Fields::from(vec![
+ field(MAP_KEY_FIELD_NAME, DataType::Utf8, false),
+ field(MAP_VALUE_FIELD_NAME, value, true),
+ ]);
+ let entries = field(DEFAULT_MAP_FIELD_NAME, DataType::Struct(entries), false);
+ DataType::Map(Arc::new(entries), sorted)
+ };
+ let struct_of = |fields: Vec<Field>| DataType::Struct(Fields::from(fields));
+
+ let a_b_a_b = struct_of(vec![field(
+ "b",
+ struct_of(vec![field(
+ "a",
+ struct_of(vec![field("b", DataType::Int32, true)]),
+ false,
+ )]),
+ false,
+ )]);
+ ArrowSchema::new(vec![
+ field("list", ints(), true),
+ field(
+ "large_list",
+ DataType::LargeList(element(DataType::Int32)),
+ false,
+ ),
+ field(
+ "fixed_list",
+ DataType::FixedSizeList(element(DataType::Int32), 2),
+ true,
+ ),
+ field("list_of_list", DataType::List(element(ints())), false),
+ field("sorted_map", map(ints(), true), false),
+ field(
+ "map_of_struct",
+ map(
+ struct_of(vec![
+ field("a", DataType::Int32, true),
+ field("b", ints(), false),
+ ]),
+ false,
+ ),
+ true,
+ ),
+ field(
+ "list_of_struct",
+ DataType::List(element(struct_of(vec![field(
+ "m",
+ map(ints(), false),
+ true,
+ )]))),
+ true,
+ ),
+ field("a", a_b_a_b, true),
+ ])
+ .with_metadata(metadata("schema"))
+ };
+
+ let (input, expected) = (schema(true), schema(false));
+ // Otherwise a builder that stopped adding metadata would pass trivially.
+ assert_ne!(input, expected);
+ pretty_assertions::assert_eq!(strip_metadata_from_schema(&input).unwrap(), expected);
+ }
+
+ /// arrow-rs names a list's element `item`, and an arrow-rs `MapBuilder` names
+ /// a map's fields `entries`, `keys` and `values`. Once stripped, a schema
+ /// using those names, with a nullable map key, compares equal to the same
+ /// schema from `schema_to_arrow_schema`.
+ #[test]
+ fn test_strip_metadata_normalizes_container_field_names() {
+ // A list of ints and a map from string to a list of ints, with the given
+ // names for the list elements and the map's entries, key and value.
+ let schema = |[element, entries, key, value]: [&str; 4], key_nullable: bool| {
+ let ints =
+ |nullable| DataType::List(Arc::new(Field::new(element, DataType::Int32, nullable)));
+ let key_value = Fields::from(vec![
+ Field::new(key, DataType::Utf8, key_nullable),
+ Field::new(value, ints(false), true),
+ ]);
+ let entries = Field::new(entries, DataType::Struct(key_value), false);
+ ArrowSchema::new(vec![
+ Field::new("l", ints(true), true),
+ Field::new("m", DataType::Map(Arc::new(entries), false), true),
+ ])
+ };
+ let arrow_rs = schema(["item", "entries", "keys", "values"], true);
+ let expected = schema(
+ [
+ LIST_FIELD_NAME,
+ DEFAULT_MAP_FIELD_NAME,
+ MAP_KEY_FIELD_NAME,
+ MAP_VALUE_FIELD_NAME,
+ ],
+ false,
+ );
+
+ // The same schema as an Iceberg table's.
+ let int_list = |id, required| {
+ let int = Type::Primitive(PrimitiveType::Int);
+ Type::List(ListType::new(
+ NestedField::list_element(id, int, required).into(),
+ ))
+ };
+ let string_to_int_list = MapType::new(
+ NestedField::map_key_element(4, Type::Primitive(PrimitiveType::String)).into(),
+ NestedField::map_value_element(5, int_list(6, true), false).into(),
+ );
+ let iceberg = Schema::builder()
+ .with_fields(vec![
+ NestedField::optional(1, "l", int_list(2, false)).into(),
+ NestedField::optional(3, "m", Type::Map(string_to_int_list)).into(),
+ ])
+ .build()
+ .unwrap();
+
+ pretty_assertions::assert_eq!(strip_metadata_from_schema(&arrow_rs).unwrap(), expected);
+ pretty_assertions::assert_eq!(
+ strip_metadata_from_schema(&schema_to_arrow_schema(&iceberg).unwrap()).unwrap(),
+ expected
+ );
+ }
+
+ /// Names and nullability that `schema_to_arrow_schema` doesn't fix, such as a
+ /// struct field's name or a list element's nullability, still tell schemas apart.
+ #[test]
+ fn test_strip_metadata_keeps_other_differences() {
+ let schema = |child: &str, element_nullable: bool| {
+ let child = Field::new(child, DataType::Int32, true);
+ let element = Field::new("item", DataType::Int32, element_nullable);
+ ArrowSchema::new(vec![
+ Field::new("s", DataType::Struct(Fields::from(vec![child])), true),
+ Field::new("l", DataType::List(Arc::new(element)), true),
+ ])
+ };
+ let strip = |schema: ArrowSchema| strip_metadata_from_schema(&schema).unwrap();
+
+ assert_ne!(strip(schema("a", true)), strip(schema("b", true)));
+ assert_ne!(strip(schema("a", true)), strip(schema("a", false)));
+ }
+
+ /// Stripping drops the `arrow.parquet.variant` extension type with the rest of
+ /// the metadata, so a variant becomes a plain struct, in a list or map too.
+ #[test]
+ fn test_strip_metadata_turns_variants_into_structs() {
+ let variant = || Type::Variant(VariantType);
+ let schema = Schema::builder()
+ .with_fields(vec![
+ NestedField::optional(1, "v", variant()).into(),
+ NestedField::optional(
+ 2,
+ "l",
+ Type::List(ListType::new(
+ NestedField::list_element(3, variant(), false).into(),
+ )),
+ )
+ .into(),
+ NestedField::optional(
+ 4,
+ "m",
+ Type::Map(MapType::new(
+ NestedField::map_key_element(5, Type::Primitive(PrimitiveType::String))
+ .into(),
+ NestedField::map_value_element(6, variant(), false).into(),
+ )),
+ )
+ .into(),
+ ])
+ .build()
+ .unwrap();
+
+ let storage = || {
+ DataType::Struct(Fields::from(vec![
+ Field::new("metadata", DataType::Binary, false),
+ Field::new("value", DataType::Binary, true),
+ ]))
+ };
+ let entries = Fields::from(vec![
+ Field::new(MAP_KEY_FIELD_NAME, DataType::Utf8, false),
+ Field::new(MAP_VALUE_FIELD_NAME, storage(), true),
+ ]);
+ let entries = Field::new(DEFAULT_MAP_FIELD_NAME, DataType::Struct(entries), false);
+ let expected = ArrowSchema::new(vec![
+ Field::new("v", storage(), true),
+ Field::new(
+ "l",
+ DataType::List(Arc::new(Field::new(LIST_FIELD_NAME, storage(), true))),
+ true,
+ ),
+ Field::new("m", DataType::Map(Arc::new(entries), false), true),
+ ]);
+
+ pretty_assertions::assert_eq!(
+ strip_metadata_from_schema(&schema_to_arrow_schema(&schema).unwrap()).unwrap(),
+ expected
+ );
+ }
+
+ /// A dictionary-encoded field compares equal to a plain field of its value
+ /// type once stripped, at any depth.
+ #[test]
+ fn test_strip_metadata_unwraps_dictionaries() {
+ let schema = |dictionary_encoded: bool| {
+ let maybe_dictionary = |key: DataType, value: DataType| {
+ if dictionary_encoded {
+ DataType::Dictionary(Box::new(key), Box::new(value))
+ } else {
+ value
+ }
+ };
+ let element = Field::new(
+ LIST_FIELD_NAME,
+ maybe_dictionary(DataType::Int8, DataType::LargeUtf8),
+ false,
+ );
+ ArrowSchema::new(vec![
+ Field::new("d", maybe_dictionary(DataType::Int32, DataType::Utf8), true),
+ Field::new("l", DataType::List(Arc::new(element)), true),
+ ])
+ };
+
+ pretty_assertions::assert_eq!(
+ strip_metadata_from_schema(&schema(true)).unwrap(),
+ schema(false)
+ );
+ }
}