From 1a27322d5b201c63b5b5c455e8f3bd3088c5ceed Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Thu, 17 Jul 2025 22:53:00 -0700 Subject: [PATCH 01/10] added TableMetadataIO --- crates/catalog/glue/src/catalog.rs | 11 +-- crates/iceberg/src/catalog/memory/catalog.rs | 15 +--- crates/iceberg/src/spec/table_metadata.rs | 77 +++++++++++++++++++- 3 files changed, 83 insertions(+), 20 deletions(-) diff --git a/crates/catalog/glue/src/catalog.rs b/crates/catalog/glue/src/catalog.rs index b0e157030f..903febcd06 100644 --- a/crates/catalog/glue/src/catalog.rs +++ b/crates/catalog/glue/src/catalog.rs @@ -23,7 +23,7 @@ use aws_sdk_glue::types::TableInput; use iceberg::io::{ FileIO, S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION, S3_SECRET_ACCESS_KEY, S3_SESSION_TOKEN, }; -use iceberg::spec::{TableMetadata, TableMetadataBuilder}; +use iceberg::spec::{TableMetadata, TableMetadataBuilder, TableMetadataIO}; use iceberg::table::Table; use iceberg::{ Catalog, Error, ErrorKind, Namespace, NamespaceIdent, Result, TableCommit, TableCreation, @@ -395,10 +395,7 @@ impl Catalog for GlueCatalog { .metadata; let metadata_location = create_metadata_location(&location, 0)?; - self.file_io - .new_output(&metadata_location)? - .write(serde_json::to_vec(&metadata)?.into()) - .await?; + TableMetadataIO::write(&self.file_io, &metadata, &metadata_location).await?; let glue_table = convert_to_glue_table( &table_name, @@ -463,9 +460,7 @@ impl Catalog for GlueCatalog { Some(table) => { let metadata_location = get_metadata_location(&table.parameters)?; - let input_file = self.file_io.new_input(&metadata_location)?; - let metadata_content = input_file.read().await?; - let metadata = serde_json::from_slice::(&metadata_content)?; + let metadata = TableMetadataIO::read(&self.file_io, &metadata_location).await?; Table::builder() .file_io(self.file_io()) diff --git a/crates/iceberg/src/catalog/memory/catalog.rs b/crates/iceberg/src/catalog/memory/catalog.rs index c233ab5925..f9141a1c33 100644 --- a/crates/iceberg/src/catalog/memory/catalog.rs +++ b/crates/iceberg/src/catalog/memory/catalog.rs @@ -26,7 +26,7 @@ use uuid::Uuid; use super::namespace_state::NamespaceState; use crate::io::FileIO; -use crate::spec::{TableMetadata, TableMetadataBuilder}; +use crate::spec::{TableMetadata, TableMetadataBuilder, TableMetadataIO}; use crate::table::Table; use crate::{ Catalog, Error, ErrorKind, Namespace, NamespaceIdent, Result, TableCommit, TableCreation, @@ -210,10 +210,7 @@ impl Catalog for MemoryCatalog { Uuid::new_v4() ); - self.file_io - .new_output(&metadata_location)? - .write(serde_json::to_vec(&metadata)?.into()) - .await?; + TableMetadataIO::write(&self.file_io, &metadata, &metadata_location).await?; root_namespace_state.insert_new_table(&table_ident, metadata_location.clone())?; @@ -230,9 +227,7 @@ impl Catalog for MemoryCatalog { let root_namespace_state = self.root_namespace_state.lock().await; let metadata_location = root_namespace_state.get_existing_table_location(table_ident)?; - let input_file = self.file_io.new_input(metadata_location)?; - let metadata_content = input_file.read().await?; - let metadata = serde_json::from_slice::(&metadata_content)?; + let metadata = TableMetadataIO::read(&self.file_io, metadata_location).await?; Table::builder() .file_io(self.file_io.clone()) @@ -284,9 +279,7 @@ impl Catalog for MemoryCatalog { let mut root_namespace_state = self.root_namespace_state.lock().await; root_namespace_state.insert_new_table(&table_ident.clone(), metadata_location.clone())?; - let input_file = self.file_io.new_input(metadata_location.clone())?; - let metadata_content = input_file.read().await?; - let metadata = serde_json::from_slice::(&metadata_content)?; + let metadata = TableMetadataIO::read(&self.file_io, &metadata_location).await?; Table::builder() .file_io(self.file_io.clone()) diff --git a/crates/iceberg/src/spec/table_metadata.rs b/crates/iceberg/src/spec/table_metadata.rs index 2604eac03d..0c3e64512a 100644 --- a/crates/iceberg/src/spec/table_metadata.rs +++ b/crates/iceberg/src/spec/table_metadata.rs @@ -37,6 +37,7 @@ use super::{ SnapshotRef, SnapshotRetention, SortOrder, SortOrderRef, StatisticsFile, StructType, }; use crate::error::{Result, timestamp_ms_to_utc}; +use crate::io::FileIO; use crate::{Error, ErrorKind}; static MAIN_BRANCH: &str = "main"; @@ -686,6 +687,27 @@ impl TableMetadata { } } +/// Utility for reading and writing table metadata. +pub struct TableMetadataIO; + +impl TableMetadataIO { + /// Read table metadata from the given location. + pub async fn read(file_io: &FileIO, metadata_location: &str) -> Result { + let input_file = file_io.new_input(metadata_location)?; + let metadata_content = input_file.read().await?; + let metadata = serde_json::from_slice::(&metadata_content)?; + Ok(metadata) + } + + /// Write table metadata to the given location. + pub async fn write(file_io: &FileIO, metadata: &TableMetadata, metadata_location: &str) -> Result<()> { + file_io + .new_output(metadata_location)? + .write(serde_json::to_vec(metadata)?.into()) + .await + } +} + pub(super) mod _serde { use std::borrow::BorrowMut; /// This is a helper module that defines types to help with serialization/deserialization. @@ -1355,9 +1377,11 @@ mod tests { use anyhow::Result; use pretty_assertions::assert_eq; + use tempfile::TempDir; use uuid::Uuid; - use super::{FormatVersion, MetadataLog, SnapshotLog, TableMetadataBuilder}; + use super::{FormatVersion, MetadataLog, SnapshotLog, TableMetadataBuilder, TableMetadataIO}; + use crate::io::FileIOBuilder; use crate::TableCreation; use crate::spec::table_metadata::TableMetadata; use crate::spec::{ @@ -3050,4 +3074,55 @@ mod tests { )]) ); } + + #[tokio::test] + async fn test_table_metadata_io_read_write() { + // Create a temporary directory for our test + let temp_dir = TempDir::new().unwrap(); + let temp_path = temp_dir.path().to_str().unwrap(); + + // Create a FileIO instance + let file_io = FileIOBuilder::new_fs_io().build().unwrap(); + + // Use an existing test metadata from the test files + let metadata_path = "testdata/table_metadata/TableMetadataV2Valid.json"; + let metadata_content = fs::read_to_string(metadata_path).unwrap(); + let original_metadata: TableMetadata = serde_json::from_str(&metadata_content).unwrap(); + + // Define the metadata location + let metadata_location = format!("{}/metadata.json", temp_path); + + // Write the metadata + TableMetadataIO::write(&file_io, &original_metadata, &metadata_location).await.unwrap(); + + // Verify the file exists + assert!(fs::metadata(&metadata_location).is_ok()); + + // Read the metadata back + let read_metadata = TableMetadataIO::read(&file_io, &metadata_location).await.unwrap(); + + // Verify the metadata matches + assert_eq!(read_metadata.format_version, original_metadata.format_version); + assert_eq!(read_metadata.table_uuid, original_metadata.table_uuid); + assert_eq!(read_metadata.location, original_metadata.location); + assert_eq!(read_metadata.last_sequence_number, original_metadata.last_sequence_number); + assert_eq!(read_metadata.last_updated_ms, original_metadata.last_updated_ms); + assert_eq!(read_metadata.last_column_id, original_metadata.last_column_id); + assert_eq!(read_metadata.current_schema_id, original_metadata.current_schema_id); + assert_eq!(read_metadata.last_partition_id, original_metadata.last_partition_id); + assert_eq!(read_metadata.default_sort_order_id, original_metadata.default_sort_order_id); + assert_eq!(read_metadata.properties, original_metadata.properties); + } + + #[tokio::test] + async fn test_table_metadata_io_read_nonexistent_file() { + // Create a FileIO instance + let file_io = FileIOBuilder::new_fs_io().build().unwrap(); + + // Try to read a non-existent file + let result = TableMetadataIO::read(&file_io, "/nonexistent/path/metadata.json").await; + + // Verify it returns an error + assert!(result.is_err()); + } } From 559a2fdb7d7277a0d2bb5e9a54ee6db06d46dd28 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Thu, 17 Jul 2025 23:08:11 -0700 Subject: [PATCH 02/10] cleanup --- crates/catalog/glue/src/catalog.rs | 2 +- crates/catalog/hms/src/catalog.rs | 10 +-- crates/catalog/s3tables/src/catalog.rs | 11 +-- crates/catalog/sql/src/catalog.rs | 10 +-- crates/iceberg/src/catalog/memory/catalog.rs | 2 +- crates/iceberg/src/spec/table_metadata.rs | 71 ++++++++++++++------ 6 files changed, 61 insertions(+), 45 deletions(-) diff --git a/crates/catalog/glue/src/catalog.rs b/crates/catalog/glue/src/catalog.rs index 903febcd06..554e045186 100644 --- a/crates/catalog/glue/src/catalog.rs +++ b/crates/catalog/glue/src/catalog.rs @@ -23,7 +23,7 @@ use aws_sdk_glue::types::TableInput; use iceberg::io::{ FileIO, S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION, S3_SECRET_ACCESS_KEY, S3_SESSION_TOKEN, }; -use iceberg::spec::{TableMetadata, TableMetadataBuilder, TableMetadataIO}; +use iceberg::spec::{TableMetadataBuilder, TableMetadataIO}; use iceberg::table::Table; use iceberg::{ Catalog, Error, ErrorKind, Namespace, NamespaceIdent, Result, TableCommit, TableCreation, diff --git a/crates/catalog/hms/src/catalog.rs b/crates/catalog/hms/src/catalog.rs index d126f2ffa5..b10d7c595d 100644 --- a/crates/catalog/hms/src/catalog.rs +++ b/crates/catalog/hms/src/catalog.rs @@ -26,7 +26,7 @@ use hive_metastore::{ ThriftHiveMetastoreGetDatabaseException, ThriftHiveMetastoreGetTableException, }; use iceberg::io::FileIO; -use iceberg::spec::{TableMetadata, TableMetadataBuilder}; +use iceberg::spec::{TableMetadataBuilder, TableMetadataIO}; use iceberg::table::Table; use iceberg::{ Catalog, Error, ErrorKind, Namespace, NamespaceIdent, Result, TableCommit, TableCreation, @@ -353,10 +353,7 @@ impl Catalog for HmsCatalog { let metadata_location = create_metadata_location(&location, 0)?; - self.file_io - .new_output(&metadata_location)? - .write(serde_json::to_vec(&metadata)?.into()) - .await?; + TableMetadataIO::write(&self.file_io, &metadata, &metadata_location).await?; let hive_table = convert_to_hive_table( db_name.clone(), @@ -406,8 +403,7 @@ impl Catalog for HmsCatalog { let metadata_location = get_metadata_location(&hive_table.parameters)?; - let metadata_content = self.file_io.new_input(&metadata_location)?.read().await?; - let metadata = serde_json::from_slice::(&metadata_content)?; + let metadata = TableMetadataIO::read(&self.file_io, &metadata_location).await?; Table::builder() .file_io(self.file_io()) diff --git a/crates/catalog/s3tables/src/catalog.rs b/crates/catalog/s3tables/src/catalog.rs index 486b184b10..0a73e98cce 100644 --- a/crates/catalog/s3tables/src/catalog.rs +++ b/crates/catalog/s3tables/src/catalog.rs @@ -25,7 +25,7 @@ use aws_sdk_s3tables::operation::get_table::GetTableOutput; use aws_sdk_s3tables::operation::list_tables::ListTablesOutput; use aws_sdk_s3tables::types::OpenTableFormat; use iceberg::io::{FileIO, FileIOBuilder}; -use iceberg::spec::{TableMetadata, TableMetadataBuilder}; +use iceberg::spec::{TableMetadataBuilder, TableMetadataIO}; use iceberg::table::Table; use iceberg::{ Catalog, Error, ErrorKind, Namespace, NamespaceIdent, Result, TableCommit, TableCreation, @@ -334,10 +334,7 @@ impl Catalog for S3TablesCatalog { let metadata = TableMetadataBuilder::from_table_creation(creation)? .build()? .metadata; - self.file_io - .new_output(&metadata_location)? - .write(serde_json::to_vec(&metadata)?.into()) - .await?; + TableMetadataIO::write(&self.file_io, &metadata, &metadata_location).await?; // update metadata location self.s3tables_client @@ -389,9 +386,7 @@ impl Catalog for S3TablesCatalog { ), ) })?; - let input_file = self.file_io.new_input(metadata_location)?; - let metadata_content = input_file.read().await?; - let metadata = serde_json::from_slice::(&metadata_content)?; + let metadata = TableMetadataIO::read(&self.file_io, metadata_location).await?; let table = Table::builder() .identifier(table_ident.clone()) diff --git a/crates/catalog/sql/src/catalog.rs b/crates/catalog/sql/src/catalog.rs index 24c22dbcf8..0183786c01 100644 --- a/crates/catalog/sql/src/catalog.rs +++ b/crates/catalog/sql/src/catalog.rs @@ -20,7 +20,7 @@ use std::time::Duration; use async_trait::async_trait; use iceberg::io::FileIO; -use iceberg::spec::{TableMetadata, TableMetadataBuilder}; +use iceberg::spec::{TableMetadataBuilder, TableMetadataIO}; use iceberg::table::Table; use iceberg::{ Catalog, Error, ErrorKind, Namespace, NamespaceIdent, Result, TableCommit, TableCreation, @@ -642,9 +642,7 @@ impl Catalog for SqlCatalog { .try_get::(CATALOG_FIELD_METADATA_LOCATION_PROP) .map_err(from_sqlx_error)?; - let file = self.fileio.new_input(&tbl_metadata_location)?; - let metadata_content = file.read().await?; - let metadata = serde_json::from_slice::(&metadata_content)?; + let metadata = TableMetadataIO::read(&self.fileio, &tbl_metadata_location).await?; Ok(Table::builder() .file_io(self.fileio.clone()) @@ -708,9 +706,7 @@ impl Catalog for SqlCatalog { Uuid::new_v4() ); - let file = self.fileio.new_output(&tbl_metadata_location)?; - file.write(serde_json::to_vec(&tbl_metadata)?.into()) - .await?; + TableMetadataIO::write(&self.fileio, &tbl_metadata, &tbl_metadata_location).await?; self.execute(&format!( "INSERT INTO {CATALOG_TABLE_NAME} diff --git a/crates/iceberg/src/catalog/memory/catalog.rs b/crates/iceberg/src/catalog/memory/catalog.rs index f9141a1c33..d0685d6735 100644 --- a/crates/iceberg/src/catalog/memory/catalog.rs +++ b/crates/iceberg/src/catalog/memory/catalog.rs @@ -26,7 +26,7 @@ use uuid::Uuid; use super::namespace_state::NamespaceState; use crate::io::FileIO; -use crate::spec::{TableMetadata, TableMetadataBuilder, TableMetadataIO}; +use crate::spec::{TableMetadataBuilder, TableMetadataIO}; use crate::table::Table; use crate::{ Catalog, Error, ErrorKind, Namespace, NamespaceIdent, Result, TableCommit, TableCreation, diff --git a/crates/iceberg/src/spec/table_metadata.rs b/crates/iceberg/src/spec/table_metadata.rs index 0c3e64512a..076232282e 100644 --- a/crates/iceberg/src/spec/table_metadata.rs +++ b/crates/iceberg/src/spec/table_metadata.rs @@ -700,7 +700,11 @@ impl TableMetadataIO { } /// Write table metadata to the given location. - pub async fn write(file_io: &FileIO, metadata: &TableMetadata, metadata_location: &str) -> Result<()> { + pub async fn write( + file_io: &FileIO, + metadata: &TableMetadata, + metadata_location: &str, + ) -> Result<()> { file_io .new_output(metadata_location)? .write(serde_json::to_vec(metadata)?.into()) @@ -1381,8 +1385,8 @@ mod tests { use uuid::Uuid; use super::{FormatVersion, MetadataLog, SnapshotLog, TableMetadataBuilder, TableMetadataIO}; - use crate::io::FileIOBuilder; use crate::TableCreation; + use crate::io::FileIOBuilder; use crate::spec::table_metadata::TableMetadata; use crate::spec::{ BlobMetadata, NestedField, NullOrder, Operation, PartitionSpec, PartitionStatisticsFile, @@ -3080,48 +3084,73 @@ mod tests { // Create a temporary directory for our test let temp_dir = TempDir::new().unwrap(); let temp_path = temp_dir.path().to_str().unwrap(); - + // Create a FileIO instance let file_io = FileIOBuilder::new_fs_io().build().unwrap(); - + // Use an existing test metadata from the test files let metadata_path = "testdata/table_metadata/TableMetadataV2Valid.json"; let metadata_content = fs::read_to_string(metadata_path).unwrap(); let original_metadata: TableMetadata = serde_json::from_str(&metadata_content).unwrap(); - + // Define the metadata location let metadata_location = format!("{}/metadata.json", temp_path); - + // Write the metadata - TableMetadataIO::write(&file_io, &original_metadata, &metadata_location).await.unwrap(); - + TableMetadataIO::write(&file_io, &original_metadata, &metadata_location) + .await + .unwrap(); + // Verify the file exists assert!(fs::metadata(&metadata_location).is_ok()); - + // Read the metadata back - let read_metadata = TableMetadataIO::read(&file_io, &metadata_location).await.unwrap(); - + let read_metadata = TableMetadataIO::read(&file_io, &metadata_location) + .await + .unwrap(); + // Verify the metadata matches - assert_eq!(read_metadata.format_version, original_metadata.format_version); + assert_eq!( + read_metadata.format_version, + original_metadata.format_version + ); assert_eq!(read_metadata.table_uuid, original_metadata.table_uuid); assert_eq!(read_metadata.location, original_metadata.location); - assert_eq!(read_metadata.last_sequence_number, original_metadata.last_sequence_number); - assert_eq!(read_metadata.last_updated_ms, original_metadata.last_updated_ms); - assert_eq!(read_metadata.last_column_id, original_metadata.last_column_id); - assert_eq!(read_metadata.current_schema_id, original_metadata.current_schema_id); - assert_eq!(read_metadata.last_partition_id, original_metadata.last_partition_id); - assert_eq!(read_metadata.default_sort_order_id, original_metadata.default_sort_order_id); + assert_eq!( + read_metadata.last_sequence_number, + original_metadata.last_sequence_number + ); + assert_eq!( + read_metadata.last_updated_ms, + original_metadata.last_updated_ms + ); + assert_eq!( + read_metadata.last_column_id, + original_metadata.last_column_id + ); + assert_eq!( + read_metadata.current_schema_id, + original_metadata.current_schema_id + ); + assert_eq!( + read_metadata.last_partition_id, + original_metadata.last_partition_id + ); + assert_eq!( + read_metadata.default_sort_order_id, + original_metadata.default_sort_order_id + ); assert_eq!(read_metadata.properties, original_metadata.properties); } - + #[tokio::test] async fn test_table_metadata_io_read_nonexistent_file() { // Create a FileIO instance let file_io = FileIOBuilder::new_fs_io().build().unwrap(); - + // Try to read a non-existent file let result = TableMetadataIO::read(&file_io, "/nonexistent/path/metadata.json").await; - + // Verify it returns an error assert!(result.is_err()); } From 2dfce139091c67339ebcbace15f41eae184ce418 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Thu, 17 Jul 2025 23:13:55 -0700 Subject: [PATCH 03/10] simplify tests --- crates/iceberg/src/spec/table_metadata.rs | 32 +---------------------- 1 file changed, 1 insertion(+), 31 deletions(-) diff --git a/crates/iceberg/src/spec/table_metadata.rs b/crates/iceberg/src/spec/table_metadata.rs index 076232282e..c62f255310 100644 --- a/crates/iceberg/src/spec/table_metadata.rs +++ b/crates/iceberg/src/spec/table_metadata.rs @@ -3110,37 +3110,7 @@ mod tests { .unwrap(); // Verify the metadata matches - assert_eq!( - read_metadata.format_version, - original_metadata.format_version - ); - assert_eq!(read_metadata.table_uuid, original_metadata.table_uuid); - assert_eq!(read_metadata.location, original_metadata.location); - assert_eq!( - read_metadata.last_sequence_number, - original_metadata.last_sequence_number - ); - assert_eq!( - read_metadata.last_updated_ms, - original_metadata.last_updated_ms - ); - assert_eq!( - read_metadata.last_column_id, - original_metadata.last_column_id - ); - assert_eq!( - read_metadata.current_schema_id, - original_metadata.current_schema_id - ); - assert_eq!( - read_metadata.last_partition_id, - original_metadata.last_partition_id - ); - assert_eq!( - read_metadata.default_sort_order_id, - original_metadata.default_sort_order_id - ); - assert_eq!(read_metadata.properties, original_metadata.properties); + assert_eq!(read_metadata, original_metadata); } #[tokio::test] From bd039a38f40ae94b728f68acd708d64fc673bf8e Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Thu, 17 Jul 2025 23:27:01 -0700 Subject: [PATCH 04/10] simpler --- Cargo.lock | 2 -- crates/catalog/s3tables/Cargo.toml | 1 - crates/catalog/sql/Cargo.toml | 1 - crates/iceberg/src/spec/table_metadata.rs | 4 +--- 4 files changed, 1 insertion(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 831b3aa4f4..a3e7052a1b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3642,7 +3642,6 @@ dependencies = [ "iceberg", "iceberg_test_utils", "itertools 0.13.0", - "serde_json", "tokio", "typed-builder 0.20.1", "uuid", @@ -3657,7 +3656,6 @@ dependencies = [ "iceberg_test_utils", "itertools 0.13.0", "regex", - "serde_json", "sqlx", "tempfile", "tokio", diff --git a/crates/catalog/s3tables/Cargo.toml b/crates/catalog/s3tables/Cargo.toml index 8b6b07493c..0ec1f55f62 100644 --- a/crates/catalog/s3tables/Cargo.toml +++ b/crates/catalog/s3tables/Cargo.toml @@ -34,7 +34,6 @@ async-trait = { workspace = true } aws-config = { workspace = true } aws-sdk-s3tables = "1.10.0" iceberg = { workspace = true } -serde_json = { workspace = true } typed-builder = { workspace = true } uuid = { workspace = true, features = ["v4"] } diff --git a/crates/catalog/sql/Cargo.toml b/crates/catalog/sql/Cargo.toml index 3c87924647..b767b68775 100644 --- a/crates/catalog/sql/Cargo.toml +++ b/crates/catalog/sql/Cargo.toml @@ -31,7 +31,6 @@ repository = { workspace = true } [dependencies] async-trait = { workspace = true } iceberg = { workspace = true } -serde_json = { workspace = true } sqlx = { version = "0.8.1", features = ["any"], default-features = false } typed-builder = { workspace = true } uuid = { workspace = true, features = ["v4"] } diff --git a/crates/iceberg/src/spec/table_metadata.rs b/crates/iceberg/src/spec/table_metadata.rs index c62f255310..30bd5fbcbf 100644 --- a/crates/iceberg/src/spec/table_metadata.rs +++ b/crates/iceberg/src/spec/table_metadata.rs @@ -3089,9 +3089,7 @@ mod tests { let file_io = FileIOBuilder::new_fs_io().build().unwrap(); // Use an existing test metadata from the test files - let metadata_path = "testdata/table_metadata/TableMetadataV2Valid.json"; - let metadata_content = fs::read_to_string(metadata_path).unwrap(); - let original_metadata: TableMetadata = serde_json::from_str(&metadata_content).unwrap(); + let original_metadata: TableMetadata = get_test_table_metadata("TableMetadataV2Valid.json"); // Define the metadata location let metadata_location = format!("{}/metadata.json", temp_path); From 838ecaa7937ddeaaf7925e673b08fb3c7a7817a9 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Fri, 18 Jul 2025 00:52:19 -0700 Subject: [PATCH 05/10] just read and write --- crates/catalog/glue/src/catalog.rs | 6 +-- crates/catalog/hms/src/catalog.rs | 6 +-- crates/catalog/s3tables/src/catalog.rs | 6 +-- crates/catalog/sql/src/catalog.rs | 6 +-- crates/iceberg/src/catalog/memory/catalog.rs | 8 +-- crates/iceberg/src/spec/table_metadata.rs | 53 +++++++++----------- 6 files changed, 40 insertions(+), 45 deletions(-) diff --git a/crates/catalog/glue/src/catalog.rs b/crates/catalog/glue/src/catalog.rs index 554e045186..c946d76ed0 100644 --- a/crates/catalog/glue/src/catalog.rs +++ b/crates/catalog/glue/src/catalog.rs @@ -23,7 +23,7 @@ use aws_sdk_glue::types::TableInput; use iceberg::io::{ FileIO, S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION, S3_SECRET_ACCESS_KEY, S3_SESSION_TOKEN, }; -use iceberg::spec::{TableMetadataBuilder, TableMetadataIO}; +use iceberg::spec::{TableMetadata, TableMetadataBuilder}; use iceberg::table::Table; use iceberg::{ Catalog, Error, ErrorKind, Namespace, NamespaceIdent, Result, TableCommit, TableCreation, @@ -395,7 +395,7 @@ impl Catalog for GlueCatalog { .metadata; let metadata_location = create_metadata_location(&location, 0)?; - TableMetadataIO::write(&self.file_io, &metadata, &metadata_location).await?; + TableMetadata::write(&self.file_io, &metadata, &metadata_location).await?; let glue_table = convert_to_glue_table( &table_name, @@ -460,7 +460,7 @@ impl Catalog for GlueCatalog { Some(table) => { let metadata_location = get_metadata_location(&table.parameters)?; - let metadata = TableMetadataIO::read(&self.file_io, &metadata_location).await?; + let metadata = TableMetadata::read(&self.file_io, &metadata_location).await?; Table::builder() .file_io(self.file_io()) diff --git a/crates/catalog/hms/src/catalog.rs b/crates/catalog/hms/src/catalog.rs index b10d7c595d..2bf10f20c7 100644 --- a/crates/catalog/hms/src/catalog.rs +++ b/crates/catalog/hms/src/catalog.rs @@ -26,7 +26,7 @@ use hive_metastore::{ ThriftHiveMetastoreGetDatabaseException, ThriftHiveMetastoreGetTableException, }; use iceberg::io::FileIO; -use iceberg::spec::{TableMetadataBuilder, TableMetadataIO}; +use iceberg::spec::{TableMetadata, TableMetadataBuilder}; use iceberg::table::Table; use iceberg::{ Catalog, Error, ErrorKind, Namespace, NamespaceIdent, Result, TableCommit, TableCreation, @@ -353,7 +353,7 @@ impl Catalog for HmsCatalog { let metadata_location = create_metadata_location(&location, 0)?; - TableMetadataIO::write(&self.file_io, &metadata, &metadata_location).await?; + TableMetadata::write(&self.file_io, &metadata, &metadata_location).await?; let hive_table = convert_to_hive_table( db_name.clone(), @@ -403,7 +403,7 @@ impl Catalog for HmsCatalog { let metadata_location = get_metadata_location(&hive_table.parameters)?; - let metadata = TableMetadataIO::read(&self.file_io, &metadata_location).await?; + let metadata = TableMetadata::read(&self.file_io, &metadata_location).await?; Table::builder() .file_io(self.file_io()) diff --git a/crates/catalog/s3tables/src/catalog.rs b/crates/catalog/s3tables/src/catalog.rs index 0a73e98cce..2b773925f9 100644 --- a/crates/catalog/s3tables/src/catalog.rs +++ b/crates/catalog/s3tables/src/catalog.rs @@ -25,7 +25,7 @@ use aws_sdk_s3tables::operation::get_table::GetTableOutput; use aws_sdk_s3tables::operation::list_tables::ListTablesOutput; use aws_sdk_s3tables::types::OpenTableFormat; use iceberg::io::{FileIO, FileIOBuilder}; -use iceberg::spec::{TableMetadataBuilder, TableMetadataIO}; +use iceberg::spec::{TableMetadata, TableMetadataBuilder}; use iceberg::table::Table; use iceberg::{ Catalog, Error, ErrorKind, Namespace, NamespaceIdent, Result, TableCommit, TableCreation, @@ -334,7 +334,7 @@ impl Catalog for S3TablesCatalog { let metadata = TableMetadataBuilder::from_table_creation(creation)? .build()? .metadata; - TableMetadataIO::write(&self.file_io, &metadata, &metadata_location).await?; + TableMetadata::write(&self.file_io, &metadata, &metadata_location).await?; // update metadata location self.s3tables_client @@ -386,7 +386,7 @@ impl Catalog for S3TablesCatalog { ), ) })?; - let metadata = TableMetadataIO::read(&self.file_io, metadata_location).await?; + let metadata = TableMetadata::read(&self.file_io, metadata_location).await?; let table = Table::builder() .identifier(table_ident.clone()) diff --git a/crates/catalog/sql/src/catalog.rs b/crates/catalog/sql/src/catalog.rs index 0183786c01..c7f2940737 100644 --- a/crates/catalog/sql/src/catalog.rs +++ b/crates/catalog/sql/src/catalog.rs @@ -20,7 +20,7 @@ use std::time::Duration; use async_trait::async_trait; use iceberg::io::FileIO; -use iceberg::spec::{TableMetadataBuilder, TableMetadataIO}; +use iceberg::spec::{TableMetadata, TableMetadataBuilder}; use iceberg::table::Table; use iceberg::{ Catalog, Error, ErrorKind, Namespace, NamespaceIdent, Result, TableCommit, TableCreation, @@ -642,7 +642,7 @@ impl Catalog for SqlCatalog { .try_get::(CATALOG_FIELD_METADATA_LOCATION_PROP) .map_err(from_sqlx_error)?; - let metadata = TableMetadataIO::read(&self.fileio, &tbl_metadata_location).await?; + let metadata = TableMetadata::read(&self.fileio, &tbl_metadata_location).await?; Ok(Table::builder() .file_io(self.fileio.clone()) @@ -706,7 +706,7 @@ impl Catalog for SqlCatalog { Uuid::new_v4() ); - TableMetadataIO::write(&self.fileio, &tbl_metadata, &tbl_metadata_location).await?; + TableMetadata::write(&self.fileio, &tbl_metadata, &tbl_metadata_location).await?; self.execute(&format!( "INSERT INTO {CATALOG_TABLE_NAME} diff --git a/crates/iceberg/src/catalog/memory/catalog.rs b/crates/iceberg/src/catalog/memory/catalog.rs index d0685d6735..4af1bc38a0 100644 --- a/crates/iceberg/src/catalog/memory/catalog.rs +++ b/crates/iceberg/src/catalog/memory/catalog.rs @@ -26,7 +26,7 @@ use uuid::Uuid; use super::namespace_state::NamespaceState; use crate::io::FileIO; -use crate::spec::{TableMetadataBuilder, TableMetadataIO}; +use crate::spec::{TableMetadata, TableMetadataBuilder}; use crate::table::Table; use crate::{ Catalog, Error, ErrorKind, Namespace, NamespaceIdent, Result, TableCommit, TableCreation, @@ -210,7 +210,7 @@ impl Catalog for MemoryCatalog { Uuid::new_v4() ); - TableMetadataIO::write(&self.file_io, &metadata, &metadata_location).await?; + TableMetadata::write(&self.file_io, &metadata, &metadata_location).await?; root_namespace_state.insert_new_table(&table_ident, metadata_location.clone())?; @@ -227,7 +227,7 @@ impl Catalog for MemoryCatalog { let root_namespace_state = self.root_namespace_state.lock().await; let metadata_location = root_namespace_state.get_existing_table_location(table_ident)?; - let metadata = TableMetadataIO::read(&self.file_io, metadata_location).await?; + let metadata = TableMetadata::read(&self.file_io, metadata_location).await?; Table::builder() .file_io(self.file_io.clone()) @@ -279,7 +279,7 @@ impl Catalog for MemoryCatalog { let mut root_namespace_state = self.root_namespace_state.lock().await; root_namespace_state.insert_new_table(&table_ident.clone(), metadata_location.clone())?; - let metadata = TableMetadataIO::read(&self.file_io, &metadata_location).await?; + let metadata = TableMetadata::read(&self.file_io, &metadata_location).await?; Table::builder() .file_io(self.file_io.clone()) diff --git a/crates/iceberg/src/spec/table_metadata.rs b/crates/iceberg/src/spec/table_metadata.rs index 30bd5fbcbf..ab5354142b 100644 --- a/crates/iceberg/src/spec/table_metadata.rs +++ b/crates/iceberg/src/spec/table_metadata.rs @@ -465,6 +465,26 @@ impl TableMetadata { self.encryption_keys.get(key_id) } + /// Read table metadata from the given location. + pub async fn read(file_io: &FileIO, metadata_location: &str) -> Result { + let input_file = file_io.new_input(metadata_location)?; + let metadata_content = input_file.read().await?; + let metadata = serde_json::from_slice::(&metadata_content)?; + Ok(metadata) + } + + /// Write table metadata to the given location. + pub async fn write( + file_io: &FileIO, + metadata: &TableMetadata, + metadata_location: &str, + ) -> Result<()> { + file_io + .new_output(metadata_location)? + .write(serde_json::to_vec(metadata)?.into()) + .await + } + /// Normalize this partition spec. /// /// This is an internal method @@ -687,31 +707,6 @@ impl TableMetadata { } } -/// Utility for reading and writing table metadata. -pub struct TableMetadataIO; - -impl TableMetadataIO { - /// Read table metadata from the given location. - pub async fn read(file_io: &FileIO, metadata_location: &str) -> Result { - let input_file = file_io.new_input(metadata_location)?; - let metadata_content = input_file.read().await?; - let metadata = serde_json::from_slice::(&metadata_content)?; - Ok(metadata) - } - - /// Write table metadata to the given location. - pub async fn write( - file_io: &FileIO, - metadata: &TableMetadata, - metadata_location: &str, - ) -> Result<()> { - file_io - .new_output(metadata_location)? - .write(serde_json::to_vec(metadata)?.into()) - .await - } -} - pub(super) mod _serde { use std::borrow::BorrowMut; /// This is a helper module that defines types to help with serialization/deserialization. @@ -1384,7 +1379,7 @@ mod tests { use tempfile::TempDir; use uuid::Uuid; - use super::{FormatVersion, MetadataLog, SnapshotLog, TableMetadataBuilder, TableMetadataIO}; + use super::{FormatVersion, MetadataLog, SnapshotLog, TableMetadata, TableMetadataBuilder}; use crate::TableCreation; use crate::io::FileIOBuilder; use crate::spec::table_metadata::TableMetadata; @@ -3095,7 +3090,7 @@ mod tests { let metadata_location = format!("{}/metadata.json", temp_path); // Write the metadata - TableMetadataIO::write(&file_io, &original_metadata, &metadata_location) + TableMetadata::write(&file_io, &original_metadata, &metadata_location) .await .unwrap(); @@ -3103,7 +3098,7 @@ mod tests { assert!(fs::metadata(&metadata_location).is_ok()); // Read the metadata back - let read_metadata = TableMetadataIO::read(&file_io, &metadata_location) + let read_metadata = TableMetadata::read(&file_io, &metadata_location) .await .unwrap(); @@ -3117,7 +3112,7 @@ mod tests { let file_io = FileIOBuilder::new_fs_io().build().unwrap(); // Try to read a non-existent file - let result = TableMetadataIO::read(&file_io, "/nonexistent/path/metadata.json").await; + let result = TableMetadata::read(&file_io, "/nonexistent/path/metadata.json").await; // Verify it returns an error assert!(result.is_err()); From 15604987c9ea937b2195de5f9fb564799c57123d Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Fri, 18 Jul 2025 01:00:49 -0700 Subject: [PATCH 06/10] minor --- crates/iceberg/src/spec/table_metadata.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/iceberg/src/spec/table_metadata.rs b/crates/iceberg/src/spec/table_metadata.rs index ab5354142b..0157eea061 100644 --- a/crates/iceberg/src/spec/table_metadata.rs +++ b/crates/iceberg/src/spec/table_metadata.rs @@ -1379,7 +1379,7 @@ mod tests { use tempfile::TempDir; use uuid::Uuid; - use super::{FormatVersion, MetadataLog, SnapshotLog, TableMetadata, TableMetadataBuilder}; + use super::{FormatVersion, MetadataLog, SnapshotLog, TableMetadataBuilder}; use crate::TableCreation; use crate::io::FileIOBuilder; use crate::spec::table_metadata::TableMetadata; From 429859f9b8a0b47d9d148c8191acb3663d76c765 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 22 Jul 2025 12:34:39 -0700 Subject: [PATCH 07/10] optimize api --- crates/catalog/glue/src/catalog.rs | 4 ++-- crates/catalog/hms/src/catalog.rs | 4 ++-- crates/catalog/s3tables/src/catalog.rs | 4 ++-- crates/catalog/sql/src/catalog.rs | 4 ++-- crates/iceberg/src/catalog/memory/catalog.rs | 6 +++--- crates/iceberg/src/spec/table_metadata.rs | 15 +++++++++------ 6 files changed, 20 insertions(+), 17 deletions(-) diff --git a/crates/catalog/glue/src/catalog.rs b/crates/catalog/glue/src/catalog.rs index c946d76ed0..533f2d32c5 100644 --- a/crates/catalog/glue/src/catalog.rs +++ b/crates/catalog/glue/src/catalog.rs @@ -395,7 +395,7 @@ impl Catalog for GlueCatalog { .metadata; let metadata_location = create_metadata_location(&location, 0)?; - TableMetadata::write(&self.file_io, &metadata, &metadata_location).await?; + TableMetadata::write_to(&self.file_io, &metadata, &metadata_location).await?; let glue_table = convert_to_glue_table( &table_name, @@ -460,7 +460,7 @@ impl Catalog for GlueCatalog { Some(table) => { let metadata_location = get_metadata_location(&table.parameters)?; - let metadata = TableMetadata::read(&self.file_io, &metadata_location).await?; + let metadata = TableMetadata::read_from(&self.file_io, &metadata_location).await?; Table::builder() .file_io(self.file_io()) diff --git a/crates/catalog/hms/src/catalog.rs b/crates/catalog/hms/src/catalog.rs index 2bf10f20c7..ae42b1ce90 100644 --- a/crates/catalog/hms/src/catalog.rs +++ b/crates/catalog/hms/src/catalog.rs @@ -353,7 +353,7 @@ impl Catalog for HmsCatalog { let metadata_location = create_metadata_location(&location, 0)?; - TableMetadata::write(&self.file_io, &metadata, &metadata_location).await?; + TableMetadata::write_to(&self.file_io, &metadata, &metadata_location).await?; let hive_table = convert_to_hive_table( db_name.clone(), @@ -403,7 +403,7 @@ impl Catalog for HmsCatalog { let metadata_location = get_metadata_location(&hive_table.parameters)?; - let metadata = TableMetadata::read(&self.file_io, &metadata_location).await?; + let metadata = TableMetadata::read_from(&self.file_io, &metadata_location).await?; Table::builder() .file_io(self.file_io()) diff --git a/crates/catalog/s3tables/src/catalog.rs b/crates/catalog/s3tables/src/catalog.rs index 2b773925f9..f00127a013 100644 --- a/crates/catalog/s3tables/src/catalog.rs +++ b/crates/catalog/s3tables/src/catalog.rs @@ -334,7 +334,7 @@ impl Catalog for S3TablesCatalog { let metadata = TableMetadataBuilder::from_table_creation(creation)? .build()? .metadata; - TableMetadata::write(&self.file_io, &metadata, &metadata_location).await?; + TableMetadata::write_to(&self.file_io, &metadata, &metadata_location).await?; // update metadata location self.s3tables_client @@ -386,7 +386,7 @@ impl Catalog for S3TablesCatalog { ), ) })?; - let metadata = TableMetadata::read(&self.file_io, metadata_location).await?; + let metadata = TableMetadata::read_from(&self.file_io, metadata_location).await?; let table = Table::builder() .identifier(table_ident.clone()) diff --git a/crates/catalog/sql/src/catalog.rs b/crates/catalog/sql/src/catalog.rs index c7f2940737..cf850bb85e 100644 --- a/crates/catalog/sql/src/catalog.rs +++ b/crates/catalog/sql/src/catalog.rs @@ -642,7 +642,7 @@ impl Catalog for SqlCatalog { .try_get::(CATALOG_FIELD_METADATA_LOCATION_PROP) .map_err(from_sqlx_error)?; - let metadata = TableMetadata::read(&self.fileio, &tbl_metadata_location).await?; + let metadata = TableMetadata::read_from(&self.fileio, &tbl_metadata_location).await?; Ok(Table::builder() .file_io(self.fileio.clone()) @@ -706,7 +706,7 @@ impl Catalog for SqlCatalog { Uuid::new_v4() ); - TableMetadata::write(&self.fileio, &tbl_metadata, &tbl_metadata_location).await?; + TableMetadata::write_to(&self.fileio, &tbl_metadata, &tbl_metadata_location).await?; self.execute(&format!( "INSERT INTO {CATALOG_TABLE_NAME} diff --git a/crates/iceberg/src/catalog/memory/catalog.rs b/crates/iceberg/src/catalog/memory/catalog.rs index 4af1bc38a0..fe7add1553 100644 --- a/crates/iceberg/src/catalog/memory/catalog.rs +++ b/crates/iceberg/src/catalog/memory/catalog.rs @@ -210,7 +210,7 @@ impl Catalog for MemoryCatalog { Uuid::new_v4() ); - TableMetadata::write(&self.file_io, &metadata, &metadata_location).await?; + TableMetadata::write_to(&self.file_io, &metadata, &metadata_location).await?; root_namespace_state.insert_new_table(&table_ident, metadata_location.clone())?; @@ -227,7 +227,7 @@ impl Catalog for MemoryCatalog { let root_namespace_state = self.root_namespace_state.lock().await; let metadata_location = root_namespace_state.get_existing_table_location(table_ident)?; - let metadata = TableMetadata::read(&self.file_io, metadata_location).await?; + let metadata = TableMetadata::read_from(&self.file_io, metadata_location).await?; Table::builder() .file_io(self.file_io.clone()) @@ -279,7 +279,7 @@ impl Catalog for MemoryCatalog { let mut root_namespace_state = self.root_namespace_state.lock().await; root_namespace_state.insert_new_table(&table_ident.clone(), metadata_location.clone())?; - let metadata = TableMetadata::read(&self.file_io, &metadata_location).await?; + let metadata = TableMetadata::read_from(&self.file_io, &metadata_location).await?; Table::builder() .file_io(self.file_io.clone()) diff --git a/crates/iceberg/src/spec/table_metadata.rs b/crates/iceberg/src/spec/table_metadata.rs index 0157eea061..ea7eb3f5b9 100644 --- a/crates/iceberg/src/spec/table_metadata.rs +++ b/crates/iceberg/src/spec/table_metadata.rs @@ -466,7 +466,10 @@ impl TableMetadata { } /// Read table metadata from the given location. - pub async fn read(file_io: &FileIO, metadata_location: &str) -> Result { + pub async fn read_from( + file_io: &FileIO, + metadata_location: impl ToString, + ) -> Result { let input_file = file_io.new_input(metadata_location)?; let metadata_content = input_file.read().await?; let metadata = serde_json::from_slice::(&metadata_content)?; @@ -474,10 +477,10 @@ impl TableMetadata { } /// Write table metadata to the given location. - pub async fn write( + pub async fn write_to( file_io: &FileIO, metadata: &TableMetadata, - metadata_location: &str, + metadata_location: impl ToString, ) -> Result<()> { file_io .new_output(metadata_location)? @@ -3090,7 +3093,7 @@ mod tests { let metadata_location = format!("{}/metadata.json", temp_path); // Write the metadata - TableMetadata::write(&file_io, &original_metadata, &metadata_location) + TableMetadata::write_to(&file_io, &original_metadata, &metadata_location) .await .unwrap(); @@ -3098,7 +3101,7 @@ mod tests { assert!(fs::metadata(&metadata_location).is_ok()); // Read the metadata back - let read_metadata = TableMetadata::read(&file_io, &metadata_location) + let read_metadata = TableMetadata::read_from(&file_io, &metadata_location) .await .unwrap(); @@ -3112,7 +3115,7 @@ mod tests { let file_io = FileIOBuilder::new_fs_io().build().unwrap(); // Try to read a non-existent file - let result = TableMetadata::read(&file_io, "/nonexistent/path/metadata.json").await; + let result = TableMetadata::read_from(&file_io, "/nonexistent/path/metadata.json").await; // Verify it returns an error assert!(result.is_err()); From f3ec4847d5ddf2a533b5e6b6c8c58bd853a90dee Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 22 Jul 2025 12:38:36 -0700 Subject: [PATCH 08/10] more optimized --- crates/catalog/glue/src/catalog.rs | 2 +- crates/catalog/hms/src/catalog.rs | 2 +- crates/catalog/s3tables/src/catalog.rs | 2 +- crates/catalog/sql/src/catalog.rs | 4 +++- crates/iceberg/src/catalog/memory/catalog.rs | 2 +- crates/iceberg/src/spec/table_metadata.rs | 11 ++++------- 6 files changed, 11 insertions(+), 12 deletions(-) diff --git a/crates/catalog/glue/src/catalog.rs b/crates/catalog/glue/src/catalog.rs index 533f2d32c5..f4b4a01f9a 100644 --- a/crates/catalog/glue/src/catalog.rs +++ b/crates/catalog/glue/src/catalog.rs @@ -395,7 +395,7 @@ impl Catalog for GlueCatalog { .metadata; let metadata_location = create_metadata_location(&location, 0)?; - TableMetadata::write_to(&self.file_io, &metadata, &metadata_location).await?; + metadata.write_to(&self.file_io, &metadata_location).await?; let glue_table = convert_to_glue_table( &table_name, diff --git a/crates/catalog/hms/src/catalog.rs b/crates/catalog/hms/src/catalog.rs index ae42b1ce90..72fb8c6b33 100644 --- a/crates/catalog/hms/src/catalog.rs +++ b/crates/catalog/hms/src/catalog.rs @@ -353,7 +353,7 @@ impl Catalog for HmsCatalog { let metadata_location = create_metadata_location(&location, 0)?; - TableMetadata::write_to(&self.file_io, &metadata, &metadata_location).await?; + metadata.write_to(&self.file_io, &metadata_location).await?; let hive_table = convert_to_hive_table( db_name.clone(), diff --git a/crates/catalog/s3tables/src/catalog.rs b/crates/catalog/s3tables/src/catalog.rs index f00127a013..191356d711 100644 --- a/crates/catalog/s3tables/src/catalog.rs +++ b/crates/catalog/s3tables/src/catalog.rs @@ -334,7 +334,7 @@ impl Catalog for S3TablesCatalog { let metadata = TableMetadataBuilder::from_table_creation(creation)? .build()? .metadata; - TableMetadata::write_to(&self.file_io, &metadata, &metadata_location).await?; + metadata.write_to(&self.file_io, &metadata_location).await?; // update metadata location self.s3tables_client diff --git a/crates/catalog/sql/src/catalog.rs b/crates/catalog/sql/src/catalog.rs index cf850bb85e..56c6fadcf1 100644 --- a/crates/catalog/sql/src/catalog.rs +++ b/crates/catalog/sql/src/catalog.rs @@ -706,7 +706,9 @@ impl Catalog for SqlCatalog { Uuid::new_v4() ); - TableMetadata::write_to(&self.fileio, &tbl_metadata, &tbl_metadata_location).await?; + tbl_metadata + .write_to(&self.fileio, &tbl_metadata_location) + .await?; self.execute(&format!( "INSERT INTO {CATALOG_TABLE_NAME} diff --git a/crates/iceberg/src/catalog/memory/catalog.rs b/crates/iceberg/src/catalog/memory/catalog.rs index fe7add1553..d1d361c7a1 100644 --- a/crates/iceberg/src/catalog/memory/catalog.rs +++ b/crates/iceberg/src/catalog/memory/catalog.rs @@ -210,7 +210,7 @@ impl Catalog for MemoryCatalog { Uuid::new_v4() ); - TableMetadata::write_to(&self.file_io, &metadata, &metadata_location).await?; + metadata.write_to(&self.file_io, &metadata_location).await?; root_namespace_state.insert_new_table(&table_ident, metadata_location.clone())?; diff --git a/crates/iceberg/src/spec/table_metadata.rs b/crates/iceberg/src/spec/table_metadata.rs index ea7eb3f5b9..8ef6334af2 100644 --- a/crates/iceberg/src/spec/table_metadata.rs +++ b/crates/iceberg/src/spec/table_metadata.rs @@ -477,14 +477,10 @@ impl TableMetadata { } /// Write table metadata to the given location. - pub async fn write_to( - file_io: &FileIO, - metadata: &TableMetadata, - metadata_location: impl ToString, - ) -> Result<()> { + pub async fn write_to(&self, file_io: &FileIO, metadata_location: impl ToString) -> Result<()> { file_io .new_output(metadata_location)? - .write(serde_json::to_vec(metadata)?.into()) + .write(serde_json::to_vec(self)?.into()) .await } @@ -3093,7 +3089,8 @@ mod tests { let metadata_location = format!("{}/metadata.json", temp_path); // Write the metadata - TableMetadata::write_to(&file_io, &original_metadata, &metadata_location) + original_metadata + .write_to(&file_io, &metadata_location) .await .unwrap(); From a38e35a59a54ae3f4e7e2413eb4d5910bf693a1b Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 22 Jul 2025 14:10:06 -0700 Subject: [PATCH 09/10] use asref str --- crates/iceberg/src/spec/table_metadata.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/iceberg/src/spec/table_metadata.rs b/crates/iceberg/src/spec/table_metadata.rs index 8ef6334af2..9767939ace 100644 --- a/crates/iceberg/src/spec/table_metadata.rs +++ b/crates/iceberg/src/spec/table_metadata.rs @@ -468,7 +468,7 @@ impl TableMetadata { /// Read table metadata from the given location. pub async fn read_from( file_io: &FileIO, - metadata_location: impl ToString, + metadata_location: impl AsRef, ) -> Result { let input_file = file_io.new_input(metadata_location)?; let metadata_content = input_file.read().await?; @@ -477,7 +477,7 @@ impl TableMetadata { } /// Write table metadata to the given location. - pub async fn write_to(&self, file_io: &FileIO, metadata_location: impl ToString) -> Result<()> { + pub async fn write_to(&self, file_io: &FileIO, metadata_location: impl AsRef) -> Result<()> { file_io .new_output(metadata_location)? .write(serde_json::to_vec(self)?.into()) From 23ca66cdc03875804fff6721af9b123f0c3b4ef2 Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Tue, 22 Jul 2025 14:11:17 -0700 Subject: [PATCH 10/10] fmt matters --- crates/iceberg/src/spec/table_metadata.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/iceberg/src/spec/table_metadata.rs b/crates/iceberg/src/spec/table_metadata.rs index 9767939ace..0f0854f7fc 100644 --- a/crates/iceberg/src/spec/table_metadata.rs +++ b/crates/iceberg/src/spec/table_metadata.rs @@ -477,7 +477,11 @@ impl TableMetadata { } /// Write table metadata to the given location. - pub async fn write_to(&self, file_io: &FileIO, metadata_location: impl AsRef) -> Result<()> { + pub async fn write_to( + &self, + file_io: &FileIO, + metadata_location: impl AsRef, + ) -> Result<()> { file_io .new_output(metadata_location)? .write(serde_json::to_vec(self)?.into())