From 47beffde229ca9de48a0bf00d52dff30bf345ed3 Mon Sep 17 00:00:00 2001 From: JD Williams Date: Tue, 19 May 2026 07:51:30 -0400 Subject: [PATCH 1/2] refactor(spec): make TableProperties own the raw HashMap --- crates/catalog/glue/src/catalog.rs | 6 +- crates/catalog/glue/src/utils.rs | 2 +- crates/catalog/hms/src/catalog.rs | 2 +- crates/catalog/rest/src/catalog.rs | 6 +- crates/catalog/sql/src/catalog.rs | 2 +- crates/iceberg/src/catalog/memory/catalog.rs | 2 +- .../iceberg/src/catalog/metadata_location.rs | 7 +- crates/iceberg/src/catalog/utils.rs | 2 +- crates/iceberg/src/spec/mod.rs | 1 - crates/iceberg/src/spec/table_metadata.rs | 103 ++++++++---------- .../src/spec/table_metadata_builder.rs | 10 +- crates/iceberg/src/spec/table_properties.rs | 41 ++++++- crates/iceberg/src/transaction/mod.rs | 4 +- .../writer/file_writer/location_generator.rs | 32 ++++-- .../datafusion/src/physical_plan/write.rs | 6 +- 15 files changed, 131 insertions(+), 95 deletions(-) diff --git a/crates/catalog/glue/src/catalog.rs b/crates/catalog/glue/src/catalog.rs index c51f6a6a89..d987d350ee 100644 --- a/crates/catalog/glue/src/catalog.rs +++ b/crates/catalog/glue/src/catalog.rs @@ -607,7 +607,7 @@ impl Catalog for GlueCatalog { &table_name, metadata_location_str.clone(), &metadata, - metadata.properties(), + metadata.properties().as_raw(), None, )?; @@ -825,7 +825,7 @@ impl Catalog for GlueCatalog { table_name, metadata_location.clone(), &metadata, - metadata.properties(), + metadata.properties().as_raw(), None, )?; @@ -894,7 +894,7 @@ impl Catalog for GlueCatalog { table_ident.name(), staged_metadata_location.to_string(), staged_table.metadata(), - staged_table.metadata().properties(), + staged_table.metadata().properties().as_raw(), Some(current_metadata_location), )?); diff --git a/crates/catalog/glue/src/utils.rs b/crates/catalog/glue/src/utils.rs index 906e6fcc18..2e5fdede86 100644 --- a/crates/catalog/glue/src/utils.rs +++ b/crates/catalog/glue/src/utils.rs @@ -340,7 +340,7 @@ mod tests { &table_name, metadata_location, &metadata, - metadata.properties(), + metadata.properties().as_raw(), None, )?; diff --git a/crates/catalog/hms/src/catalog.rs b/crates/catalog/hms/src/catalog.rs index d778a3d5fc..a255d112c5 100644 --- a/crates/catalog/hms/src/catalog.rs +++ b/crates/catalog/hms/src/catalog.rs @@ -530,7 +530,7 @@ impl Catalog for HmsCatalog { table_name.clone(), location, metadata_location_str.clone(), - metadata.properties(), + metadata.properties().as_raw(), )?; self.client diff --git a/crates/catalog/rest/src/catalog.rs b/crates/catalog/rest/src/catalog.rs index 0626ce5061..ebe70e037e 100644 --- a/crates/catalog/rest/src/catalog.rs +++ b/crates/catalog/rest/src/catalog.rs @@ -2302,7 +2302,7 @@ mod tests { "gzip".to_string() ) ]), - table.metadata().properties() + table.metadata().properties().as_raw() ); assert_eq!(vec![&Arc::new(Snapshot::builder() .with_snapshot_id(3497810964824022504) @@ -2517,7 +2517,7 @@ mod tests { "zstd".to_string() ), ]), - table.metadata().properties() + table.metadata().properties().as_raw() ); assert!(table.metadata().current_snapshot().is_none()); assert!(table.metadata().history().is_empty()); @@ -2718,7 +2718,7 @@ mod tests { "zstd".to_string() ), ]), - table.metadata().properties() + table.metadata().properties().as_raw() ); assert!(table.metadata().current_snapshot().is_none()); assert!(table.metadata().history().is_empty()); diff --git a/crates/catalog/sql/src/catalog.rs b/crates/catalog/sql/src/catalog.rs index c7bf9d0cfd..a6c774f990 100644 --- a/crates/catalog/sql/src/catalog.rs +++ b/crates/catalog/sql/src/catalog.rs @@ -1165,7 +1165,7 @@ mod tests { vec![&expected_sorted_order] ); - assert_eq!(metadata.properties(), &HashMap::new()); + assert_eq!(metadata.properties().as_raw(), &HashMap::new()); assert!(!table.readonly()); } diff --git a/crates/iceberg/src/catalog/memory/catalog.rs b/crates/iceberg/src/catalog/memory/catalog.rs index 3ae01a23df..4200fb9539 100644 --- a/crates/iceberg/src/catalog/memory/catalog.rs +++ b/crates/iceberg/src/catalog/memory/catalog.rs @@ -541,7 +541,7 @@ pub(crate) mod tests { vec![&expected_sorted_order] ); - assert_eq!(metadata.properties(), &HashMap::new()); + assert_eq!(metadata.properties().as_raw(), &HashMap::new()); assert!(!table.readonly()); } diff --git a/crates/iceberg/src/catalog/metadata_location.rs b/crates/iceberg/src/catalog/metadata_location.rs index acd041d5e1..97e0493011 100644 --- a/crates/iceberg/src/catalog/metadata_location.rs +++ b/crates/iceberg/src/catalog/metadata_location.rs @@ -15,14 +15,13 @@ // specific language governing permissions and limitations // under the License. -use std::collections::HashMap; use std::fmt::Display; use std::str::FromStr; use uuid::Uuid; use crate::compression::CompressionCodec; -use crate::spec::{TableMetadata, parse_metadata_file_compression}; +use crate::spec::{TableMetadata, TableProperties}; use crate::{Error, ErrorKind, Result}; /// Helper for parsing a location of the format: `/metadata/-.metadata.json` @@ -38,8 +37,8 @@ pub struct MetadataLocation { impl MetadataLocation { /// Determines the compression codec from table properties. /// Parse errors result in CompressionCodec::None. - fn compression_from_properties(properties: &HashMap) -> CompressionCodec { - parse_metadata_file_compression(properties).unwrap_or(CompressionCodec::None) + fn compression_from_properties(properties: &TableProperties) -> CompressionCodec { + properties.metadata_compression_codec } /// Creates a completely new metadata location starting at version 0. diff --git a/crates/iceberg/src/catalog/utils.rs b/crates/iceberg/src/catalog/utils.rs index d450f9df80..cf6e741f5e 100644 --- a/crates/iceberg/src/catalog/utils.rs +++ b/crates/iceberg/src/catalog/utils.rs @@ -62,7 +62,7 @@ pub async fn drop_table_data( } // Delete data files only if gc.enabled is true, to avoid corrupting shared tables - if metadata.table_properties()?.gc_enabled { + if metadata.properties().gc_enabled { delete_data_files(io, &manifests_to_delete).await?; } diff --git a/crates/iceberg/src/spec/mod.rs b/crates/iceberg/src/spec/mod.rs index b23ca1eda0..707ebbb630 100644 --- a/crates/iceberg/src/spec/mod.rs +++ b/crates/iceberg/src/spec/mod.rs @@ -50,7 +50,6 @@ pub use sort::*; pub use statistic_file::*; pub use table_metadata::*; pub(crate) use table_metadata_builder::FIRST_FIELD_ID; -pub(crate) use table_properties::parse_metadata_file_compression; pub use table_properties::*; pub use transform::*; pub(crate) use values::decimal_utils; diff --git a/crates/iceberg/src/spec/table_metadata.rs b/crates/iceberg/src/spec/table_metadata.rs index 607fd98350..33d852973e 100644 --- a/crates/iceberg/src/spec/table_metadata.rs +++ b/crates/iceberg/src/spec/table_metadata.rs @@ -35,7 +35,7 @@ pub use super::table_metadata_builder::{TableMetadataBuildResult, TableMetadataB use super::{ DEFAULT_PARTITION_SPEC_ID, PartitionSpecRef, PartitionStatisticsFile, SchemaId, SchemaRef, SnapshotRef, SnapshotRetention, SortOrder, SortOrderRef, StatisticsFile, StructType, - TableProperties, parse_metadata_file_compression, + TableProperties, }; use crate::catalog::MetadataLocation; use crate::compression::CompressionCodec; @@ -91,10 +91,10 @@ pub struct TableMetadata { pub(crate) default_partition_type: StructType, /// An integer; the highest assigned partition field ID across all partition specs for the table. pub(crate) last_partition_id: i32, - ///A string to string map of table properties. This is used to control settings that - /// affect reading and writing and is not intended to be used for arbitrary metadata. - /// For example, commit.retry.num-retries is used to control the number of commit retries. - pub(crate) properties: HashMap, + /// Table properties combining typed, parsed fields (commit retries, file format, + /// compression, etc.) with the raw key-value map for arbitrary/unknown properties + /// and spec-compatible JSON serialization. + pub(crate) properties: TableProperties, /// long ID of the current table snapshot; must be the same as the current /// ID of the main branch in refs. pub(crate) current_snapshot_id: Option, @@ -358,31 +358,14 @@ impl TableMetadata { self.default_sort_order_id } - /// Returns properties of table. + /// Returns table properties, providing both typed access to known properties + /// (e.g., `commit_num_retries`, `metadata_compression_codec`) and raw key-value + /// access for arbitrary/unknown properties via [`TableProperties::get`]. #[inline] - pub fn properties(&self) -> &HashMap { + pub fn properties(&self) -> &TableProperties { &self.properties } - /// Returns the metadata compression codec from table properties. - /// - /// Returns `CompressionCodec::None` if compression is disabled or not configured. - /// Returns `CompressionCodec::Gzip` if gzip compression is enabled. - /// - /// # Errors - /// - /// Returns an error if the compression codec property has an invalid value. - pub fn metadata_compression_codec(&self) -> Result { - parse_metadata_file_compression(&self.properties) - } - - /// Returns typed table properties parsed from the raw properties map with defaults. - pub fn table_properties(&self) -> Result { - TableProperties::try_from(&self.properties).map_err(|e| { - Error::new(ErrorKind::DataInvalid, "Invalid table properties").with_source(e) - }) - } - /// Return location of statistics files. #[inline] pub fn statistics_iter(&self) -> impl ExactSizeIterator { @@ -487,7 +470,7 @@ impl TableMetadata { let json_data = serde_json::to_vec(self)?; // Check if compression codec from properties matches the one in metadata_location - let codec = parse_metadata_file_compression(&self.properties)?; + let codec = self.properties.metadata_compression_codec; if codec != metadata_location.compression_codec() { return Err(Error::new( @@ -776,7 +759,7 @@ pub(super) mod _serde { use crate::spec::{ EncryptedKey, INITIAL_ROW_ID, PartitionField, PartitionSpec, PartitionSpecRef, PartitionStatisticsFile, Schema, SchemaRef, Snapshot, SnapshotReference, SnapshotRetention, - SortOrder, StatisticsFile, + SortOrder, StatisticsFile, TableProperties, }; use crate::{Error, ErrorKind}; @@ -1012,7 +995,11 @@ pub(super) mod _serde { default_partition_type, default_spec, last_partition_id: value.last_partition_id, - properties: value.properties.unwrap_or_default(), + properties: TableProperties::try_from(&value.properties.unwrap_or_default()) + .map_err(|e| { + Error::new(ErrorKind::DataInvalid, "Invalid table properties") + .with_source(e) + })?, current_snapshot_id, snapshots: snapshots .map(|snapshots| { @@ -1125,7 +1112,11 @@ pub(super) mod _serde { default_partition_type, default_spec, last_partition_id: value.last_partition_id, - properties: value.properties.unwrap_or_default(), + properties: TableProperties::try_from(&value.properties.unwrap_or_default()) + .map_err(|e| { + Error::new(ErrorKind::DataInvalid, "Invalid table properties") + .with_source(e) + })?, current_snapshot_id, snapshots: snapshots .map(|snapshots| { @@ -1277,7 +1268,11 @@ pub(super) mod _serde { .unwrap_or_else(|| partition_specs.keys().copied().max().unwrap_or_default()), partition_specs, schemas, - properties: value.properties.unwrap_or_default(), + properties: TableProperties::try_from(&value.properties.unwrap_or_default()) + .map_err(|e| { + Error::new(ErrorKind::DataInvalid, "Invalid table properties") + .with_source(e) + })?, current_snapshot_id, snapshots: value .snapshots @@ -1407,7 +1402,7 @@ pub(super) mod _serde { properties: if v.properties.is_empty() { None } else { - Some(v.properties) + Some(v.properties.as_raw().clone()) }, current_snapshot_id: v.current_snapshot_id, snapshot_log: if v.snapshot_log.is_empty() { @@ -1476,7 +1471,7 @@ pub(super) mod _serde { properties: if v.properties.is_empty() { None } else { - Some(v.properties) + Some(v.properties.as_raw().clone()) }, current_snapshot_id: v.current_snapshot_id, snapshots: if v.snapshots.is_empty() { @@ -1753,10 +1748,11 @@ mod tests { snapshots: HashMap::default(), current_snapshot_id: None, last_sequence_number: 1, - properties: HashMap::from_iter(vec![( + properties: TableProperties::try_from(&HashMap::from_iter(vec![( "commit.retry.num-retries".to_string(), "1".to_string(), - )]), + )])) + .unwrap(), snapshot_log: Vec::new(), metadata_log: vec![MetadataLog { metadata_file: "s3://bucket/.../v1.json".to_string(), @@ -1929,10 +1925,11 @@ mod tests { snapshots: HashMap::from_iter(vec![(1, snapshot.into())]), current_snapshot_id: None, last_sequence_number: 1, - properties: HashMap::from_iter(vec![( + properties: TableProperties::try_from(&HashMap::from_iter(vec![( "commit.retry.num-retries".to_string(), "1".to_string(), - )]), + )])) + .unwrap(), snapshot_log: Vec::new(), metadata_log: vec![MetadataLog { metadata_file: "s3://bucket/.../v1.json".to_string(), @@ -2109,7 +2106,7 @@ mod tests { snapshots: HashMap::from_iter(vec![(638933773299822130, Arc::new(snapshot))]), current_snapshot_id: Some(638933773299822130), last_sequence_number: 0, - properties: HashMap::from_iter(vec![("owner".to_string(), "root".to_string())]), + properties: TableProperties::try_from(&HashMap::from_iter(vec![("owner".to_string(), "root".to_string())])).unwrap(), snapshot_log: vec![SnapshotLog { snapshot_id: 638933773299822130, timestamp_ms: 1662532818843, @@ -2208,7 +2205,7 @@ mod tests { snapshots: HashMap::default(), current_snapshot_id: None, last_sequence_number: 1, - properties: HashMap::new(), + properties: TableProperties::default(), snapshot_log: Vec::new(), metadata_log: vec![MetadataLog { metadata_file: "s3://bucket/.../v1.json".to_string(), @@ -2726,7 +2723,7 @@ mod tests { snapshots: HashMap::from_iter(vec![(3055729675574597004, Arc::new(snapshot))]), current_snapshot_id: Some(3055729675574597004), last_sequence_number: 34, - properties: HashMap::new(), + properties: TableProperties::default(), snapshot_log: Vec::new(), metadata_log: Vec::new(), statistics: HashMap::from_iter(vec![(3055729675574597004, StatisticsFile { @@ -2868,7 +2865,7 @@ mod tests { snapshots: HashMap::from_iter(vec![(3055729675574597004, Arc::new(snapshot))]), current_snapshot_id: Some(3055729675574597004), last_sequence_number: 34, - properties: HashMap::new(), + properties: TableProperties::default(), snapshot_log: Vec::new(), metadata_log: Vec::new(), statistics: HashMap::new(), @@ -2994,7 +2991,7 @@ mod tests { snapshots: HashMap::default(), current_snapshot_id: None, last_sequence_number: 34, - properties: HashMap::new(), + properties: TableProperties::default(), snapshot_log: Vec::new(), metadata_log: Vec::new(), refs: HashMap::new(), @@ -3118,7 +3115,7 @@ mod tests { ]), current_snapshot_id: Some(3055729675574597004), last_sequence_number: 34, - properties: HashMap::new(), + properties: TableProperties::default(), snapshot_log: vec![ SnapshotLog { snapshot_id: 3051729675574597004, @@ -3220,7 +3217,7 @@ mod tests { snapshots: HashMap::default(), current_snapshot_id: None, last_sequence_number: 34, - properties: HashMap::new(), + properties: TableProperties::default(), snapshot_log: vec![], metadata_log: Vec::new(), refs: HashMap::new(), @@ -3290,7 +3287,7 @@ mod tests { snapshots: HashMap::new(), current_snapshot_id: None, last_sequence_number: 0, - properties: HashMap::new(), + properties: TableProperties::default(), snapshot_log: vec![], metadata_log: Vec::new(), refs: HashMap::new(), @@ -3671,7 +3668,7 @@ mod tests { let original_metadata: TableMetadata = get_test_table_metadata("TableMetadataV2Valid.json"); // Modify properties to enable gzip compression (using mixed case to test case-insensitive matching) - let mut props = original_metadata.properties.clone(); + let mut props = original_metadata.properties.as_raw().clone(); props.insert( TableProperties::PROPERTY_METADATA_COMPRESSION_CODEC.to_string(), "GziP".to_string(), @@ -4008,7 +4005,7 @@ mod tests { .unwrap() .metadata; - let props = metadata.table_properties().unwrap(); + let props = metadata.properties(); assert_eq!( props.commit_num_retries, @@ -4055,7 +4052,7 @@ mod tests { .unwrap() .metadata; - let props = metadata.table_properties().unwrap(); + let props = metadata.properties(); assert_eq!(props.commit_num_retries, 10); assert_eq!(props.write_target_file_size_bytes, 1024); @@ -4075,7 +4072,7 @@ mod tests { "not_a_number".to_string(), )]); - let metadata = TableMetadataBuilder::new( + let err = TableMetadataBuilder::new( schema, PartitionSpec::unpartition_spec().into_unbound(), SortOrder::unsorted_order(), @@ -4083,14 +4080,8 @@ mod tests { FormatVersion::V2, properties, ) - .unwrap() - .build() - .unwrap() - .metadata; - - let err = metadata.table_properties().unwrap_err(); + .unwrap_err(); assert_eq!(err.kind(), ErrorKind::DataInvalid); - assert!(err.message().contains("Invalid table properties")); } #[test] diff --git a/crates/iceberg/src/spec/table_metadata_builder.rs b/crates/iceberg/src/spec/table_metadata_builder.rs index 3191d6c13c..6223db5e0f 100644 --- a/crates/iceberg/src/spec/table_metadata_builder.rs +++ b/crates/iceberg/src/spec/table_metadata_builder.rs @@ -110,7 +110,7 @@ impl TableMetadataBuilder { ), // Overwritten immediately by add_default_partition_spec default_partition_type: StructType::new(vec![]), last_partition_id: UNPARTITIONED_LAST_ASSIGNED_ID, - properties: HashMap::new(), + properties: TableProperties::default(), current_snapshot_id: None, snapshots: HashMap::new(), snapshot_log: vec![], @@ -274,7 +274,9 @@ impl TableMetadataBuilder { return Ok(self); } - self.metadata.properties.extend(properties.clone()); + let mut raw = self.metadata.properties.as_raw().clone(); + raw.extend(properties.clone()); + self.metadata.properties = TableProperties::try_from(&raw)?; self.changes.push(TableUpdate::SetProperties { updates: properties, }); @@ -308,9 +310,11 @@ impl TableMetadataBuilder { )); } + let mut raw = self.metadata.properties.as_raw().clone(); for property in &properties { - self.metadata.properties.remove(property); + raw.remove(property); } + self.metadata.properties = TableProperties::try_from(&raw)?; if !properties.is_empty() { self.changes.push(TableUpdate::RemoveProperties { diff --git a/crates/iceberg/src/spec/table_properties.rs b/crates/iceberg/src/spec/table_properties.rs index a3d4e7fdaa..52a858b791 100644 --- a/crates/iceberg/src/spec/table_properties.rs +++ b/crates/iceberg/src/spec/table_properties.rs @@ -100,8 +100,10 @@ pub(crate) fn parse_metadata_file_compression( } /// TableProperties that contains the properties of a table. -#[derive(Debug)] +#[derive(Debug, PartialEq, Eq, Clone)] pub struct TableProperties { + /// The raw properties map. + raw: HashMap, /// The number of times to retry a commit. pub commit_num_retries: usize, /// The minimum wait time between retries. @@ -123,7 +125,43 @@ pub struct TableProperties { pub gc_enabled: bool, } +impl Default for TableProperties { + fn default() -> Self { + Self::try_from(&HashMap::new()).expect("default properties should always be valid") + } +} + impl TableProperties { + /// Get a property value by key. + pub fn get(&self, key: &str) -> Option<&String> { + self.raw.get(key) + } + + /// Check if a property key exists. + pub fn contains_key(&self, key: &str) -> bool { + self.raw.contains_key(key) + } + + /// Iterate over all raw properties. + pub fn iter(&self) -> impl Iterator { + self.raw.iter() + } + + /// Return the number of raw properties. + pub fn len(&self) -> usize { + self.raw.len() + } + + /// Return whether the raw properties map is empty. + pub fn is_empty(&self) -> bool { + self.raw.is_empty() + } + + /// Return a reference to the raw properties map. + pub fn as_raw(&self) -> &HashMap { + &self.raw + } + /// Reserved table property for table format version. /// /// Iceberg will default a new table's format version to the latest stable and recommended @@ -234,6 +272,7 @@ impl TryFrom<&HashMap> for TableProperties { fn try_from(props: &HashMap) -> Result { Ok(TableProperties { + raw: props.clone(), commit_num_retries: parse_property( props, TableProperties::PROPERTY_COMMIT_NUM_RETRIES, diff --git a/crates/iceberg/src/transaction/mod.rs b/crates/iceberg/src/transaction/mod.rs index 04ee1997d0..65d98cd173 100644 --- a/crates/iceberg/src/transaction/mod.rs +++ b/crates/iceberg/src/transaction/mod.rs @@ -171,7 +171,7 @@ impl Transaction { return Ok(self.table); } - let table_props = self.table.metadata().table_properties()?; + let table_props = self.table.metadata().properties(); let backoff = Self::build_backoff(table_props)?; let tx = self; @@ -188,7 +188,7 @@ impl Transaction { .1 } - fn build_backoff(props: TableProperties) -> Result { + fn build_backoff(props: &TableProperties) -> Result { Ok(ExponentialBuilder::new() .with_min_delay(Duration::from_millis(props.commit_min_retry_wait_ms)) .with_max_delay(Duration::from_millis(props.commit_max_retry_wait_ms)) diff --git a/crates/iceberg/src/writer/file_writer/location_generator.rs b/crates/iceberg/src/writer/file_writer/location_generator.rs index 0ad4d91ac6..bec46300d4 100644 --- a/crates/iceberg/src/writer/file_writer/location_generator.rs +++ b/crates/iceberg/src/writer/file_writer/location_generator.rs @@ -150,7 +150,7 @@ pub(crate) mod test { use super::LocationGenerator; use crate::spec::{ FormatVersion, Literal, NestedField, PartitionKey, PartitionSpec, PrimitiveType, Schema, - Struct, StructType, TableMetadata, Transform, Type, + Struct, StructType, TableMetadata, TableProperties, Transform, Type, }; use crate::writer::file_writer::location_generator::{ DefaultLocationGenerator, FileNameGenerator, WRITE_DATA_LOCATION, @@ -176,7 +176,7 @@ pub(crate) mod test { snapshots: HashMap::default(), current_snapshot_id: None, last_sequence_number: 1, - properties: HashMap::new(), + properties: TableProperties::default(), snapshot_log: Vec::new(), metadata_log: vec![], refs: HashMap::new(), @@ -200,10 +200,11 @@ pub(crate) mod test { assert_eq!(location, "s3://data.db/table/data/part-00000-test.parquet"); // test custom data location - table_metadata.properties.insert( + table_metadata.properties = TableProperties::try_from(&HashMap::from([( WRITE_FOLDER_STORAGE_LOCATION.to_string(), "s3://data.db/table/data_1".to_string(), - ); + )])) + .unwrap(); let location_generator = super::DefaultLocationGenerator::new(table_metadata.clone()).unwrap(); let location = @@ -213,10 +214,17 @@ pub(crate) mod test { "s3://data.db/table/data_1/part-00001-test.parquet" ); - table_metadata.properties.insert( - WRITE_DATA_LOCATION.to_string(), - "s3://data.db/table/data_2".to_string(), - ); + table_metadata.properties = TableProperties::try_from(&HashMap::from([ + ( + WRITE_FOLDER_STORAGE_LOCATION.to_string(), + "s3://data.db/table/data_1".to_string(), + ), + ( + WRITE_DATA_LOCATION.to_string(), + "s3://data.db/table/data_2".to_string(), + ), + ])) + .unwrap(); let location_generator = super::DefaultLocationGenerator::new(table_metadata.clone()).unwrap(); let location = @@ -226,11 +234,11 @@ pub(crate) mod test { "s3://data.db/table/data_2/part-00002-test.parquet" ); - table_metadata.properties.insert( + table_metadata.properties = TableProperties::try_from(&HashMap::from([( WRITE_DATA_LOCATION.to_string(), - // invalid table location "s3://data.db/data_3".to_string(), - ); + )])) + .unwrap(); let location_generator = super::DefaultLocationGenerator::new(table_metadata.clone()).unwrap(); let location = @@ -291,7 +299,7 @@ pub(crate) mod test { snapshots: HashMap::default(), current_snapshot_id: None, last_sequence_number: 1, - properties: HashMap::new(), + properties: TableProperties::default(), snapshot_log: Vec::new(), metadata_log: vec![], refs: HashMap::new(), diff --git a/crates/integrations/datafusion/src/physical_plan/write.rs b/crates/integrations/datafusion/src/physical_plan/write.rs index 3b227e20fa..2010f30cbb 100644 --- a/crates/integrations/datafusion/src/physical_plan/write.rs +++ b/crates/integrations/datafusion/src/physical_plan/write.rs @@ -209,11 +209,7 @@ impl ExecutionPlan for IcebergWriteExec { let format_version = self.table.metadata().format_version(); // Get typed table properties - let table_props = self - .table - .metadata() - .table_properties() - .map_err(to_datafusion_error)?; + let table_props = self.table.metadata().properties(); // Check data file format let file_format = DataFileFormat::from_str(&table_props.write_format_default) From 31e7ef85dc0b265283d846887d6c3e8d1405f8ee Mon Sep 17 00:00:00 2001 From: JD Williams Date: Wed, 20 May 2026 06:53:20 -0400 Subject: [PATCH 2/2] ci: retrigger checks