From e5870523ce1c50a9dfb3203c4bc7205e3d8c48d4 Mon Sep 17 00:00:00 2001 From: Micah Kornfield Date: Thu, 5 Feb 2026 19:39:32 +0000 Subject: [PATCH 1/5] fix: Interpret s3tables warehouse as table_location not metadata location --- crates/catalog/s3tables/src/catalog.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/catalog/s3tables/src/catalog.rs b/crates/catalog/s3tables/src/catalog.rs index 3606fac99a..bb9d8a6688 100644 --- a/crates/catalog/s3tables/src/catalog.rs +++ b/crates/catalog/s3tables/src/catalog.rs @@ -448,9 +448,9 @@ impl Catalog for S3TablesCatalog { .await .map_err(from_aws_sdk_error)?; - // prepare metadata location. the warehouse location is generated by s3tables catalog, + // prepare table location. the warehouse location is generated by s3tables catalog, // which looks like: s3://e6c9bf20-991a-46fb-kni5xs1q2yxi3xxdyxzjzigdeop1quse2b--table-s3 - let metadata_location = match &creation.location { + let table_location = match &creation.location { Some(_) => { return Err(Error::new( ErrorKind::DataInvalid, @@ -467,16 +467,16 @@ impl Catalog for S3TablesCatalog { .send() .await .map_err(from_aws_sdk_error)?; - let warehouse_location = get_resp.warehouse_location().to_string(); - MetadataLocation::new_with_table_location(warehouse_location).to_string() + get_resp.warehouse_location().to_string() } }; // write metadata to file - creation.location = Some(metadata_location.clone()); + creation.location = Some(table_location.clone()); let metadata = TableMetadataBuilder::from_table_creation(creation)? .build()? .metadata; + let metadata_location = MetadataLocation::new_with_table_location(table_location).to_string(); metadata.write_to(&self.file_io, &metadata_location).await?; // update metadata location From f5b64e7d3116028d64511200b9a8b5967207e01a Mon Sep 17 00:00:00 2001 From: Micah Kornfield Date: Thu, 5 Feb 2026 20:03:37 +0000 Subject: [PATCH 2/5] wip --- crates/catalog/s3tables/src/catalog.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/catalog/s3tables/src/catalog.rs b/crates/catalog/s3tables/src/catalog.rs index bb9d8a6688..a6ac60bc79 100644 --- a/crates/catalog/s3tables/src/catalog.rs +++ b/crates/catalog/s3tables/src/catalog.rs @@ -476,7 +476,8 @@ impl Catalog for S3TablesCatalog { let metadata = TableMetadataBuilder::from_table_creation(creation)? .build()? .metadata; - let metadata_location = MetadataLocation::new_with_table_location(table_location).to_string(); + let metadata_location = + MetadataLocation::new_with_table_location(table_location).to_string(); metadata.write_to(&self.file_io, &metadata_location).await?; // update metadata location From d0755334ce65ee47beae8d077fc63f85e09dbc62 Mon Sep 17 00:00:00 2001 From: Micah Kornfield Date: Wed, 11 Feb 2026 21:41:14 +0000 Subject: [PATCH 3/5] add test --- Cargo.lock | 160 +++++++++++++++++++++++++ Cargo.toml | 4 + crates/catalog/s3tables/Cargo.toml | 4 + crates/catalog/s3tables/src/catalog.rs | 132 ++++++++++++++++++++ 4 files changed, 300 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index ed3fd34730..95e2f88eea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -803,23 +803,29 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "59e62db736db19c488966c8d787f52e6270be565727236fd5579eaa301e7bc4a" dependencies = [ "aws-smithy-async", + "aws-smithy-protocol-test", "aws-smithy-runtime-api", "aws-smithy-types", + "bytes", "h2 0.3.27", "h2 0.4.12", "http 0.2.12", "http 1.4.0", "http-body 0.4.6", + "http-body 1.0.1", "hyper 0.14.32", "hyper 1.8.1", "hyper-rustls 0.24.2", "hyper-rustls 0.27.7", "hyper-util", + "indexmap 2.12.1", "pin-project-lite", "rustls 0.21.12", "rustls 0.23.35", "rustls-native-certs", "rustls-pki-types", + "serde", + "serde_json", "tokio", "tokio-rustls 0.26.4", "tower", @@ -844,6 +850,25 @@ dependencies = [ "aws-smithy-runtime-api", ] +[[package]] +name = "aws-smithy-protocol-test" +version = "0.63.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "01317a9e3c5c06f1af35001ef0c873c1e34e458c20b2ee1eee0fb431e6dbb010" +dependencies = [ + "assert-json-diff", + "aws-smithy-runtime-api", + "base64-simd", + "cbor-diag", + "ciborium", + "http 0.2.12", + "pretty_assertions", + "regex-lite", + "roxmltree", + "serde_json", + "thiserror 2.0.17", +] + [[package]] name = "aws-smithy-query" version = "0.60.9" @@ -876,6 +901,7 @@ dependencies = [ "pin-utils", "tokio", "tracing", + "tracing-subscriber", ] [[package]] @@ -1137,6 +1163,15 @@ dependencies = [ "alloc-stdlib", ] +[[package]] +name = "bs58" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf88ba1141d185c399bee5288d850d63b8369520c1eafc32a0430b5b6c287bf4" +dependencies = [ + "tinyvec", +] + [[package]] name = "bumpalo" version = "3.19.0" @@ -1233,6 +1268,25 @@ dependencies = [ "cipher", ] +[[package]] +name = "cbor-diag" +version = "0.1.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc245b6ecd09b23901a4fbad1ad975701fd5061ceaef6afa93a2d70605a64429" +dependencies = [ + "bs58", + "chrono", + "data-encoding", + "half", + "nom", + "num-bigint", + "num-rational", + "num-traits", + "separator", + "url", + "uuid", +] + [[package]] name = "cc" version = "1.2.49" @@ -1281,6 +1335,33 @@ dependencies = [ "phf", ] +[[package]] +name = "ciborium" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42e69ffd6f0917f5c029256a24d0161db17cea3997d185db0d35926308770f0e" +dependencies = [ + "ciborium-io", + "ciborium-ll", + "serde", +] + +[[package]] +name = "ciborium-io" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05afea1e0a06c9be33d539b876f1ce3692f4afea2cb41f740e7743225ed1c757" + +[[package]] +name = "ciborium-ll" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "57663b653d948a338bfb3eeba9bb2fd5fcfaecb9e199e87e1eda4d9e8b240fd9" +dependencies = [ + "ciborium-io", + "half", +] + [[package]] name = "cipher" version = "0.4.4" @@ -1640,6 +1721,12 @@ dependencies = [ "parking_lot_core", ] +[[package]] +name = "data-encoding" +version = "2.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7a1e2f27636f116493b8b860f5546edb47c8d8f8ea73e1d2a20be88e28d1fea" + [[package]] name = "datafusion" version = "51.0.0" @@ -3514,7 +3601,11 @@ dependencies = [ "anyhow", "async-trait", "aws-config", + "aws-credential-types", "aws-sdk-s3tables", + "aws-smithy-runtime", + "aws-smithy-runtime-api", + "aws-smithy-types", "iceberg", "iceberg_test_utils", "itertools 0.13.0", @@ -4138,6 +4229,15 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "matchers" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d1525a2a28c7f4fa0fc98bb91ae755d1e2d1505079e05539e35bc876b5d65ae9" +dependencies = [ + "regex-automata", +] + [[package]] name = "md-5" version = "0.10.6" @@ -4194,6 +4294,12 @@ dependencies = [ "serde", ] +[[package]] +name = "minimal-lexical" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" + [[package]] name = "miniz_oxide" version = "0.8.9" @@ -4382,6 +4488,16 @@ dependencies = [ "libc", ] +[[package]] +name = "nom" +version = "7.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d273983c5a657a70a3e8f2a01329822f3b8c8172b73826411a55751e404a0a4a" +dependencies = [ + "memchr", + "minimal-lexical", +] + [[package]] name = "nu-ansi-term" version = "0.50.3" @@ -4453,6 +4569,17 @@ dependencies = [ "num-traits", ] +[[package]] +name = "num-rational" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f83d14da390562dca69fc84082e73e548e1ad308d24accdedd2720017cb37824" +dependencies = [ + "num-bigint", + "num-integer", + "num-traits", +] + [[package]] name = "num-traits" version = "0.2.19" @@ -5608,6 +5735,15 @@ dependencies = [ "byteorder", ] +[[package]] +name = "roxmltree" +version = "0.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "921904a62e410e37e215c40381b7117f830d9d89ba60ab5236170541dd25646b" +dependencies = [ + "xmlparser", +] + [[package]] name = "rsa" version = "0.9.10" @@ -5971,6 +6107,12 @@ dependencies = [ "serde_core", ] +[[package]] +name = "separator" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f97841a747eef040fcd2e7b3b9a220a7205926e60488e673d9e4926d27772ce5" + [[package]] name = "seq-macro" version = "0.3.6" @@ -6043,6 +6185,7 @@ version = "1.0.145" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "402a6f66d8c709116cf22f558eab210f5a50187f702eb4d7e5ef38d9a7f1c79c" dependencies = [ + "indexmap 2.12.1", "itoa", "memchr", "ryu", @@ -7090,18 +7233,35 @@ dependencies = [ "tracing-core", ] +[[package]] +name = "tracing-serde" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "704b1aeb7be0d0a84fc9828cae51dab5970fee5088f83d1dd7ee6f6246fc6ff1" +dependencies = [ + "serde", + "tracing-core", +] + [[package]] name = "tracing-subscriber" version = "0.3.22" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2f30143827ddab0d256fd843b7a66d164e9f271cfa0dde49142c5ca0ca291f1e" dependencies = [ + "matchers", "nu-ansi-term", + "once_cell", + "regex-automata", + "serde", + "serde_json", "sharded-slab", "smallvec", "thread_local", + "tracing", "tracing-core", "tracing-log", + "tracing-serde", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index e114217b29..ec2c8fe163 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,8 +53,12 @@ arrow-string = "57.0" as-any = "0.3.2" async-trait = "0.1.89" aws-config = "1.8.7" +aws-credential-types = "1.2" aws-sdk-glue = "1.39" aws-sdk-s3tables = "1.28.0" +aws-smithy-runtime = "1.9" +aws-smithy-runtime-api = "1.9" +aws-smithy-types = "1.3" backon = "1.5.1" base64 = "0.22.1" bimap = "0.6" diff --git a/crates/catalog/s3tables/Cargo.toml b/crates/catalog/s3tables/Cargo.toml index fde08b9a49..bc296f096d 100644 --- a/crates/catalog/s3tables/Cargo.toml +++ b/crates/catalog/s3tables/Cargo.toml @@ -38,6 +38,10 @@ iceberg = { workspace = true } [dev-dependencies] +aws-credential-types = { workspace = true } +aws-smithy-runtime = { workspace = true, features = ["test-util"] } +aws-smithy-runtime-api = { workspace = true } +aws-smithy-types = { workspace = true } iceberg_test_utils = { path = "../../test_utils", features = ["tests"] } itertools = { workspace = true } tokio = { workspace = true } diff --git a/crates/catalog/s3tables/src/catalog.rs b/crates/catalog/s3tables/src/catalog.rs index a6ac60bc79..cad6851513 100644 --- a/crates/catalog/s3tables/src/catalog.rs +++ b/crates/catalog/s3tables/src/catalog.rs @@ -656,6 +656,11 @@ where T: std::fmt::Debug { #[cfg(test)] mod tests { + use aws_credential_types::Credentials; + use aws_smithy_runtime::client::http::test_util::{ReplayEvent, StaticReplayClient}; + use aws_smithy_runtime_api::client::orchestrator::HttpResponse; + use aws_smithy_runtime_api::http::{Request, Response, StatusCode}; + use aws_smithy_types::body::SdkBody; use iceberg::spec::{NestedField, PrimitiveType, Schema, Type}; use iceberg::transaction::{ApplyTransactionAction, Transaction}; @@ -1124,4 +1129,131 @@ mod tests { assert_eq!(err.message(), "Catalog name cannot be empty"); } } + + // Helper functions for testing + + /// Creates a mock HTTP response with the given JSON body + fn mock_http_response(json_body: &str) -> HttpResponse { + HttpResponse::try_from( + Response::new(StatusCode::try_from(200).unwrap(), SdkBody::from(json_body)) + ).unwrap() + } + + /// Creates a test catalog with mocked S3 Tables client that returns the given HTTP responses + fn create_test_catalog_with_responses(responses: Vec) -> S3TablesCatalog { + // Create mocked S3 Tables client + let events: Vec = responses + .into_iter() + .map(|response| { + ReplayEvent::new(Request::new(SdkBody::empty()), response) + }) + .collect(); + + let http_client = StaticReplayClient::new(events); + let creds = Credentials::new("test", "test", None, None, "test"); + + let config = aws_sdk_s3tables::Config::builder() + .region(aws_sdk_s3tables::config::Region::new("us-east-1")) + .credentials_provider(creds) + .http_client(http_client) + .behavior_version(aws_sdk_s3tables::config::BehaviorVersion::latest()) + .build(); + + let s3tables_client = aws_sdk_s3tables::Client::from_conf(config); + + // Create in-memory file IO + let file_io = FileIOBuilder::new("memory").build().unwrap(); + + // Assemble catalog + S3TablesCatalog { + config: S3TablesCatalogConfig { + name: Some("test".to_string()), + table_bucket_arn: "arn:aws:s3tables:us-east-1:123456789012:bucket/test" + .to_string(), + endpoint_url: None, + client: Some(s3tables_client.clone()), + props: HashMap::new(), + }, + s3tables_client, + file_io, + } + } + + /// Creates a simple test table creation with id and name fields + fn create_test_table_creation(table_name: &str) -> TableCreation { + let schema = Schema::builder() + .with_schema_id(0) + .with_fields(vec![ + NestedField::required(1, "id", Type::Primitive(PrimitiveType::Int)).into(), + NestedField::required(2, "name", Type::Primitive(PrimitiveType::String)).into(), + ]) + .build() + .unwrap(); + + TableCreation::builder() + .name(table_name.to_string()) + .properties(HashMap::new()) + .schema(schema) + .build() + } + + #[tokio::test] + async fn test_create_table_with_mocked_client() { + // Test values - these are the key business logic being tested + let warehouse_location = "s3://test-warehouse-bucket/table-xyz"; + let version_token = "test-version-token-123"; + + // Mock the three HTTP responses from S3 Tables API in create_table flow: + // 1. CreateTable - returns version token + // 2. GetTable - returns warehouse location and version token + // 3. UpdateTableMetadataLocation - success response + let mock_responses = vec![ + mock_http_response(&format!(r#"{{"versionToken": "{}"}}"#, version_token)), + mock_http_response(&format!( + r#"{{"warehouseLocation": "{}", "versionToken": "{}"}}"#, + warehouse_location, version_token + )), + mock_http_response("{}"), + ]; + + // Create catalog with mocked S3 Tables responses + let catalog = create_test_catalog_with_responses(mock_responses); + + // Call the actual create_table method + let namespace = NamespaceIdent::new("test_namespace".to_string()); + let creation = create_test_table_creation("test_table"); + let result = catalog.create_table(&namespace, creation).await; + + // Verify the table was created successfully + assert!( + result.is_ok(), + "Table creation should succeed, got error: {:?}", + result.err() + ); + + let table = result.unwrap(); + + // Verify: table location is set to warehouse location from S3 Tables + assert_eq!( + table.metadata().location(), + warehouse_location, + "Table location should be set to warehouse location from S3 Tables" + ); + + // Verify: metadata location is constructed FROM table location (the key fix in PR #2115) + let metadata_location = table.metadata_location().expect("metadata location should be set"); + let expected_prefix = format!("{}/metadata/00000-", warehouse_location); + assert!( + metadata_location.starts_with(&expected_prefix), + "Metadata location should be constructed from warehouse location. Expected prefix: {}, Got: {}", + expected_prefix, + metadata_location + ); + + // Verify: metadata location follows the expected pattern + assert!( + metadata_location.ends_with(".metadata.json"), + "Metadata location should end with .metadata.json" + ); + } } From c009ff74259176f9a3af3f097e2670a36a792372 Mon Sep 17 00:00:00 2001 From: Micah Kornfield Date: Wed, 11 Feb 2026 21:49:12 +0000 Subject: [PATCH 4/5] clipp --- crates/catalog/s3tables/src/catalog.rs | 30 ++++++++++++-------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/crates/catalog/s3tables/src/catalog.rs b/crates/catalog/s3tables/src/catalog.rs index cad6851513..8f7633e63f 100644 --- a/crates/catalog/s3tables/src/catalog.rs +++ b/crates/catalog/s3tables/src/catalog.rs @@ -1134,9 +1134,11 @@ mod tests { /// Creates a mock HTTP response with the given JSON body fn mock_http_response(json_body: &str) -> HttpResponse { - HttpResponse::try_from( - Response::new(StatusCode::try_from(200).unwrap(), SdkBody::from(json_body)) - ).unwrap() + HttpResponse::try_from(Response::new( + StatusCode::try_from(200).unwrap(), + SdkBody::from(json_body), + )) + .unwrap() } /// Creates a test catalog with mocked S3 Tables client that returns the given HTTP responses @@ -1144,9 +1146,7 @@ mod tests { // Create mocked S3 Tables client let events: Vec = responses .into_iter() - .map(|response| { - ReplayEvent::new(Request::new(SdkBody::empty()), response) - }) + .map(|response| ReplayEvent::new(Request::new(SdkBody::empty()), response)) .collect(); let http_client = StaticReplayClient::new(events); @@ -1168,8 +1168,7 @@ mod tests { S3TablesCatalog { config: S3TablesCatalogConfig { name: Some("test".to_string()), - table_bucket_arn: "arn:aws:s3tables:us-east-1:123456789012:bucket/test" - .to_string(), + table_bucket_arn: "arn:aws:s3tables:us-east-1:123456789012:bucket/test".to_string(), endpoint_url: None, client: Some(s3tables_client.clone()), props: HashMap::new(), @@ -1208,10 +1207,9 @@ mod tests { // 2. GetTable - returns warehouse location and version token // 3. UpdateTableMetadataLocation - success response let mock_responses = vec![ - mock_http_response(&format!(r#"{{"versionToken": "{}"}}"#, version_token)), + mock_http_response(&format!(r#"{{"versionToken": "{version_token}"}}"#)), mock_http_response(&format!( - r#"{{"warehouseLocation": "{}", "versionToken": "{}"}}"#, - warehouse_location, version_token + r#"{{"warehouseLocation": "{warehouse_location}", "versionToken": "{version_token}"}}"# )), mock_http_response("{}"), ]; @@ -1241,13 +1239,13 @@ mod tests { ); // Verify: metadata location is constructed FROM table location (the key fix in PR #2115) - let metadata_location = table.metadata_location().expect("metadata location should be set"); - let expected_prefix = format!("{}/metadata/00000-", warehouse_location); + let metadata_location = table + .metadata_location() + .expect("metadata location should be set"); + let expected_prefix = format!("{warehouse_location}/metadata/00000-"); assert!( metadata_location.starts_with(&expected_prefix), - "Metadata location should be constructed from warehouse location. Expected prefix: {}, Got: {}", - expected_prefix, - metadata_location + "Metadata location should be constructed from warehouse location. Expected prefix: {expected_prefix}, Got: {metadata_location}" ); // Verify: metadata location follows the expected pattern From 7c5c5cf6077c833d1cfef9e47840e0f7cf90e924 Mon Sep 17 00:00:00 2001 From: Micah Kornfield Date: Thu, 12 Feb 2026 06:17:27 +0000 Subject: [PATCH 5/5] remove test --- Cargo.lock | 160 ------------------------- Cargo.toml | 4 - crates/catalog/s3tables/Cargo.toml | 4 - crates/catalog/s3tables/src/catalog.rs | 130 -------------------- 4 files changed, 298 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 95e2f88eea..ed3fd34730 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -803,29 +803,23 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "59e62db736db19c488966c8d787f52e6270be565727236fd5579eaa301e7bc4a" dependencies = [ "aws-smithy-async", - "aws-smithy-protocol-test", "aws-smithy-runtime-api", "aws-smithy-types", - "bytes", "h2 0.3.27", "h2 0.4.12", "http 0.2.12", "http 1.4.0", "http-body 0.4.6", - "http-body 1.0.1", "hyper 0.14.32", "hyper 1.8.1", "hyper-rustls 0.24.2", "hyper-rustls 0.27.7", "hyper-util", - "indexmap 2.12.1", "pin-project-lite", "rustls 0.21.12", "rustls 0.23.35", "rustls-native-certs", "rustls-pki-types", - "serde", - "serde_json", "tokio", "tokio-rustls 0.26.4", "tower", @@ -850,25 +844,6 @@ dependencies = [ "aws-smithy-runtime-api", ] -[[package]] -name = "aws-smithy-protocol-test" -version = "0.63.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01317a9e3c5c06f1af35001ef0c873c1e34e458c20b2ee1eee0fb431e6dbb010" -dependencies = [ - "assert-json-diff", - "aws-smithy-runtime-api", - "base64-simd", - "cbor-diag", - "ciborium", - "http 0.2.12", - "pretty_assertions", - "regex-lite", - "roxmltree", - "serde_json", - "thiserror 2.0.17", -] - [[package]] name = "aws-smithy-query" version = "0.60.9" @@ -901,7 +876,6 @@ dependencies = [ "pin-utils", "tokio", "tracing", - "tracing-subscriber", ] [[package]] @@ -1163,15 +1137,6 @@ dependencies = [ "alloc-stdlib", ] -[[package]] -name = "bs58" -version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bf88ba1141d185c399bee5288d850d63b8369520c1eafc32a0430b5b6c287bf4" -dependencies = [ - "tinyvec", -] - [[package]] name = "bumpalo" version = "3.19.0" @@ -1268,25 +1233,6 @@ dependencies = [ "cipher", ] -[[package]] -name = "cbor-diag" -version = "0.1.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dc245b6ecd09b23901a4fbad1ad975701fd5061ceaef6afa93a2d70605a64429" -dependencies = [ - "bs58", - "chrono", - "data-encoding", - "half", - "nom", - "num-bigint", - "num-rational", - "num-traits", - "separator", - "url", - "uuid", -] - [[package]] name = "cc" version = "1.2.49" @@ -1335,33 +1281,6 @@ dependencies = [ "phf", ] -[[package]] -name = "ciborium" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "42e69ffd6f0917f5c029256a24d0161db17cea3997d185db0d35926308770f0e" -dependencies = [ - "ciborium-io", - "ciborium-ll", - "serde", -] - -[[package]] -name = "ciborium-io" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05afea1e0a06c9be33d539b876f1ce3692f4afea2cb41f740e7743225ed1c757" - -[[package]] -name = "ciborium-ll" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "57663b653d948a338bfb3eeba9bb2fd5fcfaecb9e199e87e1eda4d9e8b240fd9" -dependencies = [ - "ciborium-io", - "half", -] - [[package]] name = "cipher" version = "0.4.4" @@ -1721,12 +1640,6 @@ dependencies = [ "parking_lot_core", ] -[[package]] -name = "data-encoding" -version = "2.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d7a1e2f27636f116493b8b860f5546edb47c8d8f8ea73e1d2a20be88e28d1fea" - [[package]] name = "datafusion" version = "51.0.0" @@ -3601,11 +3514,7 @@ dependencies = [ "anyhow", "async-trait", "aws-config", - "aws-credential-types", "aws-sdk-s3tables", - "aws-smithy-runtime", - "aws-smithy-runtime-api", - "aws-smithy-types", "iceberg", "iceberg_test_utils", "itertools 0.13.0", @@ -4229,15 +4138,6 @@ dependencies = [ "pkg-config", ] -[[package]] -name = "matchers" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d1525a2a28c7f4fa0fc98bb91ae755d1e2d1505079e05539e35bc876b5d65ae9" -dependencies = [ - "regex-automata", -] - [[package]] name = "md-5" version = "0.10.6" @@ -4294,12 +4194,6 @@ dependencies = [ "serde", ] -[[package]] -name = "minimal-lexical" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" - [[package]] name = "miniz_oxide" version = "0.8.9" @@ -4488,16 +4382,6 @@ dependencies = [ "libc", ] -[[package]] -name = "nom" -version = "7.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d273983c5a657a70a3e8f2a01329822f3b8c8172b73826411a55751e404a0a4a" -dependencies = [ - "memchr", - "minimal-lexical", -] - [[package]] name = "nu-ansi-term" version = "0.50.3" @@ -4569,17 +4453,6 @@ dependencies = [ "num-traits", ] -[[package]] -name = "num-rational" -version = "0.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f83d14da390562dca69fc84082e73e548e1ad308d24accdedd2720017cb37824" -dependencies = [ - "num-bigint", - "num-integer", - "num-traits", -] - [[package]] name = "num-traits" version = "0.2.19" @@ -5735,15 +5608,6 @@ dependencies = [ "byteorder", ] -[[package]] -name = "roxmltree" -version = "0.14.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "921904a62e410e37e215c40381b7117f830d9d89ba60ab5236170541dd25646b" -dependencies = [ - "xmlparser", -] - [[package]] name = "rsa" version = "0.9.10" @@ -6107,12 +5971,6 @@ dependencies = [ "serde_core", ] -[[package]] -name = "separator" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f97841a747eef040fcd2e7b3b9a220a7205926e60488e673d9e4926d27772ce5" - [[package]] name = "seq-macro" version = "0.3.6" @@ -6185,7 +6043,6 @@ version = "1.0.145" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "402a6f66d8c709116cf22f558eab210f5a50187f702eb4d7e5ef38d9a7f1c79c" dependencies = [ - "indexmap 2.12.1", "itoa", "memchr", "ryu", @@ -7233,35 +7090,18 @@ dependencies = [ "tracing-core", ] -[[package]] -name = "tracing-serde" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "704b1aeb7be0d0a84fc9828cae51dab5970fee5088f83d1dd7ee6f6246fc6ff1" -dependencies = [ - "serde", - "tracing-core", -] - [[package]] name = "tracing-subscriber" version = "0.3.22" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2f30143827ddab0d256fd843b7a66d164e9f271cfa0dde49142c5ca0ca291f1e" dependencies = [ - "matchers", "nu-ansi-term", - "once_cell", - "regex-automata", - "serde", - "serde_json", "sharded-slab", "smallvec", "thread_local", - "tracing", "tracing-core", "tracing-log", - "tracing-serde", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index ec2c8fe163..e114217b29 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,12 +53,8 @@ arrow-string = "57.0" as-any = "0.3.2" async-trait = "0.1.89" aws-config = "1.8.7" -aws-credential-types = "1.2" aws-sdk-glue = "1.39" aws-sdk-s3tables = "1.28.0" -aws-smithy-runtime = "1.9" -aws-smithy-runtime-api = "1.9" -aws-smithy-types = "1.3" backon = "1.5.1" base64 = "0.22.1" bimap = "0.6" diff --git a/crates/catalog/s3tables/Cargo.toml b/crates/catalog/s3tables/Cargo.toml index bc296f096d..fde08b9a49 100644 --- a/crates/catalog/s3tables/Cargo.toml +++ b/crates/catalog/s3tables/Cargo.toml @@ -38,10 +38,6 @@ iceberg = { workspace = true } [dev-dependencies] -aws-credential-types = { workspace = true } -aws-smithy-runtime = { workspace = true, features = ["test-util"] } -aws-smithy-runtime-api = { workspace = true } -aws-smithy-types = { workspace = true } iceberg_test_utils = { path = "../../test_utils", features = ["tests"] } itertools = { workspace = true } tokio = { workspace = true } diff --git a/crates/catalog/s3tables/src/catalog.rs b/crates/catalog/s3tables/src/catalog.rs index 8f7633e63f..a6ac60bc79 100644 --- a/crates/catalog/s3tables/src/catalog.rs +++ b/crates/catalog/s3tables/src/catalog.rs @@ -656,11 +656,6 @@ where T: std::fmt::Debug { #[cfg(test)] mod tests { - use aws_credential_types::Credentials; - use aws_smithy_runtime::client::http::test_util::{ReplayEvent, StaticReplayClient}; - use aws_smithy_runtime_api::client::orchestrator::HttpResponse; - use aws_smithy_runtime_api::http::{Request, Response, StatusCode}; - use aws_smithy_types::body::SdkBody; use iceberg::spec::{NestedField, PrimitiveType, Schema, Type}; use iceberg::transaction::{ApplyTransactionAction, Transaction}; @@ -1129,129 +1124,4 @@ mod tests { assert_eq!(err.message(), "Catalog name cannot be empty"); } } - - // Helper functions for testing - - /// Creates a mock HTTP response with the given JSON body - fn mock_http_response(json_body: &str) -> HttpResponse { - HttpResponse::try_from(Response::new( - StatusCode::try_from(200).unwrap(), - SdkBody::from(json_body), - )) - .unwrap() - } - - /// Creates a test catalog with mocked S3 Tables client that returns the given HTTP responses - fn create_test_catalog_with_responses(responses: Vec) -> S3TablesCatalog { - // Create mocked S3 Tables client - let events: Vec = responses - .into_iter() - .map(|response| ReplayEvent::new(Request::new(SdkBody::empty()), response)) - .collect(); - - let http_client = StaticReplayClient::new(events); - let creds = Credentials::new("test", "test", None, None, "test"); - - let config = aws_sdk_s3tables::Config::builder() - .region(aws_sdk_s3tables::config::Region::new("us-east-1")) - .credentials_provider(creds) - .http_client(http_client) - .behavior_version(aws_sdk_s3tables::config::BehaviorVersion::latest()) - .build(); - - let s3tables_client = aws_sdk_s3tables::Client::from_conf(config); - - // Create in-memory file IO - let file_io = FileIOBuilder::new("memory").build().unwrap(); - - // Assemble catalog - S3TablesCatalog { - config: S3TablesCatalogConfig { - name: Some("test".to_string()), - table_bucket_arn: "arn:aws:s3tables:us-east-1:123456789012:bucket/test".to_string(), - endpoint_url: None, - client: Some(s3tables_client.clone()), - props: HashMap::new(), - }, - s3tables_client, - file_io, - } - } - - /// Creates a simple test table creation with id and name fields - fn create_test_table_creation(table_name: &str) -> TableCreation { - let schema = Schema::builder() - .with_schema_id(0) - .with_fields(vec![ - NestedField::required(1, "id", Type::Primitive(PrimitiveType::Int)).into(), - NestedField::required(2, "name", Type::Primitive(PrimitiveType::String)).into(), - ]) - .build() - .unwrap(); - - TableCreation::builder() - .name(table_name.to_string()) - .properties(HashMap::new()) - .schema(schema) - .build() - } - - #[tokio::test] - async fn test_create_table_with_mocked_client() { - // Test values - these are the key business logic being tested - let warehouse_location = "s3://test-warehouse-bucket/table-xyz"; - let version_token = "test-version-token-123"; - - // Mock the three HTTP responses from S3 Tables API in create_table flow: - // 1. CreateTable - returns version token - // 2. GetTable - returns warehouse location and version token - // 3. UpdateTableMetadataLocation - success response - let mock_responses = vec![ - mock_http_response(&format!(r#"{{"versionToken": "{version_token}"}}"#)), - mock_http_response(&format!( - r#"{{"warehouseLocation": "{warehouse_location}", "versionToken": "{version_token}"}}"# - )), - mock_http_response("{}"), - ]; - - // Create catalog with mocked S3 Tables responses - let catalog = create_test_catalog_with_responses(mock_responses); - - // Call the actual create_table method - let namespace = NamespaceIdent::new("test_namespace".to_string()); - let creation = create_test_table_creation("test_table"); - let result = catalog.create_table(&namespace, creation).await; - - // Verify the table was created successfully - assert!( - result.is_ok(), - "Table creation should succeed, got error: {:?}", - result.err() - ); - - let table = result.unwrap(); - - // Verify: table location is set to warehouse location from S3 Tables - assert_eq!( - table.metadata().location(), - warehouse_location, - "Table location should be set to warehouse location from S3 Tables" - ); - - // Verify: metadata location is constructed FROM table location (the key fix in PR #2115) - let metadata_location = table - .metadata_location() - .expect("metadata location should be set"); - let expected_prefix = format!("{warehouse_location}/metadata/00000-"); - assert!( - metadata_location.starts_with(&expected_prefix), - "Metadata location should be constructed from warehouse location. Expected prefix: {expected_prefix}, Got: {metadata_location}" - ); - - // Verify: metadata location follows the expected pattern - assert!( - metadata_location.ends_with(".metadata.json"), - "Metadata location should end with .metadata.json" - ); - } }