From 7f97f7162e0b73e8d4bb9df46d72ec6793c29c60 Mon Sep 17 00:00:00 2001 From: Ray Liu Date: Wed, 4 Feb 2026 11:11:13 +0000 Subject: [PATCH 1/5] Catalog test suite for all catalogs --- Cargo.lock | 3 + Cargo.toml | 1 + .../catalog/glue/tests/glue_catalog_test.rs | 352 +----- crates/catalog/hms/tests/hms_catalog_test.rs | 296 +---- crates/catalog/loader/Cargo.toml | 3 + crates/catalog/loader/tests/catalog_suite.rs | 1010 +++++++++++++++++ .../catalog/rest/tests/rest_catalog_test.rs | 388 +------ crates/catalog/s3tables/src/catalog.rs | 211 ---- crates/catalog/sql/src/catalog.rs | 919 +-------------- 9 files changed, 1025 insertions(+), 2158 deletions(-) create mode 100644 crates/catalog/loader/tests/catalog_suite.rs diff --git a/Cargo.lock b/Cargo.lock index 211b6416cb..f67ff74a9c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3460,6 +3460,9 @@ dependencies = [ "iceberg-catalog-rest", "iceberg-catalog-s3tables", "iceberg-catalog-sql", + "iceberg_test_utils", + "reqwest", + "rstest", "sqlx", "tempfile", "tokio", diff --git a/Cargo.toml b/Cargo.toml index 46ac4736b8..b77124e787 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -108,6 +108,7 @@ rand = "0.8.5" regex = "1.11.3" reqwest = { version = "0.12.12", default-features = false, features = ["json"] } roaring = { version = "0.11" } +rstest = "0.26" rust_decimal = { version = "1.39", default-features = false, features = ["std"] } serde = { version = "1.0.219", features = ["rc"] } serde_bytes = "0.11.17" diff --git a/crates/catalog/glue/tests/glue_catalog_test.rs b/crates/catalog/glue/tests/glue_catalog_test.rs index f6e2060c0f..55af1a6ad9 100644 --- a/crates/catalog/glue/tests/glue_catalog_test.rs +++ b/crates/catalog/glue/tests/glue_catalog_test.rs @@ -24,10 +24,7 @@ use std::collections::HashMap; use iceberg::io::{S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION, S3_SECRET_ACCESS_KEY}; use iceberg::spec::{NestedField, PrimitiveType, Schema, Type}; -use iceberg::transaction::{ApplyTransactionAction, Transaction}; -use iceberg::{ - Catalog, CatalogBuilder, Namespace, NamespaceIdent, Result, TableCreation, TableIdent, -}; +use iceberg::{Catalog, CatalogBuilder, NamespaceIdent, Result, TableCreation, TableIdent}; use iceberg_catalog_glue::{ AWS_ACCESS_KEY_ID, AWS_REGION_NAME, AWS_SECRET_ACCESS_KEY, GLUE_CATALOG_PROP_URI, GLUE_CATALOG_PROP_WAREHOUSE, GlueCatalog, GlueCatalogBuilder, @@ -116,353 +113,6 @@ fn set_table_creation(location: Option, name: impl ToString) -> Result Result<()> { - let catalog = get_catalog().await; - let creation = set_table_creation(None, "my_table")?; - // Use unique namespace to avoid conflicts - let namespace = Namespace::new(NamespaceIdent::new(normalize_test_name_with_parts!( - "test_rename_table" - ))); - cleanup_namespace(&catalog, namespace.name()).await; - - catalog - .create_namespace(namespace.name(), HashMap::new()) - .await?; - - let table = catalog.create_table(namespace.name(), creation).await?; - - let dest = TableIdent::new(namespace.name().clone(), "my_table_rename".to_string()); - - catalog.rename_table(table.identifier(), &dest).await?; - - let table = catalog.load_table(&dest).await?; - assert_eq!(table.identifier(), &dest); - - let src = TableIdent::new(namespace.name().clone(), "my_table".to_string()); - - let src_table_exists = catalog.table_exists(&src).await?; - assert!(!src_table_exists); - - Ok(()) -} - -#[tokio::test] -async fn test_table_exists() -> Result<()> { - let catalog = get_catalog().await; - let creation = set_table_creation(None, "my_table")?; - // Use unique namespace to avoid conflicts - let namespace = Namespace::new(NamespaceIdent::new(normalize_test_name_with_parts!( - "test_table_exists" - ))); - cleanup_namespace(&catalog, namespace.name()).await; - - catalog - .create_namespace(namespace.name(), HashMap::new()) - .await?; - - let ident = TableIdent::new(namespace.name().clone(), "my_table".to_string()); - - let exists = catalog.table_exists(&ident).await?; - assert!(!exists); - - let table = catalog.create_table(namespace.name(), creation).await?; - - let exists = catalog.table_exists(table.identifier()).await?; - - assert!(exists); - - Ok(()) -} - -#[tokio::test] -async fn test_drop_table() -> Result<()> { - let catalog = get_catalog().await; - let creation = set_table_creation(None, "my_table")?; - // Use unique namespace to avoid conflicts - let namespace = Namespace::new(NamespaceIdent::new(normalize_test_name_with_parts!( - "test_drop_table" - ))); - cleanup_namespace(&catalog, namespace.name()).await; - - catalog - .create_namespace(namespace.name(), HashMap::new()) - .await?; - - let table = catalog.create_table(namespace.name(), creation).await?; - - catalog.drop_table(table.identifier()).await?; - - let result = catalog.table_exists(table.identifier()).await?; - - assert!(!result); - - Ok(()) -} - -#[tokio::test] -async fn test_load_table() -> Result<()> { - let catalog = get_catalog().await; - let creation = set_table_creation(None, "my_table")?; - // Use unique namespace to avoid conflicts - let namespace = Namespace::new(NamespaceIdent::new(normalize_test_name_with_parts!( - "test_load_table" - ))); - cleanup_namespace(&catalog, namespace.name()).await; - - catalog - .create_namespace(namespace.name(), HashMap::new()) - .await?; - - let expected = catalog.create_table(namespace.name(), creation).await?; - - let result = catalog - .load_table(&TableIdent::new( - namespace.name().clone(), - "my_table".to_string(), - )) - .await?; - - assert_eq!(result.identifier(), expected.identifier()); - assert_eq!(result.metadata_location(), expected.metadata_location()); - assert_eq!(result.metadata(), expected.metadata()); - - Ok(()) -} - -#[tokio::test] -async fn test_create_table() -> Result<()> { - let catalog = get_catalog().await; - // Use unique namespace to avoid conflicts - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!("test_create_table")); - cleanup_namespace(&catalog, &namespace).await; - set_test_namespace(&catalog, &namespace).await?; - // inject custom location, ignore the namespace prefix - let creation = set_table_creation(Some("s3a://warehouse/hive".into()), "my_table")?; - let result = catalog.create_table(&namespace, creation).await?; - - assert_eq!(result.identifier().name(), "my_table"); - assert!( - result - .metadata_location() - .is_some_and(|location| location.starts_with("s3a://warehouse/hive/metadata/00000-")) - ); - assert!( - catalog - .file_io() - .exists("s3a://warehouse/hive/metadata/") - .await? - ); - - Ok(()) -} - -#[tokio::test] -async fn test_list_tables() -> Result<()> { - let catalog = get_catalog().await; - // Use unique namespace to avoid conflicts - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!("test_list_tables")); - cleanup_namespace(&catalog, &namespace).await; - set_test_namespace(&catalog, &namespace).await?; - - let expected = vec![]; - let result = catalog.list_tables(&namespace).await?; - - assert_eq!(result, expected); - - Ok(()) -} - -#[tokio::test] -async fn test_drop_namespace() -> Result<()> { - let catalog = get_catalog().await; - // Use unique namespace to avoid conflicts - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!("test_drop_namespace")); - cleanup_namespace(&catalog, &namespace).await; - set_test_namespace(&catalog, &namespace).await?; - - let exists = catalog.namespace_exists(&namespace).await?; - assert!(exists); - - catalog.drop_namespace(&namespace).await?; - - let exists = catalog.namespace_exists(&namespace).await?; - assert!(!exists); - - Ok(()) -} - -#[tokio::test] -async fn test_update_namespace() -> Result<()> { - let catalog = get_catalog().await; - // Use unique namespace to avoid conflicts - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!("test_update_namespace")); - cleanup_namespace(&catalog, &namespace).await; - set_test_namespace(&catalog, &namespace).await?; - - let before_update = catalog.get_namespace(&namespace).await?; - let before_update = before_update.properties().get("description"); - - assert_eq!(before_update, None); - - let properties = HashMap::from([("description".to_string(), "my_update".to_string())]); - - catalog.update_namespace(&namespace, properties).await?; - - let after_update = catalog.get_namespace(&namespace).await?; - let after_update = after_update.properties().get("description"); - - assert_eq!(after_update, Some("my_update".to_string()).as_ref()); - - Ok(()) -} - -#[tokio::test] -async fn test_namespace_exists() -> Result<()> { - let catalog = get_catalog().await; - - // Use unique namespace to avoid conflicts - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!("test_namespace_exists")); - cleanup_namespace(&catalog, &namespace).await; - - let exists = catalog.namespace_exists(&namespace).await?; - assert!(!exists); - - set_test_namespace(&catalog, &namespace).await?; - - let exists = catalog.namespace_exists(&namespace).await?; - assert!(exists); - - Ok(()) -} - -#[tokio::test] -async fn test_get_namespace() -> Result<()> { - let catalog = get_catalog().await; - - // Use unique namespace to avoid conflicts - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!("test_get_namespace")); - cleanup_namespace(&catalog, &namespace).await; - - let does_not_exist = catalog.get_namespace(&namespace).await; - assert!(does_not_exist.is_err()); - - set_test_namespace(&catalog, &namespace).await?; - - let result = catalog.get_namespace(&namespace).await?; - let expected = Namespace::new(namespace); - - assert_eq!(result, expected); - - Ok(()) -} - -#[tokio::test] -async fn test_create_namespace() -> Result<()> { - let catalog = get_catalog().await; - - let properties = HashMap::new(); - // Use unique namespace to avoid conflicts - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!("test_create_namespace")); - cleanup_namespace(&catalog, &namespace).await; - - let expected = Namespace::new(namespace.clone()); - - let result = catalog.create_namespace(&namespace, properties).await?; - - assert_eq!(result, expected); - - Ok(()) -} - -#[tokio::test] -async fn test_list_namespace() -> Result<()> { - let catalog = get_catalog().await; - - // Use unique namespace to avoid conflicts - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!("test_list_namespace")); - cleanup_namespace(&catalog, &namespace).await; - set_test_namespace(&catalog, &namespace).await?; - - let result = catalog.list_namespaces(None).await?; - assert!(result.contains(&namespace)); - - let empty_result = catalog.list_namespaces(Some(&namespace)).await?; - assert!(empty_result.is_empty()); - - Ok(()) -} - -#[tokio::test] -async fn test_update_table() -> Result<()> { - let catalog = get_catalog().await; - let creation = set_table_creation(None, "my_table")?; - // Use unique namespace to avoid conflicts - let namespace = Namespace::new(NamespaceIdent::new(normalize_test_name_with_parts!( - "test_update_table" - ))); - cleanup_namespace(&catalog, namespace.name()).await; - - catalog - .create_namespace(namespace.name(), HashMap::new()) - .await?; - - let expected = catalog.create_table(namespace.name(), creation).await?; - - let table = catalog - .load_table(&TableIdent::new( - namespace.name().clone(), - "my_table".to_string(), - )) - .await?; - - assert_eq!(table.identifier(), expected.identifier()); - assert_eq!(table.metadata_location(), expected.metadata_location()); - assert_eq!(table.metadata(), expected.metadata()); - - // Store the original metadata location for comparison - let original_metadata_location = table.metadata_location(); - - // Update table properties using the transaction - let tx = Transaction::new(&table); - let tx = tx - .update_table_properties() - .set("test_property".to_string(), "test_value".to_string()) - .apply(tx)?; - - // Commit the transaction to the catalog - let updated_table = tx.commit(&catalog).await?; - - // Verify the update was successful - assert_eq!( - updated_table.metadata().properties().get("test_property"), - Some(&"test_value".to_string()) - ); - - // Verify the metadata location has been updated - assert_ne!( - updated_table.metadata_location(), - original_metadata_location, - "Metadata location should be updated after commit" - ); - - // Load the table again from the catalog to verify changes were persisted - let reloaded_table = catalog.load_table(table.identifier()).await?; - - // Verify the reloaded table matches the updated table - assert_eq!( - reloaded_table.metadata().properties().get("test_property"), - Some(&"test_value".to_string()) - ); - assert_eq!( - reloaded_table.metadata_location(), - updated_table.metadata_location(), - "Reloaded table should have the same metadata location as the updated table" - ); - - Ok(()) -} - #[tokio::test] async fn test_register_table() -> Result<()> { let catalog = get_catalog().await; diff --git a/crates/catalog/hms/tests/hms_catalog_test.rs b/crates/catalog/hms/tests/hms_catalog_test.rs index bc036d0c6b..e0178ff1d3 100644 --- a/crates/catalog/hms/tests/hms_catalog_test.rs +++ b/crates/catalog/hms/tests/hms_catalog_test.rs @@ -23,15 +23,12 @@ use std::collections::HashMap; use iceberg::io::{S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION, S3_SECRET_ACCESS_KEY}; -use iceberg::spec::{NestedField, PrimitiveType, Schema, Type}; -use iceberg::{Catalog, CatalogBuilder, Namespace, NamespaceIdent, TableCreation, TableIdent}; +use iceberg::{Catalog, CatalogBuilder, Namespace, NamespaceIdent}; use iceberg_catalog_hms::{ HMS_CATALOG_PROP_THRIFT_TRANSPORT, HMS_CATALOG_PROP_URI, HMS_CATALOG_PROP_WAREHOUSE, HmsCatalog, HmsCatalogBuilder, THRIFT_TRANSPORT_BUFFERED, }; -use iceberg_test_utils::{ - cleanup_namespace, get_hms_endpoint, get_minio_endpoint, normalize_test_name_with_parts, set_up, -}; +use iceberg_test_utils::{get_hms_endpoint, get_minio_endpoint, set_up}; use tokio::time::sleep; use tracing::info; @@ -83,230 +80,6 @@ async fn get_catalog() -> HmsCatalog { .unwrap() } -async fn set_test_namespace(catalog: &HmsCatalog, namespace: &NamespaceIdent) -> Result<()> { - let properties = HashMap::new(); - - catalog.create_namespace(namespace, properties).await?; - - Ok(()) -} - -fn set_table_creation(location: Option, name: impl ToString) -> Result { - let schema = Schema::builder() - .with_schema_id(0) - .with_fields(vec![ - NestedField::required(1, "foo", Type::Primitive(PrimitiveType::Int)).into(), - NestedField::required(2, "bar", Type::Primitive(PrimitiveType::String)).into(), - ]) - .build()?; - - let builder = TableCreation::builder() - .name(name.to_string()) - .properties(HashMap::new()) - .location_opt(location) - .schema(schema); - - Ok(builder.build()) -} - -#[tokio::test] -async fn test_rename_table() -> Result<()> { - let catalog = get_catalog().await; - let creation: TableCreation = set_table_creation(None, "my_table")?; - // Use unique namespace to avoid conflicts - let namespace = Namespace::new(NamespaceIdent::new(normalize_test_name_with_parts!( - "test_rename_table" - ))); - // Clean up from any previous test runs - cleanup_namespace(&catalog, namespace.name()).await; - set_test_namespace(&catalog, namespace.name()).await?; - - let table: iceberg::table::Table = catalog.create_table(namespace.name(), creation).await?; - - let dest = TableIdent::new(namespace.name().clone(), "my_table_rename".to_string()); - - catalog.rename_table(table.identifier(), &dest).await?; - - let result = catalog.table_exists(&dest).await?; - - assert!(result); - - Ok(()) -} - -#[tokio::test] -async fn test_table_exists() -> Result<()> { - let catalog = get_catalog().await; - let creation = set_table_creation(None, "my_table")?; - // Use unique namespace to avoid conflicts - let namespace = Namespace::new(NamespaceIdent::new(normalize_test_name_with_parts!( - "test_table_exists" - ))); - cleanup_namespace(&catalog, namespace.name()).await; - set_test_namespace(&catalog, namespace.name()).await?; - - let table = catalog.create_table(namespace.name(), creation).await?; - - let result = catalog.table_exists(table.identifier()).await?; - - assert!(result); - - Ok(()) -} - -#[tokio::test] -async fn test_drop_table() -> Result<()> { - let catalog = get_catalog().await; - let creation = set_table_creation(None, "my_table")?; - // Use unique namespace to avoid conflicts - let namespace = Namespace::new(NamespaceIdent::new(normalize_test_name_with_parts!( - "test_drop_table" - ))); - cleanup_namespace(&catalog, namespace.name()).await; - set_test_namespace(&catalog, namespace.name()).await?; - - let table = catalog.create_table(namespace.name(), creation).await?; - - catalog.drop_table(table.identifier()).await?; - - let result = catalog.table_exists(table.identifier()).await?; - - assert!(!result); - - Ok(()) -} - -#[tokio::test] -async fn test_load_table() -> Result<()> { - let catalog = get_catalog().await; - let creation = set_table_creation(None, "my_table")?; - // Use unique namespace to avoid conflicts - let namespace = Namespace::new(NamespaceIdent::new(normalize_test_name_with_parts!( - "test_load_table" - ))); - cleanup_namespace(&catalog, namespace.name()).await; - set_test_namespace(&catalog, namespace.name()).await?; - - let expected = catalog.create_table(namespace.name(), creation).await?; - - let result = catalog - .load_table(&TableIdent::new( - namespace.name().clone(), - "my_table".to_string(), - )) - .await?; - - assert_eq!(result.identifier(), expected.identifier()); - assert_eq!(result.metadata_location(), expected.metadata_location()); - assert_eq!(result.metadata(), expected.metadata()); - - Ok(()) -} - -#[tokio::test] -async fn test_create_table() -> Result<()> { - let catalog = get_catalog().await; - // inject custom location, ignore the namespace prefix - let creation = set_table_creation(Some("s3a://warehouse/hive".into()), "my_table")?; - // Use unique namespace to avoid conflicts - let namespace = Namespace::new(NamespaceIdent::new(normalize_test_name_with_parts!( - "test_create_table" - ))); - cleanup_namespace(&catalog, namespace.name()).await; - set_test_namespace(&catalog, namespace.name()).await?; - - let result = catalog.create_table(namespace.name(), creation).await?; - - assert_eq!(result.identifier().name(), "my_table"); - assert!( - result - .metadata_location() - .is_some_and(|location| location.starts_with("s3a://warehouse/hive/metadata/00000-")) - ); - assert!( - catalog - .file_io() - .exists("s3a://warehouse/hive/metadata/") - .await? - ); - - Ok(()) -} - -#[tokio::test] -async fn test_list_tables() -> Result<()> { - let catalog = get_catalog().await; - // Use unique namespace to avoid conflicts - let ns = Namespace::new(NamespaceIdent::new(normalize_test_name_with_parts!( - "test_list_tables" - ))); - // Clean up and create namespace, then verify it's empty - cleanup_namespace(&catalog, ns.name()).await; - set_test_namespace(&catalog, ns.name()).await?; - let result = catalog.list_tables(ns.name()).await?; - - assert_eq!(result, vec![]); - - let creation = set_table_creation(None, "my_table")?; - catalog.create_table(ns.name(), creation).await?; - let result = catalog.list_tables(ns.name()).await?; - - assert_eq!(result, vec![TableIdent::new( - ns.name().clone(), - "my_table".to_string() - )]); - - Ok(()) -} - -#[tokio::test] -async fn test_list_namespace() -> Result<()> { - let catalog = get_catalog().await; - - let result_no_parent = catalog.list_namespaces(None).await?; - - let result_with_parent = catalog - .list_namespaces(Some(&NamespaceIdent::new("parent".into()))) - .await?; - - assert!(result_no_parent.contains(&NamespaceIdent::new("default".into()))); - assert!(result_with_parent.is_empty()); - - Ok(()) -} - -#[tokio::test] -async fn test_create_namespace() -> Result<()> { - let catalog = get_catalog().await; - - let properties = HashMap::from([ - ("comment".to_string(), "my_description".to_string()), - ("location".to_string(), "my_location".to_string()), - ( - "hive.metastore.database.owner".to_string(), - "apache".to_string(), - ), - ( - "hive.metastore.database.owner-type".to_string(), - "user".to_string(), - ), - ("key1".to_string(), "value1".to_string()), - ]); - - // Use unique namespace to avoid conflicts - let ns = Namespace::with_properties( - NamespaceIdent::new(normalize_test_name_with_parts!("test_create_namespace")), - properties.clone(), - ); - cleanup_namespace(&catalog, ns.name()).await; - - let result = catalog.create_namespace(ns.name(), properties).await?; - - assert_eq!(result, ns); - - Ok(()) -} - #[tokio::test] async fn test_get_default_namespace() -> Result<()> { let catalog = get_catalog().await; @@ -333,68 +106,3 @@ async fn test_get_default_namespace() -> Result<()> { Ok(()) } - -#[tokio::test] -async fn test_namespace_exists() -> Result<()> { - let catalog = get_catalog().await; - - let ns_exists = Namespace::new(NamespaceIdent::new("default".into())); - // Use unique namespace to ensure it doesn't exist - let ns_not_exists = Namespace::new(NamespaceIdent::new(normalize_test_name_with_parts!( - "test_namespace_exists" - ))); - cleanup_namespace(&catalog, ns_not_exists.name()).await; - - let result_exists = catalog.namespace_exists(ns_exists.name()).await?; - let result_not_exists = catalog.namespace_exists(ns_not_exists.name()).await?; - - assert!(result_exists); - assert!(!result_not_exists); - - Ok(()) -} - -#[tokio::test] -async fn test_update_namespace() -> Result<()> { - let catalog = get_catalog().await; - - // Use unique namespace to avoid conflicts - let ns = NamespaceIdent::new(normalize_test_name_with_parts!("test_update_namespace")); - cleanup_namespace(&catalog, &ns).await; - set_test_namespace(&catalog, &ns).await?; - let properties = HashMap::from([("comment".to_string(), "my_update".to_string())]); - - catalog.update_namespace(&ns, properties).await?; - - let db = catalog.get_namespace(&ns).await?; - - assert_eq!( - db.properties().get("comment"), - Some(&"my_update".to_string()) - ); - - Ok(()) -} - -#[tokio::test] -async fn test_drop_namespace() -> Result<()> { - let catalog = get_catalog().await; - - // Use unique namespace to avoid conflicts - let ns = Namespace::new(NamespaceIdent::new(normalize_test_name_with_parts!( - "test_drop_namespace" - ))); - cleanup_namespace(&catalog, ns.name()).await; - - catalog.create_namespace(ns.name(), HashMap::new()).await?; - - let result = catalog.namespace_exists(ns.name()).await?; - assert!(result); - - catalog.drop_namespace(ns.name()).await?; - - let result = catalog.namespace_exists(ns.name()).await?; - assert!(!result); - - Ok(()) -} diff --git a/crates/catalog/loader/Cargo.toml b/crates/catalog/loader/Cargo.toml index d4b925fb94..45ea3fbb14 100644 --- a/crates/catalog/loader/Cargo.toml +++ b/crates/catalog/loader/Cargo.toml @@ -39,5 +39,8 @@ tokio = { workspace = true } async-trait = { workspace = true } [dev-dependencies] +iceberg_test_utils = { path = "../../test_utils", features = ["tests"] } +reqwest = { workspace = true } +rstest = { workspace = true } sqlx = { workspace = true, features = ["runtime-tokio", "sqlite", "migrate"] } tempfile = { workspace = true } diff --git a/crates/catalog/loader/tests/catalog_suite.rs b/crates/catalog/loader/tests/catalog_suite.rs new file mode 100644 index 0000000000..873b034745 --- /dev/null +++ b/crates/catalog/loader/tests/catalog_suite.rs @@ -0,0 +1,1010 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Unified catalog integration tests. +//! +//! These assertions represent common catalog behavior that every implementation +//! should satisfy. Catalog-specific behaviors stay in their own crates. +//! +//! These tests assume Docker containers are started externally via `make docker-up`. + +use std::collections::HashMap; +use std::sync::Arc; + +use iceberg::io::{S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION, S3_SECRET_ACCESS_KEY}; +use iceberg::memory::{MEMORY_CATALOG_WAREHOUSE, MemoryCatalogBuilder}; +use iceberg::spec::{NestedField, PrimitiveType, Schema, Type}; +use iceberg::transaction::{ApplyTransactionAction, Transaction}; +use iceberg::{ + Catalog, CatalogBuilder, ErrorKind, NamespaceIdent, Result, TableCreation, TableIdent, +}; +use iceberg_catalog_glue::{ + AWS_ACCESS_KEY_ID, AWS_REGION_NAME, AWS_SECRET_ACCESS_KEY, GLUE_CATALOG_PROP_URI, + GLUE_CATALOG_PROP_WAREHOUSE, GlueCatalog, GlueCatalogBuilder, +}; +use iceberg_catalog_hms::{ + HMS_CATALOG_PROP_THRIFT_TRANSPORT, HMS_CATALOG_PROP_URI, HMS_CATALOG_PROP_WAREHOUSE, + HmsCatalog, HmsCatalogBuilder, THRIFT_TRANSPORT_BUFFERED, +}; +use iceberg_catalog_rest::{REST_CATALOG_PROP_URI, RestCatalog, RestCatalogBuilder}; +use iceberg_catalog_s3tables::{ + S3TABLES_CATALOG_PROP_ENDPOINT_URL, S3TABLES_CATALOG_PROP_TABLE_BUCKET_ARN, + S3TablesCatalogBuilder, +}; +use iceberg_catalog_sql::{ + SQL_CATALOG_PROP_BIND_STYLE, SQL_CATALOG_PROP_URI, SQL_CATALOG_PROP_WAREHOUSE, SqlBindStyle, + SqlCatalogBuilder, +}; +use iceberg_test_utils::{ + get_glue_endpoint, get_hms_endpoint, get_minio_endpoint, get_rest_catalog_endpoint, + normalize_test_name_with_parts, set_up, +}; +use rstest::rstest; +use sqlx::migrate::MigrateDatabase; +use tempfile::TempDir; +use tokio::time::sleep; + +#[derive(Debug, Clone, Copy)] +#[allow(dead_code)] +enum CatalogKind { + Rest, + Glue, + Hms, + Sql, + S3Tables, + Memory, +} + +struct CatalogHarness { + catalog: Arc, + label: &'static str, + _tempdirs: Vec, +} + +// Shared setup for each catalog implementation so the tests below exercise +// the same behavior against all backends. +async fn load_catalog(kind: CatalogKind) -> Option { + set_up(); + match kind { + CatalogKind::Rest => Some(CatalogHarness { + catalog: Arc::new(rest_catalog().await) as Arc, + label: "rest", + _tempdirs: Vec::new(), + }), + CatalogKind::Glue => Some(CatalogHarness { + catalog: Arc::new(glue_catalog().await) as Arc, + label: "glue", + _tempdirs: Vec::new(), + }), + CatalogKind::Hms => Some(CatalogHarness { + catalog: Arc::new(hms_catalog().await) as Arc, + label: "hms", + _tempdirs: Vec::new(), + }), + CatalogKind::Sql => { + let warehouse_dir = TempDir::new().unwrap(); + let db_dir = TempDir::new().unwrap(); + let db_path = db_dir.path().join("catalog.db"); + let db_uri = format!("sqlite:{}", db_path.to_str().unwrap()); + sqlx::Sqlite::create_database(&db_uri).await.unwrap(); + + let catalog = SqlCatalogBuilder::default() + .load( + "sql", + HashMap::from([ + (SQL_CATALOG_PROP_URI.to_string(), db_uri), + ( + SQL_CATALOG_PROP_WAREHOUSE.to_string(), + warehouse_dir.path().to_str().unwrap().to_string(), + ), + ( + SQL_CATALOG_PROP_BIND_STYLE.to_string(), + SqlBindStyle::QMark.to_string(), + ), + ]), + ) + .await + .unwrap(); + + Some(CatalogHarness { + catalog: Arc::new(catalog) as Arc, + label: "sql", + _tempdirs: vec![warehouse_dir, db_dir], + }) + } + CatalogKind::S3Tables => { + let table_bucket_arn = match std::env::var("TABLE_BUCKET_ARN").ok() { + Some(value) => value, + None => return None, + }; + + let mut props = HashMap::from([( + S3TABLES_CATALOG_PROP_TABLE_BUCKET_ARN.to_string(), + table_bucket_arn, + )]); + + if let Ok(endpoint_url) = std::env::var("S3TABLES_ENDPOINT_URL") { + props.insert(S3TABLES_CATALOG_PROP_ENDPOINT_URL.to_string(), endpoint_url); + } + + let catalog = S3TablesCatalogBuilder::default() + .load("s3tables", props) + .await + .unwrap(); + + Some(CatalogHarness { + catalog: Arc::new(catalog) as Arc, + label: "s3tables", + _tempdirs: Vec::new(), + }) + } + CatalogKind::Memory => { + let warehouse_dir = TempDir::new().unwrap(); + let props = HashMap::from([( + MEMORY_CATALOG_WAREHOUSE.to_string(), + warehouse_dir.path().to_str().unwrap().to_string(), + )]); + let catalog = MemoryCatalogBuilder::default() + .load("memory", props) + .await + .unwrap(); + + Some(CatalogHarness { + catalog: Arc::new(catalog) as Arc, + label: "memory", + _tempdirs: vec![warehouse_dir], + }) + } + } +} + +// Catalog-specific setup is intentionally isolated here so the test cases +// remain implementation-agnostic. +async fn rest_catalog() -> RestCatalog { + let rest_endpoint = get_rest_catalog_endpoint(); + + let client = reqwest::Client::new(); + let mut retries = 0; + while retries < 30 { + if client + .get(format!("{rest_endpoint}/v1/config")) + .send() + .await + .map(|resp| resp.status().is_success()) + .unwrap_or(false) + { + break; + } + sleep(std::time::Duration::from_millis(1000)).await; + retries += 1; + } + + RestCatalogBuilder::default() + .load( + "rest", + HashMap::from([(REST_CATALOG_PROP_URI.to_string(), rest_endpoint)]), + ) + .await + .unwrap() +} + +async fn glue_catalog() -> GlueCatalog { + let glue_endpoint = get_glue_endpoint(); + let minio_endpoint = get_minio_endpoint(); + + let props = HashMap::from([ + (AWS_ACCESS_KEY_ID.to_string(), "my_access_id".to_string()), + ( + AWS_SECRET_ACCESS_KEY.to_string(), + "my_secret_key".to_string(), + ), + (AWS_REGION_NAME.to_string(), "us-east-1".to_string()), + (S3_ENDPOINT.to_string(), minio_endpoint), + (S3_ACCESS_KEY_ID.to_string(), "admin".to_string()), + (S3_SECRET_ACCESS_KEY.to_string(), "password".to_string()), + (S3_REGION.to_string(), "us-east-1".to_string()), + ]); + + let file_io = iceberg::io::FileIO::from_path("s3a://") + .unwrap() + .with_props(props.clone()) + .build() + .unwrap(); + + let mut retries = 0; + while retries < 30 { + if file_io.exists("s3a://warehouse/").await.unwrap_or(false) { + break; + } + sleep(std::time::Duration::from_millis(1000)).await; + retries += 1; + } + + let mut glue_props = HashMap::from([ + (GLUE_CATALOG_PROP_URI.to_string(), glue_endpoint), + ( + GLUE_CATALOG_PROP_WAREHOUSE.to_string(), + "s3a://warehouse/hive".to_string(), + ), + ]); + glue_props.extend(props); + + GlueCatalogBuilder::default() + .load("glue", glue_props) + .await + .unwrap() +} + +async fn hms_catalog() -> HmsCatalog { + let hms_endpoint = get_hms_endpoint(); + let minio_endpoint = get_minio_endpoint(); + + let props = HashMap::from([ + (HMS_CATALOG_PROP_URI.to_string(), hms_endpoint), + ( + HMS_CATALOG_PROP_THRIFT_TRANSPORT.to_string(), + THRIFT_TRANSPORT_BUFFERED.to_string(), + ), + ( + HMS_CATALOG_PROP_WAREHOUSE.to_string(), + "s3a://warehouse/hive".to_string(), + ), + (S3_ENDPOINT.to_string(), minio_endpoint), + (S3_ACCESS_KEY_ID.to_string(), "admin".to_string()), + (S3_SECRET_ACCESS_KEY.to_string(), "password".to_string()), + (S3_REGION.to_string(), "us-east-1".to_string()), + ]); + + let file_io = iceberg::io::FileIO::from_path("s3a://") + .unwrap() + .with_props(props.clone()) + .build() + .unwrap(); + + let mut retries = 0; + while retries < 30 { + if file_io.exists("s3a://warehouse/").await.unwrap_or(false) { + break; + } + sleep(std::time::Duration::from_millis(1000)).await; + retries += 1; + } + + HmsCatalogBuilder::default() + .load("hms", props) + .await + .unwrap() +} + +// Common table schema used across the suite to validate shared behavior. +fn table_creation(name: impl ToString) -> TableCreation { + let schema = Schema::builder() + .with_schema_id(0) + .with_fields(vec![ + NestedField::required(1, "foo", Type::Primitive(PrimitiveType::Int)).into(), + NestedField::required(2, "bar", Type::Primitive(PrimitiveType::String)).into(), + ]) + .build() + .unwrap(); + + TableCreation::builder() + .name(name.to_string()) + .properties(HashMap::new()) + .schema(schema) + .build() +} + +fn assert_map_contains(expected: &HashMap, actual: &HashMap) { + for (key, value) in expected { + assert_eq!(actual.get(key), Some(value)); + } +} + +async fn cleanup_namespace_dyn(catalog: &dyn Catalog, namespace: &NamespaceIdent) { + if let Ok(tables) = catalog.list_tables(namespace).await { + for table in tables { + let _ = catalog.drop_table(&table).await; + } + } + let _ = catalog.drop_namespace(namespace).await; +} + +// Common behavior: querying a missing namespace should error. +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_namespace_missing_returns_error(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_namespace_missing_returns_error", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + + assert!(catalog.get_namespace(&namespace).await.is_err()); + + Ok(()) +} + +// Common behavior: namespace lifecycle CRUD (with optional update support). +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_namespace_lifecycle(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_namespace_lifecycle", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + + assert!(!catalog.namespace_exists(&namespace).await?); + + let props = HashMap::from([ + ("owner".to_string(), "rust".to_string()), + ("purpose".to_string(), "catalog_suite".to_string()), + ]); + let created = catalog.create_namespace(&namespace, props.clone()).await?; + assert_eq!(created.name(), &namespace); + assert_map_contains(&props, created.properties()); + + let fetched = catalog.get_namespace(&namespace).await?; + assert_eq!(fetched.name(), &namespace); + assert_map_contains(&props, fetched.properties()); + + let namespaces = catalog.list_namespaces(None).await?; + assert!(namespaces.contains(&namespace)); + + let updated_props = HashMap::from([("owner".to_string(), "updated".to_string())]); + match catalog + .update_namespace(&namespace, updated_props.clone()) + .await + { + Ok(()) => { + let updated = catalog.get_namespace(&namespace).await?; + assert_map_contains(&updated_props, updated.properties()); + } + Err(err) if err.kind() == ErrorKind::FeatureUnsupported => {} + Err(err) => return Err(err), + } + + catalog.drop_namespace(&namespace).await?; + assert!(!catalog.namespace_exists(&namespace).await?); + + Ok(()) +} + +// Common behavior: table lifecycle CRUD. +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_table_lifecycle(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_table_lifecycle", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let table_name = + normalize_test_name_with_parts!("catalog_table_lifecycle", harness.label, "table"); + let table = catalog + .create_table(&namespace, table_creation(table_name)) + .await?; + let ident = table.identifier().clone(); + + assert!(catalog.table_exists(&ident).await?); + let loaded = catalog.load_table(&ident).await?; + assert_eq!(loaded.identifier(), &ident); + + let tables = catalog.list_tables(&namespace).await?; + assert!(tables.contains(&ident)); + + let dest = TableIdent::new(ident.namespace.clone(), format!("{}_renamed", ident.name)); + catalog.rename_table(&ident, &dest).await?; + assert!(catalog.table_exists(&dest).await?); + assert!(!catalog.table_exists(&ident).await?); + + catalog.drop_table(&dest).await?; + assert!(!catalog.table_exists(&dest).await?); + + catalog.drop_namespace(&namespace).await?; + + Ok(()) +} + +// Common behavior: listing namespaces under a parent returns its children. +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_namespace_listing_with_parent(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let parent_name = + normalize_test_name_with_parts!("catalog_namespace_listing_with_parent", harness.label); + let parent = NamespaceIdent::new(parent_name.clone()); + let child1 = NamespaceIdent::from_strs([&parent_name, "child1"]).unwrap(); + let child2 = NamespaceIdent::from_strs([&parent_name, "child2"]).unwrap(); + + cleanup_namespace_dyn(catalog.as_ref(), &child1).await; + cleanup_namespace_dyn(catalog.as_ref(), &child2).await; + cleanup_namespace_dyn(catalog.as_ref(), &parent).await; + + catalog.create_namespace(&parent, HashMap::new()).await?; + catalog.create_namespace(&child1, HashMap::new()).await?; + catalog.create_namespace(&child2, HashMap::new()).await?; + + let top_level = catalog.list_namespaces(None).await?; + assert!(top_level.contains(&parent)); + + let children = catalog.list_namespaces(Some(&parent)).await?; + assert!(children.contains(&child1)); + assert!(children.contains(&child2)); + + Ok(()) +} + +// Common behavior: creating an existing namespace should error. +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_create_namespace_duplicate_fails(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_create_namespace_duplicate_fails", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + assert!( + catalog + .create_namespace(&namespace, HashMap::new()) + .await + .is_err() + ); + + Ok(()) +} + +// Common behavior: update on a missing namespace should error (or be unsupported). +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_update_namespace_missing_errors(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_update_namespace_missing_errors", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + + match catalog + .update_namespace( + &namespace, + HashMap::from([("key".to_string(), "value".to_string())]), + ) + .await + { + Err(err) if err.kind() == ErrorKind::FeatureUnsupported => Ok(()), + Err(_) => Ok(()), + Ok(()) => Err(iceberg::Error::new( + ErrorKind::Unexpected, + "Expected update_namespace to fail for missing namespace", + )), + } +} + +// Common behavior: dropping a missing namespace should error. +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_drop_namespace_missing_errors(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_drop_namespace_missing_errors", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + + assert!(catalog.drop_namespace(&namespace).await.is_err()); + + Ok(()) +} + +// Common behavior: listing tables for a missing namespace should error. +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_list_tables_missing_namespace_errors( + #[case] kind: CatalogKind, +) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_list_tables_missing_namespace_errors", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + + assert!(catalog.list_tables(&namespace).await.is_err()); + + Ok(()) +} + +// Common behavior: listing tables in an empty namespace returns empty. +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_list_tables_empty_namespace(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_list_tables_empty_namespace", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let tables = catalog.list_tables(&namespace).await?; + assert!(tables.is_empty()); + + Ok(()) +} + +// Common behavior: created tables expose the requested schema. +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_create_table_schema(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_create_table_schema", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let table_name = + normalize_test_name_with_parts!("catalog_create_table_schema", harness.label, "table"); + let creation = table_creation(table_name); + let expected_schema = creation.schema.clone(); + + let table = catalog.create_table(&namespace, creation).await?; + assert_eq!(table.identifier().namespace, namespace); + assert_eq!(table.metadata().current_schema().as_ref(), &expected_schema); + + Ok(()) +} + +// Common behavior: updating table properties persists through the catalog. +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_update_table_properties(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_update_table_properties", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let table_name = + normalize_test_name_with_parts!("catalog_update_table_properties", harness.label, "table"); + let table = catalog + .create_table(&namespace, table_creation(table_name)) + .await?; + + let tx = Transaction::new(&table); + let tx = tx + .update_table_properties() + .set("test_property".to_string(), "test_value".to_string()) + .apply(tx)?; + let updated = tx.commit(catalog.as_ref()).await?; + + assert_eq!( + updated.metadata().properties().get("test_property"), + Some(&"test_value".to_string()) + ); + + Ok(()) +} + +// Common behavior: renaming across namespaces moves the table. +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_rename_table_across_namespaces(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let src_namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_rename_table_across_namespaces", + harness.label, + "src" + )); + let dst_namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_rename_table_across_namespaces", + harness.label, + "dst" + )); + + cleanup_namespace_dyn(catalog.as_ref(), &src_namespace).await; + cleanup_namespace_dyn(catalog.as_ref(), &dst_namespace).await; + catalog + .create_namespace(&src_namespace, HashMap::new()) + .await?; + catalog + .create_namespace(&dst_namespace, HashMap::new()) + .await?; + + let table = catalog + .create_table( + &src_namespace, + table_creation(normalize_test_name_with_parts!( + "catalog_rename_table_across_namespaces", + harness.label, + "table" + )), + ) + .await?; + let src_ident = table.identifier().clone(); + let dst_ident = TableIdent::new(dst_namespace.clone(), src_ident.name.clone()); + + match catalog.rename_table(&src_ident, &dst_ident).await { + Ok(()) => { + assert!(catalog.table_exists(&dst_ident).await?); + assert!(!catalog.table_exists(&src_ident).await?); + } + Err(err) if err.kind() == ErrorKind::FeatureUnsupported => return Ok(()), + Err(err) => return Err(err), + } + + Ok(()) +} + +// Common behavior: renaming a missing table should error. +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_rename_table_missing_source_errors(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_rename_table_missing_source_errors", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let src_ident = TableIdent::new(namespace.clone(), "missing".to_string()); + let dst_ident = TableIdent::new(namespace.clone(), "dest".to_string()); + + match catalog.rename_table(&src_ident, &dst_ident).await { + Err(err) if err.kind() == ErrorKind::FeatureUnsupported => Ok(()), + Err(_) => Ok(()), + Ok(()) => Err(iceberg::Error::new( + ErrorKind::Unexpected, + "Expected rename_table to fail for missing source table", + )), + } +} + +// Common behavior: renaming to an existing destination should error. +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_rename_table_dest_exists_errors(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_rename_table_dest_exists_errors", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let src = catalog + .create_table( + &namespace, + table_creation(normalize_test_name_with_parts!( + "catalog_rename_table_dest_exists_errors", + harness.label, + "src" + )), + ) + .await? + .identifier() + .clone(); + let dst = catalog + .create_table( + &namespace, + table_creation(normalize_test_name_with_parts!( + "catalog_rename_table_dest_exists_errors", + harness.label, + "dst" + )), + ) + .await? + .identifier() + .clone(); + + match catalog.rename_table(&src, &dst).await { + Err(err) if err.kind() == ErrorKind::FeatureUnsupported => Ok(()), + Err(_) => Ok(()), + Ok(()) => Err(iceberg::Error::new( + ErrorKind::Unexpected, + "Expected rename_table to fail for existing destination table", + )), + } +} + +// Common behavior: dropping a missing table should error. +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_drop_table_missing_errors(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_drop_table_missing_errors", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let table_ident = TableIdent::new(namespace.clone(), "missing".to_string()); + assert!(catalog.drop_table(&table_ident).await.is_err()); + + Ok(()) +} + +// Common behavior: register_table rehydrates a dropped table (if supported). +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_register_table_roundtrip(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_register_table_roundtrip", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let table = catalog + .create_table( + &namespace, + table_creation(normalize_test_name_with_parts!( + "catalog_register_table_roundtrip", + harness.label, + "table" + )), + ) + .await?; + let table_ident = table.identifier().clone(); + let metadata_location = table + .metadata_location() + .ok_or_else(|| iceberg::Error::new(ErrorKind::Unexpected, "Missing metadata location"))? + .to_string(); + + catalog.drop_table(&table_ident).await?; + + match catalog + .register_table(&table_ident, metadata_location.clone()) + .await + { + Ok(registered) => { + assert_eq!(registered.identifier(), &table_ident); + assert_eq!( + registered.metadata_location(), + Some(metadata_location.as_str()) + ); + } + Err(err) if err.kind() == ErrorKind::FeatureUnsupported => return Ok(()), + Err(err) => return Err(err), + } + + Ok(()) +} + +// Common behavior: registering a table with an existing name should error (if supported). +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_register_table_conflict_errors(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_register_table_conflict_errors", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let table_ident = TableIdent::new( + namespace.clone(), + normalize_test_name_with_parts!( + "catalog_register_table_conflict_errors", + harness.label, + "table" + ), + ); + let table = catalog + .create_table(&namespace, table_creation(table_ident.name.clone())) + .await?; + let metadata_location = table + .metadata_location() + .ok_or_else(|| iceberg::Error::new(ErrorKind::Unexpected, "Missing metadata location"))? + .to_string(); + + match catalog + .register_table(&table_ident, metadata_location) + .await + { + Err(err) if err.kind() == ErrorKind::FeatureUnsupported => Ok(()), + Err(_) => Ok(()), + Ok(_) => Err(iceberg::Error::new( + ErrorKind::Unexpected, + "Expected register_table to fail for existing table", + )), + } +} diff --git a/crates/catalog/rest/tests/rest_catalog_test.rs b/crates/catalog/rest/tests/rest_catalog_test.rs index 60c67caab3..12bb0c8181 100644 --- a/crates/catalog/rest/tests/rest_catalog_test.rs +++ b/crates/catalog/rest/tests/rest_catalog_test.rs @@ -22,9 +22,8 @@ use std::collections::HashMap; -use iceberg::spec::{FormatVersion, NestedField, PrimitiveType, Schema, Type}; -use iceberg::transaction::{ApplyTransactionAction, Transaction}; -use iceberg::{Catalog, CatalogBuilder, Namespace, NamespaceIdent, TableCreation, TableIdent}; +use iceberg::spec::Schema; +use iceberg::{Catalog, CatalogBuilder, NamespaceIdent, TableCreation, TableIdent}; use iceberg_catalog_rest::{REST_CATALOG_PROP_URI, RestCatalog, RestCatalogBuilder}; use iceberg_test_utils::{ cleanup_namespace, get_rest_catalog_endpoint, normalize_test_name_with_parts, set_up, @@ -70,389 +69,6 @@ async fn get_catalog() -> RestCatalog { .unwrap() } -#[tokio::test] -async fn test_get_non_exist_namespace() { - let catalog = get_catalog().await; - - // Use unique namespace name to ensure it doesn't exist - let ns_ident = NamespaceIdent::new(normalize_test_name_with_parts!( - "test_get_non_exist_namespace" - )); - // Clean up from any previous test runs - cleanup_namespace(&catalog, &ns_ident).await; - - let result = catalog.get_namespace(&ns_ident).await; - - assert!(result.is_err()); - assert!(result.unwrap_err().to_string().contains("does not exist")); -} - -#[tokio::test] -async fn test_get_namespace() { - let catalog = get_catalog().await; - - // Use unique namespace to avoid conflicts with other tests - let ns = Namespace::with_properties( - NamespaceIdent::from_strs([ - "apple", - "ios", - &normalize_test_name_with_parts!("test_get_namespace"), - ]) - .unwrap(), - HashMap::from([ - ("owner".to_string(), "ray".to_string()), - ("community".to_string(), "apache".to_string()), - ]), - ); - - // Clean up from any previous test runs - cleanup_namespace(&catalog, ns.name()).await; - - // Verify that namespace doesn't exist - assert!(catalog.get_namespace(ns.name()).await.is_err()); - - // Create this namespace - let created_ns = catalog - .create_namespace(ns.name(), ns.properties().clone()) - .await - .unwrap(); - - assert_eq!(ns.name(), created_ns.name()); - assert_map_contains(ns.properties(), created_ns.properties()); - - // Check that this namespace already exists - let get_ns = catalog.get_namespace(ns.name()).await.unwrap(); - assert_eq!(ns.name(), get_ns.name()); - assert_map_contains(ns.properties(), created_ns.properties()); -} - -#[tokio::test] -async fn test_list_namespace() { - let catalog = get_catalog().await; - - // Use unique parent namespace to avoid conflicts - let parent_ns_name = normalize_test_name_with_parts!("test_list_namespace"); - let parent_ident = NamespaceIdent::from_strs([&parent_ns_name]).unwrap(); - - let ns1 = Namespace::with_properties( - NamespaceIdent::from_strs([&parent_ns_name, "ios"]).unwrap(), - HashMap::from([ - ("owner".to_string(), "ray".to_string()), - ("community".to_string(), "apache".to_string()), - ]), - ); - - let ns2 = Namespace::with_properties( - NamespaceIdent::from_strs([&parent_ns_name, "macos"]).unwrap(), - HashMap::from([ - ("owner".to_string(), "xuanwo".to_string()), - ("community".to_string(), "apache".to_string()), - ]), - ); - - // Clean up from any previous test runs - cleanup_namespace(&catalog, ns1.name()).await; - cleanup_namespace(&catalog, ns2.name()).await; - cleanup_namespace(&catalog, &parent_ident).await; - - // Currently this namespace doesn't exist, so it should return error. - assert!(catalog.list_namespaces(Some(&parent_ident)).await.is_err()); - - // Create namespaces - catalog - .create_namespace(ns1.name(), ns1.properties().clone()) - .await - .unwrap(); - catalog - .create_namespace(ns2.name(), ns1.properties().clone()) - .await - .unwrap(); - - // List namespace - let nss = catalog.list_namespaces(Some(&parent_ident)).await.unwrap(); - - assert!(nss.contains(ns1.name())); - assert!(nss.contains(ns2.name())); -} - -#[tokio::test] -async fn test_list_empty_namespace() { - let catalog = get_catalog().await; - - // Use unique namespace to avoid conflicts - let ns_apple = Namespace::with_properties( - NamespaceIdent::from_strs([ - "list_empty", - "apple", - &normalize_test_name_with_parts!("test_list_empty_namespace"), - ]) - .unwrap(), - HashMap::from([ - ("owner".to_string(), "ray".to_string()), - ("community".to_string(), "apache".to_string()), - ]), - ); - - // Clean up from any previous test runs - cleanup_namespace(&catalog, ns_apple.name()).await; - - // Currently this namespace doesn't exist, so it should return error. - assert!( - catalog - .list_namespaces(Some(ns_apple.name())) - .await - .is_err() - ); - - // Create namespaces - catalog - .create_namespace(ns_apple.name(), ns_apple.properties().clone()) - .await - .unwrap(); - - // List namespace - let nss = catalog - .list_namespaces(Some(ns_apple.name())) - .await - .unwrap(); - assert!(nss.is_empty()); -} - -#[tokio::test] -async fn test_list_root_namespace() { - let catalog = get_catalog().await; - - // Use unique root namespace to avoid conflicts - let root_ns_name = normalize_test_name_with_parts!("test_list_root_namespace"); - let root_ident = NamespaceIdent::from_strs([&root_ns_name]).unwrap(); - - let ns1 = Namespace::with_properties( - NamespaceIdent::from_strs([&root_ns_name, "apple", "ios"]).unwrap(), - HashMap::from([ - ("owner".to_string(), "ray".to_string()), - ("community".to_string(), "apache".to_string()), - ]), - ); - - let ns2 = Namespace::with_properties( - NamespaceIdent::from_strs([&root_ns_name, "google", "android"]).unwrap(), - HashMap::from([ - ("owner".to_string(), "xuanwo".to_string()), - ("community".to_string(), "apache".to_string()), - ]), - ); - - // Clean up from any previous test runs - cleanup_namespace(&catalog, ns1.name()).await; - cleanup_namespace(&catalog, ns2.name()).await; - cleanup_namespace(&catalog, &root_ident).await; - - // Currently this namespace doesn't exist, so it should return error. - assert!(catalog.list_namespaces(Some(&root_ident)).await.is_err()); - - // Create namespaces - catalog - .create_namespace(ns1.name(), ns1.properties().clone()) - .await - .unwrap(); - catalog - .create_namespace(ns2.name(), ns1.properties().clone()) - .await - .unwrap(); - - // List namespace - let nss = catalog.list_namespaces(None).await.unwrap(); - assert!(nss.contains(&root_ident)); -} - -#[tokio::test] -async fn test_create_table() { - let catalog = get_catalog().await; - - // Use unique namespace to avoid conflicts - let ns = Namespace::with_properties( - NamespaceIdent::from_strs([ - "create_table", - "apple", - "ios", - &normalize_test_name_with_parts!("test_create_table"), - ]) - .unwrap(), - HashMap::from([ - ("owner".to_string(), "ray".to_string()), - ("community".to_string(), "apache".to_string()), - ]), - ); - - // Clean up from any previous test runs - cleanup_namespace(&catalog, ns.name()).await; - - // Create namespaces - catalog - .create_namespace(ns.name(), ns.properties().clone()) - .await - .unwrap(); - - let schema = Schema::builder() - .with_schema_id(1) - .with_identifier_field_ids(vec![2]) - .with_fields(vec![ - NestedField::optional(1, "foo", Type::Primitive(PrimitiveType::String)).into(), - NestedField::required(2, "bar", Type::Primitive(PrimitiveType::Int)).into(), - NestedField::optional(3, "baz", Type::Primitive(PrimitiveType::Boolean)).into(), - ]) - .build() - .unwrap(); - - let table_creation = TableCreation::builder() - .name("t1".to_string()) - .schema(schema.clone()) - .build(); - - let table = catalog - .create_table(ns.name(), table_creation) - .await - .unwrap(); - - assert_eq!( - table.identifier(), - &TableIdent::new(ns.name().clone(), "t1".to_string()) - ); - - assert_eq!( - table.metadata().current_schema().as_struct(), - schema.as_struct() - ); - assert_eq!(table.metadata().format_version(), FormatVersion::V2); - assert!(table.metadata().current_snapshot().is_none()); - assert!(table.metadata().history().is_empty()); - assert!(table.metadata().default_sort_order().is_unsorted()); - assert!(table.metadata().default_partition_spec().is_unpartitioned()); -} - -#[tokio::test] -async fn test_update_table() { - let catalog = get_catalog().await; - - // Use unique namespace to avoid conflicts - let ns = Namespace::with_properties( - NamespaceIdent::from_strs([ - "update_table", - "apple", - "ios", - &normalize_test_name_with_parts!("test_update_table"), - ]) - .unwrap(), - HashMap::from([ - ("owner".to_string(), "ray".to_string()), - ("community".to_string(), "apache".to_string()), - ]), - ); - - // Clean up from any previous test runs - cleanup_namespace(&catalog, ns.name()).await; - - // Create namespaces - catalog - .create_namespace(ns.name(), ns.properties().clone()) - .await - .unwrap(); - - let schema = Schema::builder() - .with_schema_id(1) - .with_identifier_field_ids(vec![2]) - .with_fields(vec![ - NestedField::optional(1, "foo", Type::Primitive(PrimitiveType::String)).into(), - NestedField::required(2, "bar", Type::Primitive(PrimitiveType::Int)).into(), - NestedField::optional(3, "baz", Type::Primitive(PrimitiveType::Boolean)).into(), - ]) - .build() - .unwrap(); - - // Now we create a table - let table_creation = TableCreation::builder() - .name("t1".to_string()) - .schema(schema.clone()) - .build(); - - let table = catalog - .create_table(ns.name(), table_creation) - .await - .unwrap(); - - assert_eq!( - table.identifier(), - &TableIdent::new(ns.name().clone(), "t1".to_string()) - ); - - let tx = Transaction::new(&table); - // Update table by committing transaction - let table2 = tx - .update_table_properties() - .set("prop1".to_string(), "v1".to_string()) - .apply(tx) - .unwrap() - .commit(&catalog) - .await - .unwrap(); - - assert_map_contains( - &HashMap::from([("prop1".to_string(), "v1".to_string())]), - table2.metadata().properties(), - ); -} - -fn assert_map_contains(map1: &HashMap, map2: &HashMap) { - for (k, v) in map1 { - assert!(map2.contains_key(k)); - assert_eq!(map2.get(k).unwrap(), v); - } -} - -#[tokio::test] -async fn test_list_empty_multi_level_namespace() { - let catalog = get_catalog().await; - - // Use unique namespace to avoid conflicts - let ns_apple = Namespace::with_properties( - NamespaceIdent::from_strs([ - "multi_level", - "a_a", - "apple", - &normalize_test_name_with_parts!("test_list_empty_multi_level_namespace"), - ]) - .unwrap(), - HashMap::from([ - ("owner".to_string(), "ray".to_string()), - ("community".to_string(), "apache".to_string()), - ]), - ); - - // Clean up from any previous test runs - cleanup_namespace(&catalog, ns_apple.name()).await; - - // Currently this namespace doesn't exist, so it should return error. - assert!( - catalog - .list_namespaces(Some(ns_apple.name())) - .await - .is_err() - ); - - // Create namespaces - catalog - .create_namespace(ns_apple.name(), ns_apple.properties().clone()) - .await - .unwrap(); - - // List namespace - let nss = catalog - .list_namespaces(Some(ns_apple.name())) - .await - .unwrap(); - assert!(nss.is_empty()); -} - #[tokio::test] async fn test_register_table() { let catalog = get_catalog().await; diff --git a/crates/catalog/s3tables/src/catalog.rs b/crates/catalog/s3tables/src/catalog.rs index 3606fac99a..bd3f8d0150 100644 --- a/crates/catalog/s3tables/src/catalog.rs +++ b/crates/catalog/s3tables/src/catalog.rs @@ -655,219 +655,8 @@ where T: std::fmt::Debug { #[cfg(test)] mod tests { - use iceberg::spec::{NestedField, PrimitiveType, Schema, Type}; - use iceberg::transaction::{ApplyTransactionAction, Transaction}; - use super::*; - async fn load_s3tables_catalog_from_env() -> Result> { - let table_bucket_arn = match std::env::var("TABLE_BUCKET_ARN").ok() { - Some(table_bucket_arn) => table_bucket_arn, - None => return Ok(None), - }; - - let config = S3TablesCatalogConfig { - name: None, - table_bucket_arn, - endpoint_url: None, - client: None, - props: HashMap::new(), - }; - - Ok(Some(S3TablesCatalog::new(config).await?)) - } - - #[tokio::test] - async fn test_s3tables_list_namespace() { - let catalog = match load_s3tables_catalog_from_env().await { - Ok(Some(catalog)) => catalog, - Ok(None) => return, - Err(e) => panic!("Error loading catalog: {e}"), - }; - - let namespaces = catalog.list_namespaces(None).await.unwrap(); - assert!(!namespaces.is_empty()); - } - - #[tokio::test] - async fn test_s3tables_list_tables() { - let catalog = match load_s3tables_catalog_from_env().await { - Ok(Some(catalog)) => catalog, - Ok(None) => return, - Err(e) => panic!("Error loading catalog: {e}"), - }; - - let tables = catalog - .list_tables(&NamespaceIdent::new("aws_s3_metadata".to_string())) - .await - .unwrap(); - assert!(!tables.is_empty()); - } - - #[tokio::test] - async fn test_s3tables_load_table() { - let catalog = match load_s3tables_catalog_from_env().await { - Ok(Some(catalog)) => catalog, - Ok(None) => return, - Err(e) => panic!("Error loading catalog: {e}"), - }; - - let table = catalog - .load_table(&TableIdent::new( - NamespaceIdent::new("aws_s3_metadata".to_string()), - "query_storage_metadata".to_string(), - )) - .await - .unwrap(); - println!("{table:?}"); - } - - #[tokio::test] - async fn test_s3tables_create_delete_namespace() { - let catalog = match load_s3tables_catalog_from_env().await { - Ok(Some(catalog)) => catalog, - Ok(None) => return, - Err(e) => panic!("Error loading catalog: {e}"), - }; - - let namespace = NamespaceIdent::new("test_s3tables_create_delete_namespace".to_string()); - catalog - .create_namespace(&namespace, HashMap::new()) - .await - .unwrap(); - assert!(catalog.namespace_exists(&namespace).await.unwrap()); - catalog.drop_namespace(&namespace).await.unwrap(); - assert!(!catalog.namespace_exists(&namespace).await.unwrap()); - } - - #[tokio::test] - async fn test_s3tables_create_delete_table() { - let catalog = match load_s3tables_catalog_from_env().await { - Ok(Some(catalog)) => catalog, - Ok(None) => return, - Err(e) => panic!("Error loading catalog: {e}"), - }; - - let creation = { - let schema = Schema::builder() - .with_schema_id(0) - .with_fields(vec![ - NestedField::required(1, "foo", Type::Primitive(PrimitiveType::Int)).into(), - NestedField::required(2, "bar", Type::Primitive(PrimitiveType::String)).into(), - ]) - .build() - .unwrap(); - TableCreation::builder() - .name("test_s3tables_create_delete_table".to_string()) - .properties(HashMap::new()) - .schema(schema) - .build() - }; - - let namespace = NamespaceIdent::new("test_s3tables_create_delete_table".to_string()); - let table_ident = TableIdent::new( - namespace.clone(), - "test_s3tables_create_delete_table".to_string(), - ); - catalog.drop_namespace(&namespace).await.ok(); - catalog.drop_table(&table_ident).await.ok(); - - catalog - .create_namespace(&namespace, HashMap::new()) - .await - .unwrap(); - catalog.create_table(&namespace, creation).await.unwrap(); - assert!(catalog.table_exists(&table_ident).await.unwrap()); - catalog.drop_table(&table_ident).await.unwrap(); - assert!(!catalog.table_exists(&table_ident).await.unwrap()); - catalog.drop_namespace(&namespace).await.unwrap(); - } - - #[tokio::test] - async fn test_s3tables_update_table() { - let catalog = match load_s3tables_catalog_from_env().await { - Ok(Some(catalog)) => catalog, - Ok(None) => return, - Err(e) => panic!("Error loading catalog: {e}"), - }; - - // Create a test namespace and table - let namespace = NamespaceIdent::new("test_s3tables_update_table".to_string()); - let table_ident = - TableIdent::new(namespace.clone(), "test_s3tables_update_table".to_string()); - - // Clean up any existing resources from previous test runs - catalog.drop_table(&table_ident).await.ok(); - catalog.drop_namespace(&namespace).await.ok(); - - // Create namespace and table - catalog - .create_namespace(&namespace, HashMap::new()) - .await - .unwrap(); - - let creation = { - let schema = Schema::builder() - .with_schema_id(0) - .with_fields(vec![ - NestedField::required(1, "foo", Type::Primitive(PrimitiveType::Int)).into(), - NestedField::required(2, "bar", Type::Primitive(PrimitiveType::String)).into(), - ]) - .build() - .unwrap(); - TableCreation::builder() - .name(table_ident.name().to_string()) - .properties(HashMap::new()) - .schema(schema) - .build() - }; - - let table = catalog.create_table(&namespace, creation).await.unwrap(); - - // Create a transaction to update the table - let tx = Transaction::new(&table); - - // Store the original metadata location for comparison - let original_metadata_location = table.metadata_location(); - - // Update table properties using the transaction - let tx = tx - .update_table_properties() - .set("test_property".to_string(), "test_value".to_string()) - .apply(tx) - .unwrap(); - - // Commit the transaction to the catalog - let updated_table = tx.commit(&catalog).await.unwrap(); - - // Verify the update was successful - assert_eq!( - updated_table.metadata().properties().get("test_property"), - Some(&"test_value".to_string()) - ); - - // Verify the metadata location has been updated - assert_ne!( - updated_table.metadata_location(), - original_metadata_location, - "Metadata location should be updated after commit" - ); - - // Load the table again from the catalog to verify changes were persisted - let reloaded_table = catalog.load_table(&table_ident).await.unwrap(); - - // Verify the reloaded table matches the updated table - assert_eq!( - reloaded_table.metadata().properties().get("test_property"), - Some(&"test_value".to_string()) - ); - assert_eq!( - reloaded_table.metadata_location(), - updated_table.metadata_location(), - "Reloaded table should have the same metadata location as the updated table" - ); - } - #[tokio::test] async fn test_builder_load_missing_bucket_arn() { let builder = S3TablesCatalogBuilder::default(); diff --git a/crates/catalog/sql/src/catalog.rs b/crates/catalog/sql/src/catalog.rs index 8209cd04c1..803e203c1f 100644 --- a/crates/catalog/sql/src/catalog.rs +++ b/crates/catalog/sql/src/catalog.rs @@ -971,35 +971,23 @@ impl Catalog for SqlCatalog { #[cfg(test)] mod tests { - use std::collections::{HashMap, HashSet}; - use std::hash::Hash; + use std::collections::HashMap; - use iceberg::spec::{NestedField, PartitionSpec, PrimitiveType, Schema, SortOrder, Type}; - use iceberg::table::Table; - use iceberg::transaction::{ApplyTransactionAction, Transaction}; + use iceberg::spec::{NestedField, PrimitiveType, Schema, Type}; use iceberg::{Catalog, CatalogBuilder, Namespace, NamespaceIdent, TableCreation, TableIdent}; - use itertools::Itertools; - use regex::Regex; use sqlx::migrate::MigrateDatabase; use tempfile::TempDir; use crate::catalog::{ - NAMESPACE_LOCATION_PROPERTY_KEY, SQL_CATALOG_PROP_BIND_STYLE, SQL_CATALOG_PROP_URI, - SQL_CATALOG_PROP_WAREHOUSE, + SQL_CATALOG_PROP_BIND_STYLE, SQL_CATALOG_PROP_URI, SQL_CATALOG_PROP_WAREHOUSE, }; use crate::{SqlBindStyle, SqlCatalogBuilder}; - const UUID_REGEX_STR: &str = "[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}"; - fn temp_path() -> String { let temp_dir = TempDir::new().unwrap(); temp_dir.path().to_str().unwrap().to_string() } - fn to_set(vec: Vec) -> HashSet { - HashSet::from_iter(vec) - } - fn default_properties() -> HashMap { HashMap::from([("exists".to_string(), "true".to_string())]) } @@ -1064,51 +1052,6 @@ mod tests { } } - fn assert_table_eq(table: &Table, expected_table_ident: &TableIdent, expected_schema: &Schema) { - assert_eq!(table.identifier(), expected_table_ident); - - let metadata = table.metadata(); - - assert_eq!(metadata.current_schema().as_ref(), expected_schema); - - let expected_partition_spec = PartitionSpec::builder(expected_schema.clone()) - .with_spec_id(0) - .build() - .unwrap(); - - assert_eq!( - metadata - .partition_specs_iter() - .map(|p| p.as_ref()) - .collect_vec(), - vec![&expected_partition_spec] - ); - - let expected_sorted_order = SortOrder::builder() - .with_order_id(0) - .with_fields(vec![]) - .build(expected_schema) - .unwrap(); - - assert_eq!( - metadata - .sort_orders_iter() - .map(|s| s.as_ref()) - .collect_vec(), - vec![&expected_sorted_order] - ); - - assert_eq!(metadata.properties(), &HashMap::new()); - - assert!(!table.readonly()); - } - - fn assert_table_metadata_location_matches(table: &Table, regex_str: &str) { - let actual = table.metadata_location().unwrap().to_string(); - let regex = Regex::new(regex_str).unwrap(); - assert!(regex.is_match(&actual)) - } - #[tokio::test] async fn test_initialized() { let warehouse_loc = temp_path(); @@ -1315,199 +1258,6 @@ mod tests { assert!(catalog.is_err()); } - #[tokio::test] - async fn test_list_namespaces_returns_empty_vector() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - - assert_eq!(catalog.list_namespaces(None).await.unwrap(), vec![]); - } - - #[tokio::test] - async fn test_list_namespaces_returns_multiple_namespaces() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident_1 = NamespaceIdent::new("a".into()); - let namespace_ident_2 = NamespaceIdent::new("b".into()); - create_namespaces(&catalog, &vec![&namespace_ident_1, &namespace_ident_2]).await; - - assert_eq!( - to_set(catalog.list_namespaces(None).await.unwrap()), - to_set(vec![namespace_ident_1, namespace_ident_2]) - ); - } - - #[tokio::test] - async fn test_list_namespaces_returns_only_top_level_namespaces() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident_1 = NamespaceIdent::new("a".into()); - let namespace_ident_2 = NamespaceIdent::from_strs(vec!["a", "b"]).unwrap(); - let namespace_ident_3 = NamespaceIdent::new("b".into()); - create_namespaces(&catalog, &vec![ - &namespace_ident_1, - &namespace_ident_2, - &namespace_ident_3, - ]) - .await; - - assert_eq!( - to_set(catalog.list_namespaces(None).await.unwrap()), - to_set(vec![namespace_ident_1, namespace_ident_3]) - ); - } - - #[tokio::test] - async fn test_list_namespaces_returns_no_namespaces_under_parent() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident_1 = NamespaceIdent::new("a".into()); - let namespace_ident_2 = NamespaceIdent::new("b".into()); - create_namespaces(&catalog, &vec![&namespace_ident_1, &namespace_ident_2]).await; - - assert_eq!( - catalog - .list_namespaces(Some(&namespace_ident_1)) - .await - .unwrap(), - vec![] - ); - } - - #[tokio::test] - async fn test_list_namespaces_returns_namespace_under_parent() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident_1 = NamespaceIdent::new("a".into()); - let namespace_ident_2 = NamespaceIdent::from_strs(vec!["a", "b"]).unwrap(); - let namespace_ident_3 = NamespaceIdent::new("c".into()); - create_namespaces(&catalog, &vec![ - &namespace_ident_1, - &namespace_ident_2, - &namespace_ident_3, - ]) - .await; - - assert_eq!( - to_set(catalog.list_namespaces(None).await.unwrap()), - to_set(vec![namespace_ident_1.clone(), namespace_ident_3]) - ); - - assert_eq!( - catalog - .list_namespaces(Some(&namespace_ident_1)) - .await - .unwrap(), - vec![NamespaceIdent::from_strs(vec!["a", "b"]).unwrap()] - ); - } - - #[tokio::test] - async fn test_list_namespaces_returns_multiple_namespaces_under_parent() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident_1 = NamespaceIdent::new("a".to_string()); - let namespace_ident_2 = NamespaceIdent::from_strs(vec!["a", "a"]).unwrap(); - let namespace_ident_3 = NamespaceIdent::from_strs(vec!["a", "b"]).unwrap(); - let namespace_ident_4 = NamespaceIdent::from_strs(vec!["a", "c"]).unwrap(); - let namespace_ident_5 = NamespaceIdent::new("b".into()); - create_namespaces(&catalog, &vec![ - &namespace_ident_1, - &namespace_ident_2, - &namespace_ident_3, - &namespace_ident_4, - &namespace_ident_5, - ]) - .await; - - assert_eq!( - to_set( - catalog - .list_namespaces(Some(&namespace_ident_1)) - .await - .unwrap() - ), - to_set(vec![ - NamespaceIdent::from_strs(vec!["a", "a"]).unwrap(), - NamespaceIdent::from_strs(vec!["a", "b"]).unwrap(), - NamespaceIdent::from_strs(vec!["a", "c"]).unwrap(), - ]) - ); - } - - #[tokio::test] - async fn test_namespace_exists_returns_false() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - - assert!( - !catalog - .namespace_exists(&NamespaceIdent::new("b".into())) - .await - .unwrap() - ); - } - - #[tokio::test] - async fn test_namespace_exists_returns_true() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - - assert!(catalog.namespace_exists(&namespace_ident).await.unwrap()); - } - - #[tokio::test] - async fn test_create_namespace_with_properties() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident = NamespaceIdent::new("abc".into()); - - let mut properties = default_properties(); - properties.insert("k".into(), "v".into()); - - assert_eq!( - catalog - .create_namespace(&namespace_ident, properties.clone()) - .await - .unwrap(), - Namespace::with_properties(namespace_ident.clone(), properties.clone()) - ); - - assert_eq!( - catalog.get_namespace(&namespace_ident).await.unwrap(), - Namespace::with_properties(namespace_ident, properties) - ); - } - - #[tokio::test] - async fn test_create_namespace_throws_error_if_namespace_already_exists() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - - assert_eq!( - catalog - .create_namespace(&namespace_ident, HashMap::new()) - .await - .unwrap_err() - .to_string(), - format!( - "Unexpected => Namespace {:?} already exists", - &namespace_ident - ) - ); - - assert_eq!( - catalog.get_namespace(&namespace_ident).await.unwrap(), - Namespace::with_properties(namespace_ident, default_properties()) - ); - } - #[tokio::test] async fn test_create_nested_namespace() { let warehouse_loc = temp_path(); @@ -1555,57 +1305,6 @@ mod tests { ); } - #[tokio::test] - async fn test_update_namespace_noop() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - - catalog - .update_namespace(&namespace_ident, HashMap::new()) - .await - .unwrap(); - - assert_eq!( - *catalog - .get_namespace(&namespace_ident) - .await - .unwrap() - .properties(), - HashMap::from_iter([("exists".to_string(), "true".to_string())]) - ) - } - - #[tokio::test] - async fn test_update_namespace() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - - let mut props = HashMap::from_iter([ - ("prop1".to_string(), "val1".to_string()), - ("prop2".into(), "val2".into()), - ]); - - catalog - .update_namespace(&namespace_ident, props.clone()) - .await - .unwrap(); - - props.insert("exists".into(), "true".into()); - - assert_eq!( - *catalog - .get_namespace(&namespace_ident) - .await - .unwrap() - .properties(), - props - ) - } - #[tokio::test] async fn test_update_nested_namespace() { let warehouse_loc = temp_path(); @@ -1635,28 +1334,6 @@ mod tests { ) } - #[tokio::test] - async fn test_update_namespace_errors_if_namespace_doesnt_exist() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident = NamespaceIdent::new("a".into()); - - let props = HashMap::from_iter([ - ("prop1".to_string(), "val1".to_string()), - ("prop2".into(), "val2".into()), - ]); - - let err = catalog - .update_namespace(&namespace_ident, props) - .await - .unwrap_err(); - - assert_eq!( - err.message(), - format!("No such namespace: {namespace_ident:?}") - ); - } - #[tokio::test] async fn test_update_namespace_errors_if_nested_namespace_doesnt_exist() { let warehouse_loc = temp_path(); @@ -1679,18 +1356,6 @@ mod tests { ); } - #[tokio::test] - async fn test_drop_namespace() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident = NamespaceIdent::new("abc".into()); - create_namespace(&catalog, &namespace_ident).await; - - catalog.drop_namespace(&namespace_ident).await.unwrap(); - - assert!(!catalog.namespace_exists(&namespace_ident).await.unwrap()) - } - #[tokio::test] async fn test_drop_nested_namespace() { let warehouse_loc = temp_path(); @@ -1747,22 +1412,6 @@ mod tests { assert!(catalog.namespace_exists(&namespace_ident_a).await.unwrap()); } - #[tokio::test] - async fn test_drop_namespace_throws_error_if_namespace_doesnt_exist() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - - let non_existent_namespace_ident = NamespaceIdent::new("abc".into()); - assert_eq!( - catalog - .drop_namespace(&non_existent_namespace_ident) - .await - .unwrap_err() - .to_string(), - format!("Unexpected => No such namespace: {non_existent_namespace_ident:?}") - ) - } - #[tokio::test] async fn test_drop_namespace_throws_error_if_nested_namespace_doesnt_exist() { let warehouse_loc = temp_path(); @@ -1801,340 +1450,6 @@ mod tests { ); } - #[tokio::test] - async fn test_list_tables_returns_empty_vector() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - - assert_eq!(catalog.list_tables(&namespace_ident).await.unwrap(), vec![]); - } - - #[tokio::test] - async fn test_list_tables_throws_error_if_namespace_doesnt_exist() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - - let non_existent_namespace_ident = NamespaceIdent::new("n1".into()); - - assert_eq!( - catalog - .list_tables(&non_existent_namespace_ident) - .await - .unwrap_err() - .to_string(), - format!("Unexpected => No such namespace: {non_existent_namespace_ident:?}"), - ); - } - - #[tokio::test] - async fn test_create_table_with_location() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc.clone()).await; - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - - let table_name = "abc"; - let location = warehouse_loc.clone(); - let table_creation = TableCreation::builder() - .name(table_name.into()) - .location(location.clone()) - .schema(simple_table_schema()) - .build(); - - let expected_table_ident = TableIdent::new(namespace_ident.clone(), table_name.into()); - - assert_table_eq( - &catalog - .create_table(&namespace_ident, table_creation) - .await - .unwrap(), - &expected_table_ident, - &simple_table_schema(), - ); - - let table = catalog.load_table(&expected_table_ident).await.unwrap(); - - assert_table_eq(&table, &expected_table_ident, &simple_table_schema()); - - assert!( - table - .metadata_location() - .unwrap() - .to_string() - .starts_with(&location) - ) - } - - #[tokio::test] - async fn test_create_table_falls_back_to_namespace_location_if_table_location_is_missing() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - - let namespace_ident = NamespaceIdent::new("a".into()); - let mut namespace_properties = HashMap::new(); - let namespace_location = temp_path(); - namespace_properties.insert( - NAMESPACE_LOCATION_PROPERTY_KEY.to_string(), - namespace_location.to_string(), - ); - catalog - .create_namespace(&namespace_ident, namespace_properties) - .await - .unwrap(); - - let table_name = "tbl1"; - let expected_table_ident = TableIdent::new(namespace_ident.clone(), table_name.into()); - let expected_table_metadata_location_regex = - format!("^{namespace_location}/tbl1/metadata/00000-{UUID_REGEX_STR}.metadata.json$",); - - let table = catalog - .create_table( - &namespace_ident, - TableCreation::builder() - .name(table_name.into()) - .schema(simple_table_schema()) - // no location specified for table - .build(), - ) - .await - .unwrap(); - assert_table_eq(&table, &expected_table_ident, &simple_table_schema()); - assert_table_metadata_location_matches(&table, &expected_table_metadata_location_regex); - - let table = catalog.load_table(&expected_table_ident).await.unwrap(); - assert_table_eq(&table, &expected_table_ident, &simple_table_schema()); - assert_table_metadata_location_matches(&table, &expected_table_metadata_location_regex); - } - - #[tokio::test] - async fn test_create_table_in_nested_namespace_falls_back_to_nested_namespace_location_if_table_location_is_missing() - { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - - let namespace_ident = NamespaceIdent::new("a".into()); - let mut namespace_properties = HashMap::new(); - let namespace_location = temp_path(); - namespace_properties.insert( - NAMESPACE_LOCATION_PROPERTY_KEY.to_string(), - namespace_location.to_string(), - ); - catalog - .create_namespace(&namespace_ident, namespace_properties) - .await - .unwrap(); - - let nested_namespace_ident = NamespaceIdent::from_strs(vec!["a", "b"]).unwrap(); - let mut nested_namespace_properties = HashMap::new(); - let nested_namespace_location = temp_path(); - nested_namespace_properties.insert( - NAMESPACE_LOCATION_PROPERTY_KEY.to_string(), - nested_namespace_location.to_string(), - ); - catalog - .create_namespace(&nested_namespace_ident, nested_namespace_properties) - .await - .unwrap(); - - let table_name = "tbl1"; - let expected_table_ident = - TableIdent::new(nested_namespace_ident.clone(), table_name.into()); - let expected_table_metadata_location_regex = format!( - "^{nested_namespace_location}/tbl1/metadata/00000-{UUID_REGEX_STR}.metadata.json$", - ); - - let table = catalog - .create_table( - &nested_namespace_ident, - TableCreation::builder() - .name(table_name.into()) - .schema(simple_table_schema()) - // no location specified for table - .build(), - ) - .await - .unwrap(); - assert_table_eq(&table, &expected_table_ident, &simple_table_schema()); - assert_table_metadata_location_matches(&table, &expected_table_metadata_location_regex); - - let table = catalog.load_table(&expected_table_ident).await.unwrap(); - assert_table_eq(&table, &expected_table_ident, &simple_table_schema()); - assert_table_metadata_location_matches(&table, &expected_table_metadata_location_regex); - } - - #[tokio::test] - async fn test_create_table_falls_back_to_warehouse_location_if_both_table_location_and_namespace_location_are_missing() - { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc.clone()).await; - - let namespace_ident = NamespaceIdent::new("a".into()); - // note: no location specified in namespace_properties - let namespace_properties = HashMap::new(); - catalog - .create_namespace(&namespace_ident, namespace_properties) - .await - .unwrap(); - - let table_name = "tbl1"; - let expected_table_ident = TableIdent::new(namespace_ident.clone(), table_name.into()); - let expected_table_metadata_location_regex = - format!("^{warehouse_loc}/a/tbl1/metadata/00000-{UUID_REGEX_STR}.metadata.json$"); - - let table = catalog - .create_table( - &namespace_ident, - TableCreation::builder() - .name(table_name.into()) - .schema(simple_table_schema()) - // no location specified for table - .build(), - ) - .await - .unwrap(); - assert_table_eq(&table, &expected_table_ident, &simple_table_schema()); - assert_table_metadata_location_matches(&table, &expected_table_metadata_location_regex); - - let table = catalog.load_table(&expected_table_ident).await.unwrap(); - assert_table_eq(&table, &expected_table_ident, &simple_table_schema()); - assert_table_metadata_location_matches(&table, &expected_table_metadata_location_regex); - } - - #[tokio::test] - async fn test_create_table_in_nested_namespace_falls_back_to_warehouse_location_if_both_table_location_and_namespace_location_are_missing() - { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc.clone()).await; - - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - - let nested_namespace_ident = NamespaceIdent::from_strs(vec!["a", "b"]).unwrap(); - create_namespace(&catalog, &nested_namespace_ident).await; - - let table_name = "tbl1"; - let expected_table_ident = - TableIdent::new(nested_namespace_ident.clone(), table_name.into()); - let expected_table_metadata_location_regex = - format!("^{warehouse_loc}/a/b/tbl1/metadata/00000-{UUID_REGEX_STR}.metadata.json$"); - - let table = catalog - .create_table( - &nested_namespace_ident, - TableCreation::builder() - .name(table_name.into()) - .schema(simple_table_schema()) - // no location specified for table - .build(), - ) - .await - .unwrap(); - assert_table_eq(&table, &expected_table_ident, &simple_table_schema()); - assert_table_metadata_location_matches(&table, &expected_table_metadata_location_regex); - - let table = catalog.load_table(&expected_table_ident).await.unwrap(); - assert_table_eq(&table, &expected_table_ident, &simple_table_schema()); - assert_table_metadata_location_matches(&table, &expected_table_metadata_location_regex); - } - - #[tokio::test] - async fn test_create_table_throws_error_if_table_with_same_name_already_exists() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc.clone()).await; - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - let table_name = "tbl1"; - let table_ident = TableIdent::new(namespace_ident.clone(), table_name.into()); - create_table(&catalog, &table_ident).await; - - let tmp_dir = TempDir::new().unwrap(); - let location = tmp_dir.path().to_str().unwrap().to_string(); - - assert_eq!( - catalog - .create_table( - &namespace_ident, - TableCreation::builder() - .name(table_name.into()) - .schema(simple_table_schema()) - .location(location) - .build() - ) - .await - .unwrap_err() - .to_string(), - format!("Unexpected => Table {:?} already exists.", &table_ident) - ); - } - - #[tokio::test] - async fn test_rename_table_in_same_namespace() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident = NamespaceIdent::new("n1".into()); - create_namespace(&catalog, &namespace_ident).await; - let src_table_ident = TableIdent::new(namespace_ident.clone(), "tbl1".into()); - let dst_table_ident = TableIdent::new(namespace_ident.clone(), "tbl2".into()); - create_table(&catalog, &src_table_ident).await; - - catalog - .rename_table(&src_table_ident, &dst_table_ident) - .await - .unwrap(); - - assert_eq!(catalog.list_tables(&namespace_ident).await.unwrap(), vec![ - dst_table_ident - ],); - } - - #[tokio::test] - async fn test_rename_table_across_namespaces() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let src_namespace_ident = NamespaceIdent::new("a".into()); - let dst_namespace_ident = NamespaceIdent::new("b".into()); - create_namespaces(&catalog, &vec![&src_namespace_ident, &dst_namespace_ident]).await; - let src_table_ident = TableIdent::new(src_namespace_ident.clone(), "tbl1".into()); - let dst_table_ident = TableIdent::new(dst_namespace_ident.clone(), "tbl2".into()); - create_table(&catalog, &src_table_ident).await; - - catalog - .rename_table(&src_table_ident, &dst_table_ident) - .await - .unwrap(); - - assert_eq!( - catalog.list_tables(&src_namespace_ident).await.unwrap(), - vec![], - ); - - assert_eq!( - catalog.list_tables(&dst_namespace_ident).await.unwrap(), - vec![dst_table_ident], - ); - } - - #[tokio::test] - async fn test_rename_table_src_table_is_same_as_dst_table() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident = NamespaceIdent::new("n1".into()); - create_namespace(&catalog, &namespace_ident).await; - let table_ident = TableIdent::new(namespace_ident.clone(), "tbl".into()); - create_table(&catalog, &table_ident).await; - - catalog - .rename_table(&table_ident, &table_ident) - .await - .unwrap(); - - assert_eq!(catalog.list_tables(&namespace_ident).await.unwrap(), vec![ - table_ident - ],); - } - #[tokio::test] async fn test_rename_table_across_nested_namespaces() { let warehouse_loc = temp_path(); @@ -2162,232 +1477,4 @@ mod tests { assert!(catalog.table_exists(&dst_table_ident).await.unwrap()); } - - #[tokio::test] - async fn test_rename_table_throws_error_if_dst_namespace_doesnt_exist() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let src_namespace_ident = NamespaceIdent::new("n1".into()); - let src_table_ident = TableIdent::new(src_namespace_ident.clone(), "tbl1".into()); - create_namespace(&catalog, &src_namespace_ident).await; - create_table(&catalog, &src_table_ident).await; - - let non_existent_dst_namespace_ident = NamespaceIdent::new("n2".into()); - let dst_table_ident = - TableIdent::new(non_existent_dst_namespace_ident.clone(), "tbl1".into()); - assert_eq!( - catalog - .rename_table(&src_table_ident, &dst_table_ident) - .await - .unwrap_err() - .to_string(), - format!("Unexpected => No such namespace: {non_existent_dst_namespace_ident:?}"), - ); - } - - #[tokio::test] - async fn test_rename_table_throws_error_if_src_table_doesnt_exist() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident = NamespaceIdent::new("n1".into()); - create_namespace(&catalog, &namespace_ident).await; - let src_table_ident = TableIdent::new(namespace_ident.clone(), "tbl1".into()); - let dst_table_ident = TableIdent::new(namespace_ident.clone(), "tbl2".into()); - - assert_eq!( - catalog - .rename_table(&src_table_ident, &dst_table_ident) - .await - .unwrap_err() - .to_string(), - format!("Unexpected => No such table: {src_table_ident:?}"), - ); - } - - #[tokio::test] - async fn test_rename_table_throws_error_if_dst_table_already_exists() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - let namespace_ident = NamespaceIdent::new("n1".into()); - create_namespace(&catalog, &namespace_ident).await; - let src_table_ident = TableIdent::new(namespace_ident.clone(), "tbl1".into()); - let dst_table_ident = TableIdent::new(namespace_ident.clone(), "tbl2".into()); - create_tables(&catalog, vec![&src_table_ident, &dst_table_ident]).await; - - assert_eq!( - catalog - .rename_table(&src_table_ident, &dst_table_ident) - .await - .unwrap_err() - .to_string(), - format!("Unexpected => Table {:?} already exists.", &dst_table_ident), - ); - } - - #[tokio::test] - async fn test_drop_table_throws_error_if_table_not_exist() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc.clone()).await; - let namespace_ident = NamespaceIdent::new("a".into()); - let table_name = "tbl1"; - let table_ident = TableIdent::new(namespace_ident.clone(), table_name.into()); - create_namespace(&catalog, &namespace_ident).await; - - let err = catalog - .drop_table(&table_ident) - .await - .unwrap_err() - .to_string(); - assert_eq!( - err, - "Unexpected => No such table: TableIdent { namespace: NamespaceIdent([\"a\"]), name: \"tbl1\" }" - ); - } - - #[tokio::test] - async fn test_drop_table() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc.clone()).await; - let namespace_ident = NamespaceIdent::new("a".into()); - let table_name = "tbl1"; - let table_ident = TableIdent::new(namespace_ident.clone(), table_name.into()); - create_namespace(&catalog, &namespace_ident).await; - - let location = warehouse_loc.clone(); - let table_creation = TableCreation::builder() - .name(table_name.into()) - .location(location.clone()) - .schema(simple_table_schema()) - .build(); - - catalog - .create_table(&namespace_ident, table_creation) - .await - .unwrap(); - - let table = catalog.load_table(&table_ident).await.unwrap(); - assert_table_eq(&table, &table_ident, &simple_table_schema()); - - catalog.drop_table(&table_ident).await.unwrap(); - let err = catalog - .load_table(&table_ident) - .await - .unwrap_err() - .to_string(); - assert_eq!( - err, - "Unexpected => No such table: TableIdent { namespace: NamespaceIdent([\"a\"]), name: \"tbl1\" }" - ); - } - - #[tokio::test] - async fn test_register_table_throws_error_if_table_with_same_name_already_exists() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc.clone()).await; - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - let table_name = "tbl1"; - let table_ident = TableIdent::new(namespace_ident.clone(), table_name.into()); - create_table(&catalog, &table_ident).await; - - assert_eq!( - catalog - .register_table(&table_ident, warehouse_loc) - .await - .unwrap_err() - .to_string(), - format!("Unexpected => Table {:?} already exists.", &table_ident) - ); - } - - #[tokio::test] - async fn test_register_table() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc.clone()).await; - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - - let table_name = "abc"; - let location = warehouse_loc.clone(); - let table_creation = TableCreation::builder() - .name(table_name.into()) - .location(location.clone()) - .schema(simple_table_schema()) - .build(); - - let table_ident = TableIdent::new(namespace_ident.clone(), table_name.into()); - let expected_table = catalog - .create_table(&namespace_ident, table_creation) - .await - .unwrap(); - - let metadata_location = expected_table - .metadata_location() - .expect("Expected metadata location to be set") - .to_string(); - - assert_table_eq(&expected_table, &table_ident, &simple_table_schema()); - - let _ = catalog.drop_table(&table_ident).await; - - let table = catalog - .register_table(&table_ident, metadata_location.clone()) - .await - .unwrap(); - - assert_eq!(table.identifier(), expected_table.identifier()); - assert_eq!(table.metadata_location(), Some(metadata_location.as_str())); - } - - #[tokio::test] - async fn test_update_table() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc).await; - - // Create a test namespace and table - let namespace_ident = NamespaceIdent::new("ns1".into()); - create_namespace(&catalog, &namespace_ident).await; - let table_ident = TableIdent::new(namespace_ident.clone(), "tbl1".into()); - create_table(&catalog, &table_ident).await; - - let table = catalog.load_table(&table_ident).await.unwrap(); - - // Store the original metadata location for comparison - let original_metadata_location = table.metadata_location().unwrap().to_string(); - - // Create a transaction to update the table - let tx = Transaction::new(&table); - let tx = tx - .update_table_properties() - .set("test_property".to_string(), "test_value".to_string()) - .apply(tx) - .unwrap(); - - // Commit the transaction to the catalog - let updated_table = tx.commit(&catalog).await.unwrap(); - - // Verify the update was successful - assert_eq!( - updated_table.metadata().properties().get("test_property"), - Some(&"test_value".to_string()) - ); - // Verify the metadata location has been updated - assert_ne!( - updated_table.metadata_location().unwrap(), - original_metadata_location.as_str() - ); - - // Load the table again from the catalog to verify changes were persisted - let reloaded = catalog.load_table(&table_ident).await.unwrap(); - - // Verify the reloaded table matches the updated table - assert_eq!( - reloaded.metadata().properties().get("test_property"), - Some(&"test_value".to_string()) - ); - assert_eq!( - reloaded.metadata_location(), - updated_table.metadata_location() - ); - } } From e5ae9625c225fdd6ea62962a49e3f0a97956c002 Mon Sep 17 00:00:00 2001 From: Ray Liu Date: Wed, 4 Feb 2026 11:30:21 +0000 Subject: [PATCH 2/5] Fix build break --- crates/catalog/loader/tests/catalog_suite.rs | 38 ++++++++++ crates/catalog/sql/src/catalog.rs | 77 +++++++++++++++----- 2 files changed, 98 insertions(+), 17 deletions(-) diff --git a/crates/catalog/loader/tests/catalog_suite.rs b/crates/catalog/loader/tests/catalog_suite.rs index 873b034745..9e80442141 100644 --- a/crates/catalog/loader/tests/catalog_suite.rs +++ b/crates/catalog/loader/tests/catalog_suite.rs @@ -493,6 +493,44 @@ async fn test_catalog_namespace_listing_with_parent(#[case] kind: CatalogKind) - Ok(()) } +// Common behavior: listing top-level namespaces includes created namespaces. +#[rstest] +#[tokio::test] +#[case::rest(CatalogKind::Rest)] +#[case::glue(CatalogKind::Glue)] +#[case::hms(CatalogKind::Hms)] +#[case::sql(CatalogKind::Sql)] +#[case::s3tables(CatalogKind::S3Tables)] +#[case::memory(CatalogKind::Memory)] +async fn test_catalog_list_namespaces_contains_created(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let ns_one = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_list_namespaces_contains_created", + harness.label, + "one" + )); + let ns_two = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_list_namespaces_contains_created", + harness.label, + "two" + )); + + cleanup_namespace_dyn(catalog.as_ref(), &ns_one).await; + cleanup_namespace_dyn(catalog.as_ref(), &ns_two).await; + + catalog.create_namespace(&ns_one, HashMap::new()).await?; + catalog.create_namespace(&ns_two, HashMap::new()).await?; + + let namespaces = catalog.list_namespaces(None).await?; + assert!(namespaces.contains(&ns_one)); + assert!(namespaces.contains(&ns_two)); + + Ok(()) +} + // Common behavior: creating an existing namespace should error. #[rstest] #[tokio::test] diff --git a/crates/catalog/sql/src/catalog.rs b/crates/catalog/sql/src/catalog.rs index 27c345dcaf..1fb21e5fde 100644 --- a/crates/catalog/sql/src/catalog.rs +++ b/crates/catalog/sql/src/catalog.rs @@ -974,23 +974,35 @@ impl Catalog for SqlCatalog { #[cfg(test)] mod tests { - use std::collections::HashMap; + use std::collections::{HashMap, HashSet}; + use std::hash::Hash; - use iceberg::spec::{NestedField, PrimitiveType, Schema, Type}; + use iceberg::spec::{NestedField, PartitionSpec, PrimitiveType, Schema, SortOrder, Type}; + use iceberg::table::Table; + use iceberg::transaction::{ApplyTransactionAction, Transaction}; use iceberg::{Catalog, CatalogBuilder, Namespace, NamespaceIdent, TableCreation, TableIdent}; + use itertools::Itertools; + use regex::Regex; use sqlx::migrate::MigrateDatabase; use tempfile::TempDir; use crate::catalog::{ - SQL_CATALOG_PROP_BIND_STYLE, SQL_CATALOG_PROP_URI, SQL_CATALOG_PROP_WAREHOUSE, + NAMESPACE_LOCATION_PROPERTY_KEY, SQL_CATALOG_PROP_BIND_STYLE, SQL_CATALOG_PROP_URI, + SQL_CATALOG_PROP_WAREHOUSE, }; use crate::{SqlBindStyle, SqlCatalogBuilder}; + const UUID_REGEX_STR: &str = "[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}"; + fn temp_path() -> String { let temp_dir = TempDir::new().unwrap(); temp_dir.path().to_str().unwrap().to_string() } + fn to_set(vec: Vec) -> HashSet { + HashSet::from_iter(vec) + } + fn default_properties() -> HashMap { HashMap::from([("exists".to_string(), "true".to_string())]) } @@ -1064,6 +1076,51 @@ mod tests { } } + fn assert_table_eq(table: &Table, expected_table_ident: &TableIdent, expected_schema: &Schema) { + assert_eq!(table.identifier(), expected_table_ident); + + let metadata = table.metadata(); + + assert_eq!(metadata.current_schema().as_ref(), expected_schema); + + let expected_partition_spec = PartitionSpec::builder(expected_schema.clone()) + .with_spec_id(0) + .build() + .unwrap(); + + assert_eq!( + metadata + .partition_specs_iter() + .map(|p| p.as_ref()) + .collect_vec(), + vec![&expected_partition_spec] + ); + + let expected_sorted_order = SortOrder::builder() + .with_order_id(0) + .with_fields(vec![]) + .build(expected_schema) + .unwrap(); + + assert_eq!( + metadata + .sort_orders_iter() + .map(|s| s.as_ref()) + .collect_vec(), + vec![&expected_sorted_order] + ); + + assert_eq!(metadata.properties(), &HashMap::new()); + + assert!(!table.readonly()); + } + + fn assert_table_metadata_location_matches(table: &Table, regex_str: &str) { + let actual = table.metadata_location().unwrap().to_string(); + let regex = Regex::new(regex_str).unwrap(); + assert!(regex.is_match(&actual)) + } + #[tokio::test] async fn test_initialized() { let warehouse_loc = temp_path(); @@ -1294,20 +1351,6 @@ mod tests { assert_eq!(catalog2.list_namespaces(None).await.unwrap(), vec![]); } - #[tokio::test] - async fn test_list_namespaces_returns_multiple_namespaces() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc, Some("iceberg")).await; - let namespace_ident_1 = NamespaceIdent::new("a".into()); - let namespace_ident_2 = NamespaceIdent::new("b".into()); - create_namespaces(&catalog, &vec![&namespace_ident_1, &namespace_ident_2]).await; - - assert_eq!( - to_set(catalog.list_namespaces(None).await.unwrap()), - to_set(vec![namespace_ident_1, namespace_ident_2]) - ); - } - #[tokio::test] async fn test_list_namespaces_returns_only_top_level_namespaces() { let warehouse_loc = temp_path(); From 0e7d22643d625395a571c627586d04504e772606 Mon Sep 17 00:00:00 2001 From: Ray Liu Date: Tue, 10 Feb 2026 03:33:02 +0000 Subject: [PATCH 3/5] Split catalog suite --- crates/catalog/hms/src/catalog.rs | 40 + crates/catalog/loader/tests/catalog_suite.rs | 1048 ----------------- crates/catalog/loader/tests/common/mod.rs | 330 ++++++ .../catalog/loader/tests/namespace_suite.rs | 358 ++++++ .../loader/tests/table_register_suite.rs | 167 +++ .../loader/tests/table_rename_suite.rs | 163 +++ crates/catalog/loader/tests/table_suite.rs | 276 +++++ crates/iceberg/src/catalog/memory/catalog.rs | 4 +- 8 files changed, 1336 insertions(+), 1050 deletions(-) delete mode 100644 crates/catalog/loader/tests/catalog_suite.rs create mode 100644 crates/catalog/loader/tests/common/mod.rs create mode 100644 crates/catalog/loader/tests/namespace_suite.rs create mode 100644 crates/catalog/loader/tests/table_register_suite.rs create mode 100644 crates/catalog/loader/tests/table_rename_suite.rs create mode 100644 crates/catalog/loader/tests/table_suite.rs diff --git a/crates/catalog/hms/src/catalog.rs b/crates/catalog/hms/src/catalog.rs index b7d192210b..f5909f306a 100644 --- a/crates/catalog/hms/src/catalog.rs +++ b/crates/catalog/hms/src/catalog.rs @@ -258,6 +258,12 @@ impl Catalog for HmsCatalog { namespace: &NamespaceIdent, properties: HashMap, ) -> Result { + if self.namespace_exists(namespace).await? { + return Err(Error::new( + ErrorKind::DataInvalid, + "Namespace already exists", + )); + } let database = convert_to_database(namespace, &properties)?; self.client @@ -341,6 +347,12 @@ impl Catalog for HmsCatalog { namespace: &NamespaceIdent, properties: HashMap, ) -> Result<()> { + if !self.namespace_exists(namespace).await? { + return Err(Error::new( + ErrorKind::DataInvalid, + "Namespace does not exist", + )); + } let db = convert_to_database(namespace, &properties)?; let name = match &db.name { @@ -372,6 +384,13 @@ impl Catalog for HmsCatalog { async fn drop_namespace(&self, namespace: &NamespaceIdent) -> Result<()> { let name = validate_namespace(namespace)?; + if !self.namespace_exists(namespace).await? { + return Err(Error::new( + ErrorKind::DataInvalid, + "Namespace does not exist", + )); + } + self.client .0 .drop_database(name.into(), false, false) @@ -392,6 +411,12 @@ impl Catalog for HmsCatalog { /// querying the database. async fn list_tables(&self, namespace: &NamespaceIdent) -> Result> { let name = validate_namespace(namespace)?; + if !self.namespace_exists(namespace).await? { + return Err(Error::new( + ErrorKind::DataInvalid, + "Namespace does not exist", + )); + } let tables = self .client @@ -520,6 +545,15 @@ impl Catalog for HmsCatalog { /// - Any network or communication error occurs with the database backend. async fn drop_table(&self, table: &TableIdent) -> Result<()> { let db_name = validate_namespace(table.namespace())?; + if !self.namespace_exists(table.namespace()).await? { + return Err(Error::new( + ErrorKind::DataInvalid, + "Namespace does not exist", + )); + } + if !self.table_exists(table).await? { + return Err(Error::new(ErrorKind::DataInvalid, "Table does not exist")); + } self.client .0 @@ -568,6 +602,12 @@ impl Catalog for HmsCatalog { async fn rename_table(&self, src: &TableIdent, dest: &TableIdent) -> Result<()> { let src_dbname = validate_namespace(src.namespace())?; let dest_dbname = validate_namespace(dest.namespace())?; + if self.table_exists(dest).await? { + return Err(Error::new( + ErrorKind::DataInvalid, + "Destination table already exists", + )); + } let src_tbl_name = src.name.clone(); let dest_tbl_name = dest.name.clone(); diff --git a/crates/catalog/loader/tests/catalog_suite.rs b/crates/catalog/loader/tests/catalog_suite.rs deleted file mode 100644 index 9e80442141..0000000000 --- a/crates/catalog/loader/tests/catalog_suite.rs +++ /dev/null @@ -1,1048 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -//! Unified catalog integration tests. -//! -//! These assertions represent common catalog behavior that every implementation -//! should satisfy. Catalog-specific behaviors stay in their own crates. -//! -//! These tests assume Docker containers are started externally via `make docker-up`. - -use std::collections::HashMap; -use std::sync::Arc; - -use iceberg::io::{S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION, S3_SECRET_ACCESS_KEY}; -use iceberg::memory::{MEMORY_CATALOG_WAREHOUSE, MemoryCatalogBuilder}; -use iceberg::spec::{NestedField, PrimitiveType, Schema, Type}; -use iceberg::transaction::{ApplyTransactionAction, Transaction}; -use iceberg::{ - Catalog, CatalogBuilder, ErrorKind, NamespaceIdent, Result, TableCreation, TableIdent, -}; -use iceberg_catalog_glue::{ - AWS_ACCESS_KEY_ID, AWS_REGION_NAME, AWS_SECRET_ACCESS_KEY, GLUE_CATALOG_PROP_URI, - GLUE_CATALOG_PROP_WAREHOUSE, GlueCatalog, GlueCatalogBuilder, -}; -use iceberg_catalog_hms::{ - HMS_CATALOG_PROP_THRIFT_TRANSPORT, HMS_CATALOG_PROP_URI, HMS_CATALOG_PROP_WAREHOUSE, - HmsCatalog, HmsCatalogBuilder, THRIFT_TRANSPORT_BUFFERED, -}; -use iceberg_catalog_rest::{REST_CATALOG_PROP_URI, RestCatalog, RestCatalogBuilder}; -use iceberg_catalog_s3tables::{ - S3TABLES_CATALOG_PROP_ENDPOINT_URL, S3TABLES_CATALOG_PROP_TABLE_BUCKET_ARN, - S3TablesCatalogBuilder, -}; -use iceberg_catalog_sql::{ - SQL_CATALOG_PROP_BIND_STYLE, SQL_CATALOG_PROP_URI, SQL_CATALOG_PROP_WAREHOUSE, SqlBindStyle, - SqlCatalogBuilder, -}; -use iceberg_test_utils::{ - get_glue_endpoint, get_hms_endpoint, get_minio_endpoint, get_rest_catalog_endpoint, - normalize_test_name_with_parts, set_up, -}; -use rstest::rstest; -use sqlx::migrate::MigrateDatabase; -use tempfile::TempDir; -use tokio::time::sleep; - -#[derive(Debug, Clone, Copy)] -#[allow(dead_code)] -enum CatalogKind { - Rest, - Glue, - Hms, - Sql, - S3Tables, - Memory, -} - -struct CatalogHarness { - catalog: Arc, - label: &'static str, - _tempdirs: Vec, -} - -// Shared setup for each catalog implementation so the tests below exercise -// the same behavior against all backends. -async fn load_catalog(kind: CatalogKind) -> Option { - set_up(); - match kind { - CatalogKind::Rest => Some(CatalogHarness { - catalog: Arc::new(rest_catalog().await) as Arc, - label: "rest", - _tempdirs: Vec::new(), - }), - CatalogKind::Glue => Some(CatalogHarness { - catalog: Arc::new(glue_catalog().await) as Arc, - label: "glue", - _tempdirs: Vec::new(), - }), - CatalogKind::Hms => Some(CatalogHarness { - catalog: Arc::new(hms_catalog().await) as Arc, - label: "hms", - _tempdirs: Vec::new(), - }), - CatalogKind::Sql => { - let warehouse_dir = TempDir::new().unwrap(); - let db_dir = TempDir::new().unwrap(); - let db_path = db_dir.path().join("catalog.db"); - let db_uri = format!("sqlite:{}", db_path.to_str().unwrap()); - sqlx::Sqlite::create_database(&db_uri).await.unwrap(); - - let catalog = SqlCatalogBuilder::default() - .load( - "sql", - HashMap::from([ - (SQL_CATALOG_PROP_URI.to_string(), db_uri), - ( - SQL_CATALOG_PROP_WAREHOUSE.to_string(), - warehouse_dir.path().to_str().unwrap().to_string(), - ), - ( - SQL_CATALOG_PROP_BIND_STYLE.to_string(), - SqlBindStyle::QMark.to_string(), - ), - ]), - ) - .await - .unwrap(); - - Some(CatalogHarness { - catalog: Arc::new(catalog) as Arc, - label: "sql", - _tempdirs: vec![warehouse_dir, db_dir], - }) - } - CatalogKind::S3Tables => { - let table_bucket_arn = match std::env::var("TABLE_BUCKET_ARN").ok() { - Some(value) => value, - None => return None, - }; - - let mut props = HashMap::from([( - S3TABLES_CATALOG_PROP_TABLE_BUCKET_ARN.to_string(), - table_bucket_arn, - )]); - - if let Ok(endpoint_url) = std::env::var("S3TABLES_ENDPOINT_URL") { - props.insert(S3TABLES_CATALOG_PROP_ENDPOINT_URL.to_string(), endpoint_url); - } - - let catalog = S3TablesCatalogBuilder::default() - .load("s3tables", props) - .await - .unwrap(); - - Some(CatalogHarness { - catalog: Arc::new(catalog) as Arc, - label: "s3tables", - _tempdirs: Vec::new(), - }) - } - CatalogKind::Memory => { - let warehouse_dir = TempDir::new().unwrap(); - let props = HashMap::from([( - MEMORY_CATALOG_WAREHOUSE.to_string(), - warehouse_dir.path().to_str().unwrap().to_string(), - )]); - let catalog = MemoryCatalogBuilder::default() - .load("memory", props) - .await - .unwrap(); - - Some(CatalogHarness { - catalog: Arc::new(catalog) as Arc, - label: "memory", - _tempdirs: vec![warehouse_dir], - }) - } - } -} - -// Catalog-specific setup is intentionally isolated here so the test cases -// remain implementation-agnostic. -async fn rest_catalog() -> RestCatalog { - let rest_endpoint = get_rest_catalog_endpoint(); - - let client = reqwest::Client::new(); - let mut retries = 0; - while retries < 30 { - if client - .get(format!("{rest_endpoint}/v1/config")) - .send() - .await - .map(|resp| resp.status().is_success()) - .unwrap_or(false) - { - break; - } - sleep(std::time::Duration::from_millis(1000)).await; - retries += 1; - } - - RestCatalogBuilder::default() - .load( - "rest", - HashMap::from([(REST_CATALOG_PROP_URI.to_string(), rest_endpoint)]), - ) - .await - .unwrap() -} - -async fn glue_catalog() -> GlueCatalog { - let glue_endpoint = get_glue_endpoint(); - let minio_endpoint = get_minio_endpoint(); - - let props = HashMap::from([ - (AWS_ACCESS_KEY_ID.to_string(), "my_access_id".to_string()), - ( - AWS_SECRET_ACCESS_KEY.to_string(), - "my_secret_key".to_string(), - ), - (AWS_REGION_NAME.to_string(), "us-east-1".to_string()), - (S3_ENDPOINT.to_string(), minio_endpoint), - (S3_ACCESS_KEY_ID.to_string(), "admin".to_string()), - (S3_SECRET_ACCESS_KEY.to_string(), "password".to_string()), - (S3_REGION.to_string(), "us-east-1".to_string()), - ]); - - let file_io = iceberg::io::FileIO::from_path("s3a://") - .unwrap() - .with_props(props.clone()) - .build() - .unwrap(); - - let mut retries = 0; - while retries < 30 { - if file_io.exists("s3a://warehouse/").await.unwrap_or(false) { - break; - } - sleep(std::time::Duration::from_millis(1000)).await; - retries += 1; - } - - let mut glue_props = HashMap::from([ - (GLUE_CATALOG_PROP_URI.to_string(), glue_endpoint), - ( - GLUE_CATALOG_PROP_WAREHOUSE.to_string(), - "s3a://warehouse/hive".to_string(), - ), - ]); - glue_props.extend(props); - - GlueCatalogBuilder::default() - .load("glue", glue_props) - .await - .unwrap() -} - -async fn hms_catalog() -> HmsCatalog { - let hms_endpoint = get_hms_endpoint(); - let minio_endpoint = get_minio_endpoint(); - - let props = HashMap::from([ - (HMS_CATALOG_PROP_URI.to_string(), hms_endpoint), - ( - HMS_CATALOG_PROP_THRIFT_TRANSPORT.to_string(), - THRIFT_TRANSPORT_BUFFERED.to_string(), - ), - ( - HMS_CATALOG_PROP_WAREHOUSE.to_string(), - "s3a://warehouse/hive".to_string(), - ), - (S3_ENDPOINT.to_string(), minio_endpoint), - (S3_ACCESS_KEY_ID.to_string(), "admin".to_string()), - (S3_SECRET_ACCESS_KEY.to_string(), "password".to_string()), - (S3_REGION.to_string(), "us-east-1".to_string()), - ]); - - let file_io = iceberg::io::FileIO::from_path("s3a://") - .unwrap() - .with_props(props.clone()) - .build() - .unwrap(); - - let mut retries = 0; - while retries < 30 { - if file_io.exists("s3a://warehouse/").await.unwrap_or(false) { - break; - } - sleep(std::time::Duration::from_millis(1000)).await; - retries += 1; - } - - HmsCatalogBuilder::default() - .load("hms", props) - .await - .unwrap() -} - -// Common table schema used across the suite to validate shared behavior. -fn table_creation(name: impl ToString) -> TableCreation { - let schema = Schema::builder() - .with_schema_id(0) - .with_fields(vec![ - NestedField::required(1, "foo", Type::Primitive(PrimitiveType::Int)).into(), - NestedField::required(2, "bar", Type::Primitive(PrimitiveType::String)).into(), - ]) - .build() - .unwrap(); - - TableCreation::builder() - .name(name.to_string()) - .properties(HashMap::new()) - .schema(schema) - .build() -} - -fn assert_map_contains(expected: &HashMap, actual: &HashMap) { - for (key, value) in expected { - assert_eq!(actual.get(key), Some(value)); - } -} - -async fn cleanup_namespace_dyn(catalog: &dyn Catalog, namespace: &NamespaceIdent) { - if let Ok(tables) = catalog.list_tables(namespace).await { - for table in tables { - let _ = catalog.drop_table(&table).await; - } - } - let _ = catalog.drop_namespace(namespace).await; -} - -// Common behavior: querying a missing namespace should error. -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_namespace_missing_returns_error(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_namespace_missing_returns_error", - harness.label - )); - - cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - - assert!(catalog.get_namespace(&namespace).await.is_err()); - - Ok(()) -} - -// Common behavior: namespace lifecycle CRUD (with optional update support). -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_namespace_lifecycle(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_namespace_lifecycle", - harness.label - )); - - cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - - assert!(!catalog.namespace_exists(&namespace).await?); - - let props = HashMap::from([ - ("owner".to_string(), "rust".to_string()), - ("purpose".to_string(), "catalog_suite".to_string()), - ]); - let created = catalog.create_namespace(&namespace, props.clone()).await?; - assert_eq!(created.name(), &namespace); - assert_map_contains(&props, created.properties()); - - let fetched = catalog.get_namespace(&namespace).await?; - assert_eq!(fetched.name(), &namespace); - assert_map_contains(&props, fetched.properties()); - - let namespaces = catalog.list_namespaces(None).await?; - assert!(namespaces.contains(&namespace)); - - let updated_props = HashMap::from([("owner".to_string(), "updated".to_string())]); - match catalog - .update_namespace(&namespace, updated_props.clone()) - .await - { - Ok(()) => { - let updated = catalog.get_namespace(&namespace).await?; - assert_map_contains(&updated_props, updated.properties()); - } - Err(err) if err.kind() == ErrorKind::FeatureUnsupported => {} - Err(err) => return Err(err), - } - - catalog.drop_namespace(&namespace).await?; - assert!(!catalog.namespace_exists(&namespace).await?); - - Ok(()) -} - -// Common behavior: table lifecycle CRUD. -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_table_lifecycle(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_table_lifecycle", - harness.label - )); - - cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - catalog.create_namespace(&namespace, HashMap::new()).await?; - - let table_name = - normalize_test_name_with_parts!("catalog_table_lifecycle", harness.label, "table"); - let table = catalog - .create_table(&namespace, table_creation(table_name)) - .await?; - let ident = table.identifier().clone(); - - assert!(catalog.table_exists(&ident).await?); - let loaded = catalog.load_table(&ident).await?; - assert_eq!(loaded.identifier(), &ident); - - let tables = catalog.list_tables(&namespace).await?; - assert!(tables.contains(&ident)); - - let dest = TableIdent::new(ident.namespace.clone(), format!("{}_renamed", ident.name)); - catalog.rename_table(&ident, &dest).await?; - assert!(catalog.table_exists(&dest).await?); - assert!(!catalog.table_exists(&ident).await?); - - catalog.drop_table(&dest).await?; - assert!(!catalog.table_exists(&dest).await?); - - catalog.drop_namespace(&namespace).await?; - - Ok(()) -} - -// Common behavior: listing namespaces under a parent returns its children. -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_namespace_listing_with_parent(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let parent_name = - normalize_test_name_with_parts!("catalog_namespace_listing_with_parent", harness.label); - let parent = NamespaceIdent::new(parent_name.clone()); - let child1 = NamespaceIdent::from_strs([&parent_name, "child1"]).unwrap(); - let child2 = NamespaceIdent::from_strs([&parent_name, "child2"]).unwrap(); - - cleanup_namespace_dyn(catalog.as_ref(), &child1).await; - cleanup_namespace_dyn(catalog.as_ref(), &child2).await; - cleanup_namespace_dyn(catalog.as_ref(), &parent).await; - - catalog.create_namespace(&parent, HashMap::new()).await?; - catalog.create_namespace(&child1, HashMap::new()).await?; - catalog.create_namespace(&child2, HashMap::new()).await?; - - let top_level = catalog.list_namespaces(None).await?; - assert!(top_level.contains(&parent)); - - let children = catalog.list_namespaces(Some(&parent)).await?; - assert!(children.contains(&child1)); - assert!(children.contains(&child2)); - - Ok(()) -} - -// Common behavior: listing top-level namespaces includes created namespaces. -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_list_namespaces_contains_created(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let ns_one = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_list_namespaces_contains_created", - harness.label, - "one" - )); - let ns_two = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_list_namespaces_contains_created", - harness.label, - "two" - )); - - cleanup_namespace_dyn(catalog.as_ref(), &ns_one).await; - cleanup_namespace_dyn(catalog.as_ref(), &ns_two).await; - - catalog.create_namespace(&ns_one, HashMap::new()).await?; - catalog.create_namespace(&ns_two, HashMap::new()).await?; - - let namespaces = catalog.list_namespaces(None).await?; - assert!(namespaces.contains(&ns_one)); - assert!(namespaces.contains(&ns_two)); - - Ok(()) -} - -// Common behavior: creating an existing namespace should error. -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_create_namespace_duplicate_fails(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_create_namespace_duplicate_fails", - harness.label - )); - - cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - catalog.create_namespace(&namespace, HashMap::new()).await?; - - assert!( - catalog - .create_namespace(&namespace, HashMap::new()) - .await - .is_err() - ); - - Ok(()) -} - -// Common behavior: update on a missing namespace should error (or be unsupported). -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_update_namespace_missing_errors(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_update_namespace_missing_errors", - harness.label - )); - - cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - - match catalog - .update_namespace( - &namespace, - HashMap::from([("key".to_string(), "value".to_string())]), - ) - .await - { - Err(err) if err.kind() == ErrorKind::FeatureUnsupported => Ok(()), - Err(_) => Ok(()), - Ok(()) => Err(iceberg::Error::new( - ErrorKind::Unexpected, - "Expected update_namespace to fail for missing namespace", - )), - } -} - -// Common behavior: dropping a missing namespace should error. -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_drop_namespace_missing_errors(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_drop_namespace_missing_errors", - harness.label - )); - - cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - - assert!(catalog.drop_namespace(&namespace).await.is_err()); - - Ok(()) -} - -// Common behavior: listing tables for a missing namespace should error. -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_list_tables_missing_namespace_errors( - #[case] kind: CatalogKind, -) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_list_tables_missing_namespace_errors", - harness.label - )); - - cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - - assert!(catalog.list_tables(&namespace).await.is_err()); - - Ok(()) -} - -// Common behavior: listing tables in an empty namespace returns empty. -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_list_tables_empty_namespace(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_list_tables_empty_namespace", - harness.label - )); - - cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - catalog.create_namespace(&namespace, HashMap::new()).await?; - - let tables = catalog.list_tables(&namespace).await?; - assert!(tables.is_empty()); - - Ok(()) -} - -// Common behavior: created tables expose the requested schema. -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_create_table_schema(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_create_table_schema", - harness.label - )); - - cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - catalog.create_namespace(&namespace, HashMap::new()).await?; - - let table_name = - normalize_test_name_with_parts!("catalog_create_table_schema", harness.label, "table"); - let creation = table_creation(table_name); - let expected_schema = creation.schema.clone(); - - let table = catalog.create_table(&namespace, creation).await?; - assert_eq!(table.identifier().namespace, namespace); - assert_eq!(table.metadata().current_schema().as_ref(), &expected_schema); - - Ok(()) -} - -// Common behavior: updating table properties persists through the catalog. -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_update_table_properties(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_update_table_properties", - harness.label - )); - - cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - catalog.create_namespace(&namespace, HashMap::new()).await?; - - let table_name = - normalize_test_name_with_parts!("catalog_update_table_properties", harness.label, "table"); - let table = catalog - .create_table(&namespace, table_creation(table_name)) - .await?; - - let tx = Transaction::new(&table); - let tx = tx - .update_table_properties() - .set("test_property".to_string(), "test_value".to_string()) - .apply(tx)?; - let updated = tx.commit(catalog.as_ref()).await?; - - assert_eq!( - updated.metadata().properties().get("test_property"), - Some(&"test_value".to_string()) - ); - - Ok(()) -} - -// Common behavior: renaming across namespaces moves the table. -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_rename_table_across_namespaces(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let src_namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_rename_table_across_namespaces", - harness.label, - "src" - )); - let dst_namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_rename_table_across_namespaces", - harness.label, - "dst" - )); - - cleanup_namespace_dyn(catalog.as_ref(), &src_namespace).await; - cleanup_namespace_dyn(catalog.as_ref(), &dst_namespace).await; - catalog - .create_namespace(&src_namespace, HashMap::new()) - .await?; - catalog - .create_namespace(&dst_namespace, HashMap::new()) - .await?; - - let table = catalog - .create_table( - &src_namespace, - table_creation(normalize_test_name_with_parts!( - "catalog_rename_table_across_namespaces", - harness.label, - "table" - )), - ) - .await?; - let src_ident = table.identifier().clone(); - let dst_ident = TableIdent::new(dst_namespace.clone(), src_ident.name.clone()); - - match catalog.rename_table(&src_ident, &dst_ident).await { - Ok(()) => { - assert!(catalog.table_exists(&dst_ident).await?); - assert!(!catalog.table_exists(&src_ident).await?); - } - Err(err) if err.kind() == ErrorKind::FeatureUnsupported => return Ok(()), - Err(err) => return Err(err), - } - - Ok(()) -} - -// Common behavior: renaming a missing table should error. -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_rename_table_missing_source_errors(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_rename_table_missing_source_errors", - harness.label - )); - - cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - catalog.create_namespace(&namespace, HashMap::new()).await?; - - let src_ident = TableIdent::new(namespace.clone(), "missing".to_string()); - let dst_ident = TableIdent::new(namespace.clone(), "dest".to_string()); - - match catalog.rename_table(&src_ident, &dst_ident).await { - Err(err) if err.kind() == ErrorKind::FeatureUnsupported => Ok(()), - Err(_) => Ok(()), - Ok(()) => Err(iceberg::Error::new( - ErrorKind::Unexpected, - "Expected rename_table to fail for missing source table", - )), - } -} - -// Common behavior: renaming to an existing destination should error. -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_rename_table_dest_exists_errors(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_rename_table_dest_exists_errors", - harness.label - )); - - cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - catalog.create_namespace(&namespace, HashMap::new()).await?; - - let src = catalog - .create_table( - &namespace, - table_creation(normalize_test_name_with_parts!( - "catalog_rename_table_dest_exists_errors", - harness.label, - "src" - )), - ) - .await? - .identifier() - .clone(); - let dst = catalog - .create_table( - &namespace, - table_creation(normalize_test_name_with_parts!( - "catalog_rename_table_dest_exists_errors", - harness.label, - "dst" - )), - ) - .await? - .identifier() - .clone(); - - match catalog.rename_table(&src, &dst).await { - Err(err) if err.kind() == ErrorKind::FeatureUnsupported => Ok(()), - Err(_) => Ok(()), - Ok(()) => Err(iceberg::Error::new( - ErrorKind::Unexpected, - "Expected rename_table to fail for existing destination table", - )), - } -} - -// Common behavior: dropping a missing table should error. -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_drop_table_missing_errors(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_drop_table_missing_errors", - harness.label - )); - - cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - catalog.create_namespace(&namespace, HashMap::new()).await?; - - let table_ident = TableIdent::new(namespace.clone(), "missing".to_string()); - assert!(catalog.drop_table(&table_ident).await.is_err()); - - Ok(()) -} - -// Common behavior: register_table rehydrates a dropped table (if supported). -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_register_table_roundtrip(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_register_table_roundtrip", - harness.label - )); - - cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - catalog.create_namespace(&namespace, HashMap::new()).await?; - - let table = catalog - .create_table( - &namespace, - table_creation(normalize_test_name_with_parts!( - "catalog_register_table_roundtrip", - harness.label, - "table" - )), - ) - .await?; - let table_ident = table.identifier().clone(); - let metadata_location = table - .metadata_location() - .ok_or_else(|| iceberg::Error::new(ErrorKind::Unexpected, "Missing metadata location"))? - .to_string(); - - catalog.drop_table(&table_ident).await?; - - match catalog - .register_table(&table_ident, metadata_location.clone()) - .await - { - Ok(registered) => { - assert_eq!(registered.identifier(), &table_ident); - assert_eq!( - registered.metadata_location(), - Some(metadata_location.as_str()) - ); - } - Err(err) if err.kind() == ErrorKind::FeatureUnsupported => return Ok(()), - Err(err) => return Err(err), - } - - Ok(()) -} - -// Common behavior: registering a table with an existing name should error (if supported). -#[rstest] -#[tokio::test] -#[case::rest(CatalogKind::Rest)] -#[case::glue(CatalogKind::Glue)] -#[case::hms(CatalogKind::Hms)] -#[case::sql(CatalogKind::Sql)] -#[case::s3tables(CatalogKind::S3Tables)] -#[case::memory(CatalogKind::Memory)] -async fn test_catalog_register_table_conflict_errors(#[case] kind: CatalogKind) -> Result<()> { - let Some(harness) = load_catalog(kind).await else { - return Ok(()); - }; - let catalog = harness.catalog; - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( - "catalog_register_table_conflict_errors", - harness.label - )); - - cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - catalog.create_namespace(&namespace, HashMap::new()).await?; - - let table_ident = TableIdent::new( - namespace.clone(), - normalize_test_name_with_parts!( - "catalog_register_table_conflict_errors", - harness.label, - "table" - ), - ); - let table = catalog - .create_table(&namespace, table_creation(table_ident.name.clone())) - .await?; - let metadata_location = table - .metadata_location() - .ok_or_else(|| iceberg::Error::new(ErrorKind::Unexpected, "Missing metadata location"))? - .to_string(); - - match catalog - .register_table(&table_ident, metadata_location) - .await - { - Err(err) if err.kind() == ErrorKind::FeatureUnsupported => Ok(()), - Err(_) => Ok(()), - Ok(_) => Err(iceberg::Error::new( - ErrorKind::Unexpected, - "Expected register_table to fail for existing table", - )), - } -} diff --git a/crates/catalog/loader/tests/common/mod.rs b/crates/catalog/loader/tests/common/mod.rs new file mode 100644 index 0000000000..c36213b78a --- /dev/null +++ b/crates/catalog/loader/tests/common/mod.rs @@ -0,0 +1,330 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Shared helpers for catalog integration suites. + +#![allow(dead_code)] + +use std::collections::HashMap; +use std::fmt; +use std::sync::Arc; + +use iceberg::io::{S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION, S3_SECRET_ACCESS_KEY}; +use iceberg::memory::{MEMORY_CATALOG_WAREHOUSE, MemoryCatalogBuilder}; +use iceberg::spec::{NestedField, PrimitiveType, Schema, Type}; +use iceberg::{Catalog, CatalogBuilder, NamespaceIdent, TableCreation}; +use iceberg_catalog_glue::{ + AWS_ACCESS_KEY_ID, AWS_REGION_NAME, AWS_SECRET_ACCESS_KEY, GLUE_CATALOG_PROP_URI, + GLUE_CATALOG_PROP_WAREHOUSE, GlueCatalog, GlueCatalogBuilder, +}; +use iceberg_catalog_hms::{ + HMS_CATALOG_PROP_THRIFT_TRANSPORT, HMS_CATALOG_PROP_URI, HMS_CATALOG_PROP_WAREHOUSE, + HmsCatalog, HmsCatalogBuilder, THRIFT_TRANSPORT_BUFFERED, +}; +use iceberg_catalog_rest::{REST_CATALOG_PROP_URI, RestCatalog, RestCatalogBuilder}; +use iceberg_catalog_s3tables::{ + S3TABLES_CATALOG_PROP_ENDPOINT_URL, S3TABLES_CATALOG_PROP_TABLE_BUCKET_ARN, + S3TablesCatalogBuilder, +}; +use iceberg_catalog_sql::{ + SQL_CATALOG_PROP_BIND_STYLE, SQL_CATALOG_PROP_URI, SQL_CATALOG_PROP_WAREHOUSE, SqlBindStyle, + SqlCatalogBuilder, +}; +use iceberg_test_utils::{ + get_glue_endpoint, get_hms_endpoint, get_minio_endpoint, get_rest_catalog_endpoint, set_up, +}; +use sqlx::migrate::MigrateDatabase; +use tempfile::TempDir; +use tokio::time::sleep; + +#[derive(Debug, Clone, Copy)] +pub enum CatalogKind { + Rest, + Glue, + Hms, + Sql, + S3Tables, + Memory, +} + +impl fmt::Display for CatalogKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let name = match self { + CatalogKind::Rest => "rest_catalog", + CatalogKind::Glue => "glue_catalog", + CatalogKind::Hms => "hms_catalog", + CatalogKind::Sql => "sql_catalog", + CatalogKind::S3Tables => "s3tables_catalog", + CatalogKind::Memory => "memory_catalog", + }; + f.write_str(name) + } +} + +pub struct CatalogHarness { + pub catalog: Arc, + pub label: &'static str, + _tempdirs: Vec, +} + +// Shared setup for each catalog implementation so the suites exercise +// the same behavior against all backends. +pub async fn load_catalog(kind: CatalogKind) -> Option { + set_up(); + match kind { + CatalogKind::Rest => Some(CatalogHarness { + catalog: Arc::new(rest_catalog().await) as Arc, + label: "rest", + _tempdirs: Vec::new(), + }), + CatalogKind::Glue => Some(CatalogHarness { + catalog: Arc::new(glue_catalog().await) as Arc, + label: "glue", + _tempdirs: Vec::new(), + }), + CatalogKind::Hms => Some(CatalogHarness { + catalog: Arc::new(hms_catalog().await) as Arc, + label: "hms", + _tempdirs: Vec::new(), + }), + CatalogKind::Sql => { + let warehouse_dir = TempDir::new().unwrap(); + let db_dir = TempDir::new().unwrap(); + let db_path = db_dir.path().join("catalog.db"); + let db_uri = format!("sqlite:{}", db_path.to_str().unwrap()); + sqlx::Sqlite::create_database(&db_uri).await.unwrap(); + + let catalog = SqlCatalogBuilder::default() + .load( + "sql", + HashMap::from([ + (SQL_CATALOG_PROP_URI.to_string(), db_uri), + ( + SQL_CATALOG_PROP_WAREHOUSE.to_string(), + warehouse_dir.path().to_str().unwrap().to_string(), + ), + ( + SQL_CATALOG_PROP_BIND_STYLE.to_string(), + SqlBindStyle::QMark.to_string(), + ), + ]), + ) + .await + .unwrap(); + + Some(CatalogHarness { + catalog: Arc::new(catalog) as Arc, + label: "sql", + _tempdirs: vec![warehouse_dir, db_dir], + }) + } + CatalogKind::S3Tables => { + let table_bucket_arn = match std::env::var("TABLE_BUCKET_ARN").ok() { + Some(value) => value, + None => return None, + }; + + let mut props = HashMap::from([( + S3TABLES_CATALOG_PROP_TABLE_BUCKET_ARN.to_string(), + table_bucket_arn, + )]); + + if let Ok(endpoint_url) = std::env::var("S3TABLES_ENDPOINT_URL") { + props.insert(S3TABLES_CATALOG_PROP_ENDPOINT_URL.to_string(), endpoint_url); + } + + let catalog = S3TablesCatalogBuilder::default() + .load("s3tables", props) + .await + .unwrap(); + + Some(CatalogHarness { + catalog: Arc::new(catalog) as Arc, + label: "s3tables", + _tempdirs: Vec::new(), + }) + } + CatalogKind::Memory => { + let warehouse_dir = TempDir::new().unwrap(); + let props = HashMap::from([( + MEMORY_CATALOG_WAREHOUSE.to_string(), + warehouse_dir.path().to_str().unwrap().to_string(), + )]); + let catalog = MemoryCatalogBuilder::default() + .load("memory", props) + .await + .unwrap(); + + Some(CatalogHarness { + catalog: Arc::new(catalog) as Arc, + label: "memory", + _tempdirs: vec![warehouse_dir], + }) + } + } +} + +// Catalog-specific setup is intentionally isolated here so the suites +// remain implementation-agnostic. +async fn rest_catalog() -> RestCatalog { + let rest_endpoint = get_rest_catalog_endpoint(); + + let client = reqwest::Client::new(); + let mut retries = 0; + while retries < 30 { + if client + .get(format!("{rest_endpoint}/v1/config")) + .send() + .await + .map(|resp| resp.status().is_success()) + .unwrap_or(false) + { + break; + } + sleep(std::time::Duration::from_millis(1000)).await; + retries += 1; + } + + RestCatalogBuilder::default() + .load( + "rest", + HashMap::from([(REST_CATALOG_PROP_URI.to_string(), rest_endpoint)]), + ) + .await + .unwrap() +} + +async fn glue_catalog() -> GlueCatalog { + let glue_endpoint = get_glue_endpoint(); + let minio_endpoint = get_minio_endpoint(); + + let props = HashMap::from([ + (AWS_ACCESS_KEY_ID.to_string(), "my_access_id".to_string()), + ( + AWS_SECRET_ACCESS_KEY.to_string(), + "my_secret_key".to_string(), + ), + (AWS_REGION_NAME.to_string(), "us-east-1".to_string()), + (S3_ENDPOINT.to_string(), minio_endpoint), + (S3_ACCESS_KEY_ID.to_string(), "admin".to_string()), + (S3_SECRET_ACCESS_KEY.to_string(), "password".to_string()), + (S3_REGION.to_string(), "us-east-1".to_string()), + ]); + + let file_io = iceberg::io::FileIO::from_path("s3a://") + .unwrap() + .with_props(props.clone()) + .build() + .unwrap(); + + let mut retries = 0; + while retries < 30 { + if file_io.exists("s3a://warehouse/").await.unwrap_or(false) { + break; + } + sleep(std::time::Duration::from_millis(1000)).await; + retries += 1; + } + + let mut glue_props = HashMap::from([ + (GLUE_CATALOG_PROP_URI.to_string(), glue_endpoint), + ( + GLUE_CATALOG_PROP_WAREHOUSE.to_string(), + "s3a://warehouse/hive".to_string(), + ), + ]); + glue_props.extend(props); + + GlueCatalogBuilder::default() + .load("glue", glue_props) + .await + .unwrap() +} + +async fn hms_catalog() -> HmsCatalog { + let hms_endpoint = get_hms_endpoint(); + let minio_endpoint = get_minio_endpoint(); + + let props = HashMap::from([ + (HMS_CATALOG_PROP_URI.to_string(), hms_endpoint), + ( + HMS_CATALOG_PROP_THRIFT_TRANSPORT.to_string(), + THRIFT_TRANSPORT_BUFFERED.to_string(), + ), + ( + HMS_CATALOG_PROP_WAREHOUSE.to_string(), + "s3a://warehouse/hive".to_string(), + ), + (S3_ENDPOINT.to_string(), minio_endpoint), + (S3_ACCESS_KEY_ID.to_string(), "admin".to_string()), + (S3_SECRET_ACCESS_KEY.to_string(), "password".to_string()), + (S3_REGION.to_string(), "us-east-1".to_string()), + ]); + + let file_io = iceberg::io::FileIO::from_path("s3a://") + .unwrap() + .with_props(props.clone()) + .build() + .unwrap(); + + let mut retries = 0; + while retries < 30 { + if file_io.exists("s3a://warehouse/").await.unwrap_or(false) { + break; + } + sleep(std::time::Duration::from_millis(1000)).await; + retries += 1; + } + + HmsCatalogBuilder::default() + .load("hms", props) + .await + .unwrap() +} + +// Common table schema used across suites to validate shared behavior. +pub fn table_creation(name: impl ToString) -> TableCreation { + let schema = Schema::builder() + .with_schema_id(0) + .with_fields(vec![ + NestedField::required(1, "foo", Type::Primitive(PrimitiveType::Int)).into(), + NestedField::required(2, "bar", Type::Primitive(PrimitiveType::String)).into(), + ]) + .build() + .unwrap(); + + TableCreation::builder() + .name(name.to_string()) + .properties(HashMap::new()) + .schema(schema) + .build() +} + +pub fn assert_map_contains(expected: &HashMap, actual: &HashMap) { + for (key, value) in expected { + assert_eq!(actual.get(key), Some(value)); + } +} + +pub async fn cleanup_namespace_dyn(catalog: &dyn Catalog, namespace: &NamespaceIdent) { + if let Ok(tables) = catalog.list_tables(namespace).await { + for table in tables { + let _ = catalog.drop_table(&table).await; + } + } + let _ = catalog.drop_namespace(namespace).await; +} diff --git a/crates/catalog/loader/tests/namespace_suite.rs b/crates/catalog/loader/tests/namespace_suite.rs new file mode 100644 index 0000000000..e317643033 --- /dev/null +++ b/crates/catalog/loader/tests/namespace_suite.rs @@ -0,0 +1,358 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Common namespace behavior across catalogs. +//! +//! These tests assume Docker containers are started externally via `make docker-up`. + +mod common; + +use std::collections::HashMap; + +use common::{CatalogKind, assert_map_contains, cleanup_namespace_dyn, load_catalog}; +use iceberg::{ErrorKind, NamespaceIdent, Result}; +use iceberg_test_utils::normalize_test_name_with_parts; +use rstest::rstest; + +// Common behavior: querying a missing namespace should error. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::hms_catalog(CatalogKind::Hms)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_namespace_missing_returns_error(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_namespace_missing_returns_error", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + + assert!(catalog.get_namespace(&namespace).await.is_err()); + + Ok(()) +} + +// Common behavior: namespace lifecycle CRUD. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::hms_catalog(CatalogKind::Hms)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_namespace_lifecycle(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_namespace_lifecycle", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + + assert!(!catalog.namespace_exists(&namespace).await?); + + let props = HashMap::from([ + ("owner".to_string(), "rust".to_string()), + ("purpose".to_string(), "catalog_suite".to_string()), + ]); + let created = catalog.create_namespace(&namespace, props.clone()).await?; + assert_eq!(created.name(), &namespace); + assert_map_contains(&props, created.properties()); + + let fetched = catalog.get_namespace(&namespace).await?; + assert_eq!(fetched.name(), &namespace); + assert_map_contains(&props, fetched.properties()); + + let namespaces = catalog.list_namespaces(None).await?; + assert!(namespaces.contains(&namespace)); + + catalog.drop_namespace(&namespace).await?; + assert!(!catalog.namespace_exists(&namespace).await?); + + Ok(()) +} + +// Common behavior: update_namespace persists changes when supported. +#[rstest] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::hms_catalog(CatalogKind::Hms)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_update_namespace_supported(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_update_namespace_supported", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let updated_props = HashMap::from([("owner".to_string(), "updated".to_string())]); + catalog + .update_namespace(&namespace, updated_props.clone()) + .await?; + + let updated = catalog.get_namespace(&namespace).await?; + assert_map_contains(&updated_props, updated.properties()); + + Ok(()) +} + +// Common behavior: update_namespace returns FeatureUnsupported when not implemented. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[tokio::test] +async fn test_catalog_update_namespace_unsupported(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_update_namespace_unsupported", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let err = catalog + .update_namespace( + &namespace, + HashMap::from([("key".to_string(), "value".to_string())]), + ) + .await + .unwrap_err(); + assert_eq!(err.kind(), ErrorKind::FeatureUnsupported); + + Ok(()) +} + +// Common behavior: listing namespaces under a parent returns its children. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_namespace_listing_with_parent(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let parent_name = + normalize_test_name_with_parts!("catalog_namespace_listing_with_parent", harness.label); + let parent = NamespaceIdent::new(parent_name.clone()); + let child1 = NamespaceIdent::from_strs([&parent_name, "child1"]).unwrap(); + let child2 = NamespaceIdent::from_strs([&parent_name, "child2"]).unwrap(); + + cleanup_namespace_dyn(catalog.as_ref(), &child1).await; + cleanup_namespace_dyn(catalog.as_ref(), &child2).await; + cleanup_namespace_dyn(catalog.as_ref(), &parent).await; + + catalog.create_namespace(&parent, HashMap::new()).await?; + + catalog.create_namespace(&child1, HashMap::new()).await?; + catalog.create_namespace(&child2, HashMap::new()).await?; + + let top_level = catalog.list_namespaces(None).await?; + assert!(top_level.contains(&parent)); + + let children = catalog.list_namespaces(Some(&parent)).await?; + assert!(children.contains(&child1)); + assert!(children.contains(&child2)); + + Ok(()) +} + +// Common behavior: hierarchical namespaces are rejected when unsupported. +#[rstest] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::hms_catalog(CatalogKind::Hms)] +#[tokio::test] +async fn test_catalog_namespace_listing_with_parent_unsupported( + #[case] kind: CatalogKind, +) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let parent_name = normalize_test_name_with_parts!( + "catalog_namespace_listing_with_parent_unsupported", + harness.label + ); + let parent = NamespaceIdent::new(parent_name.clone()); + let child = NamespaceIdent::from_strs([&parent_name, "child"]).unwrap(); + + cleanup_namespace_dyn(catalog.as_ref(), &child).await; + cleanup_namespace_dyn(catalog.as_ref(), &parent).await; + + catalog.create_namespace(&parent, HashMap::new()).await?; + + let err = catalog + .create_namespace(&child, HashMap::new()) + .await + .unwrap_err(); + assert_eq!(err.kind(), ErrorKind::DataInvalid); + + Ok(()) +} + +// Common behavior: listing top-level namespaces includes created namespaces. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::hms_catalog(CatalogKind::Hms)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_list_namespaces_contains_created(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let ns_one = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_list_namespaces_contains_created", + harness.label, + "one" + )); + let ns_two = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_list_namespaces_contains_created", + harness.label, + "two" + )); + + cleanup_namespace_dyn(catalog.as_ref(), &ns_one).await; + cleanup_namespace_dyn(catalog.as_ref(), &ns_two).await; + + catalog.create_namespace(&ns_one, HashMap::new()).await?; + catalog.create_namespace(&ns_two, HashMap::new()).await?; + + let namespaces = catalog.list_namespaces(None).await?; + assert!(namespaces.contains(&ns_one)); + assert!(namespaces.contains(&ns_two)); + + Ok(()) +} + +// Common behavior: creating an existing namespace should error. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::hms_catalog(CatalogKind::Hms)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_create_namespace_duplicate_fails(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_create_namespace_duplicate_fails", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + assert!( + catalog + .create_namespace(&namespace, HashMap::new()) + .await + .is_err() + ); + Ok(()) +} + +// Common behavior: update on a missing namespace should error. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::hms_catalog(CatalogKind::Hms)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_update_namespace_missing_errors(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_update_namespace_missing_errors", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + + assert!( + catalog + .update_namespace( + &namespace, + HashMap::from([("key".to_string(), "value".to_string())]), + ) + .await + .is_err() + ); + + Ok(()) +} + +// Common behavior: dropping a missing namespace should error. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::hms_catalog(CatalogKind::Hms)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_drop_namespace_missing_errors(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_drop_namespace_missing_errors", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + + assert!(catalog.drop_namespace(&namespace).await.is_err()); + Ok(()) +} diff --git a/crates/catalog/loader/tests/table_register_suite.rs b/crates/catalog/loader/tests/table_register_suite.rs new file mode 100644 index 0000000000..b43054b833 --- /dev/null +++ b/crates/catalog/loader/tests/table_register_suite.rs @@ -0,0 +1,167 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Common register-table behavior across catalogs. +//! +//! These tests assume Docker containers are started externally via `make docker-up`. + +mod common; + +use std::collections::HashMap; + +use common::{CatalogKind, cleanup_namespace_dyn, load_catalog, table_creation}; +use iceberg::{ErrorKind, NamespaceIdent, Result, TableIdent}; +use iceberg_test_utils::normalize_test_name_with_parts; +use rstest::rstest; + +// Common behavior: register_table rehydrates a dropped table. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_register_table_roundtrip(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_register_table_roundtrip", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let table = catalog + .create_table( + &namespace, + table_creation(normalize_test_name_with_parts!( + "catalog_register_table_roundtrip", + harness.label, + "table" + )), + ) + .await?; + let table_ident = table.identifier().clone(); + let metadata_location = table + .metadata_location() + .ok_or_else(|| iceberg::Error::new(ErrorKind::Unexpected, "Missing metadata location"))? + .to_string(); + + catalog.drop_table(&table_ident).await?; + + let registered = catalog + .register_table(&table_ident, metadata_location.clone()) + .await?; + assert_eq!(registered.identifier(), &table_ident); + assert_eq!( + registered.metadata_location(), + Some(metadata_location.as_str()) + ); + + Ok(()) +} + +// HMS and S3Tables do not support register_table yet. +#[rstest] +#[case::hms_catalog(CatalogKind::Hms)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[tokio::test] +async fn test_catalog_register_table_unsupported(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_register_table_unsupported", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let table = catalog + .create_table( + &namespace, + table_creation(normalize_test_name_with_parts!( + "catalog_register_table_unsupported", + harness.label, + "table" + )), + ) + .await?; + let table_ident = table.identifier().clone(); + let metadata_location = table + .metadata_location() + .ok_or_else(|| iceberg::Error::new(ErrorKind::Unexpected, "Missing metadata location"))? + .to_string(); + + let err = catalog + .register_table(&table_ident, metadata_location) + .await + .unwrap_err(); + assert_eq!(err.kind(), ErrorKind::FeatureUnsupported); + + Ok(()) +} + +// Common behavior: registering a table with an existing name should error. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_register_table_conflict_errors(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_register_table_conflict_errors", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let table_ident = TableIdent::new( + namespace.clone(), + normalize_test_name_with_parts!( + "catalog_register_table_conflict_errors", + harness.label, + "table" + ), + ); + let table = catalog + .create_table(&namespace, table_creation(table_ident.name.clone())) + .await?; + let metadata_location = table + .metadata_location() + .ok_or_else(|| iceberg::Error::new(ErrorKind::Unexpected, "Missing metadata location"))? + .to_string(); + + assert!( + catalog + .register_table(&table_ident, metadata_location) + .await + .is_err() + ); + Ok(()) +} diff --git a/crates/catalog/loader/tests/table_rename_suite.rs b/crates/catalog/loader/tests/table_rename_suite.rs new file mode 100644 index 0000000000..757d1cd044 --- /dev/null +++ b/crates/catalog/loader/tests/table_rename_suite.rs @@ -0,0 +1,163 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Common rename behavior across catalogs. +//! +//! These tests assume Docker containers are started externally via `make docker-up`. + +mod common; + +use std::collections::HashMap; + +use common::{CatalogKind, cleanup_namespace_dyn, load_catalog, table_creation}; +use iceberg::{NamespaceIdent, Result, TableIdent}; +use iceberg_test_utils::normalize_test_name_with_parts; +use rstest::rstest; + +// Common behavior: renaming across namespaces moves the table. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::hms_catalog(CatalogKind::Hms)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_rename_table_across_namespaces(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let src_namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_rename_table_across_namespaces", + harness.label, + "src" + )); + let dst_namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_rename_table_across_namespaces", + harness.label, + "dst" + )); + + cleanup_namespace_dyn(catalog.as_ref(), &src_namespace).await; + cleanup_namespace_dyn(catalog.as_ref(), &dst_namespace).await; + catalog + .create_namespace(&src_namespace, HashMap::new()) + .await?; + catalog + .create_namespace(&dst_namespace, HashMap::new()) + .await?; + + let table = catalog + .create_table( + &src_namespace, + table_creation(normalize_test_name_with_parts!( + "catalog_rename_table_across_namespaces", + harness.label, + "table" + )), + ) + .await?; + let src_ident = table.identifier().clone(); + let dst_ident = TableIdent::new(dst_namespace.clone(), src_ident.name.clone()); + + catalog.rename_table(&src_ident, &dst_ident).await?; + assert!(catalog.table_exists(&dst_ident).await?); + assert!(!catalog.table_exists(&src_ident).await?); + + Ok(()) +} + +// Common behavior: renaming a missing table should error. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::hms_catalog(CatalogKind::Hms)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_rename_table_missing_source_errors(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_rename_table_missing_source_errors", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let src_ident = TableIdent::new(namespace.clone(), "missing".to_string()); + let dst_ident = TableIdent::new(namespace.clone(), "dest".to_string()); + + assert!(catalog.rename_table(&src_ident, &dst_ident).await.is_err()); + Ok(()) +} + +// Common behavior: renaming to an existing destination should error. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::hms_catalog(CatalogKind::Hms)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_rename_table_dest_exists_errors(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_rename_table_dest_exists_errors", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let src = catalog + .create_table( + &namespace, + table_creation(normalize_test_name_with_parts!( + "catalog_rename_table_dest_exists_errors", + harness.label, + "src" + )), + ) + .await? + .identifier() + .clone(); + let dst = catalog + .create_table( + &namespace, + table_creation(normalize_test_name_with_parts!( + "catalog_rename_table_dest_exists_errors", + harness.label, + "dst" + )), + ) + .await? + .identifier() + .clone(); + + assert!(catalog.rename_table(&src, &dst).await.is_err()); + Ok(()) +} diff --git a/crates/catalog/loader/tests/table_suite.rs b/crates/catalog/loader/tests/table_suite.rs new file mode 100644 index 0000000000..6b7a3a822c --- /dev/null +++ b/crates/catalog/loader/tests/table_suite.rs @@ -0,0 +1,276 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//! Common table behavior across catalogs. +//! +//! These tests assume Docker containers are started externally via `make docker-up`. + +mod common; + +use std::collections::HashMap; + +use common::{CatalogKind, cleanup_namespace_dyn, load_catalog, table_creation}; +use iceberg::transaction::{ApplyTransactionAction, Transaction}; +use iceberg::{ErrorKind, NamespaceIdent, Result, TableIdent}; +use iceberg_test_utils::normalize_test_name_with_parts; +use rstest::rstest; + +// Common behavior: table lifecycle CRUD. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::hms_catalog(CatalogKind::Hms)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_table_lifecycle(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_table_lifecycle", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let table_name = + normalize_test_name_with_parts!("catalog_table_lifecycle", harness.label, "table"); + let table = catalog + .create_table(&namespace, table_creation(table_name)) + .await?; + let ident = table.identifier().clone(); + + assert!(catalog.table_exists(&ident).await?); + let loaded = catalog.load_table(&ident).await?; + assert_eq!(loaded.identifier(), &ident); + + let tables = catalog.list_tables(&namespace).await?; + assert!(tables.contains(&ident)); + + let dest = TableIdent::new(ident.namespace.clone(), format!("{}_renamed", ident.name)); + catalog.rename_table(&ident, &dest).await?; + assert!(catalog.table_exists(&dest).await?); + assert!(!catalog.table_exists(&ident).await?); + + catalog.drop_table(&dest).await?; + assert!(!catalog.table_exists(&dest).await?); + + catalog.drop_namespace(&namespace).await?; + + Ok(()) +} + +// Common behavior: listing tables for a missing namespace should error. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::hms_catalog(CatalogKind::Hms)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_list_tables_missing_namespace_errors( + #[case] kind: CatalogKind, +) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_list_tables_missing_namespace_errors", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + + assert!(catalog.list_tables(&namespace).await.is_err()); + Ok(()) +} + +// Common behavior: listing tables in an empty namespace returns empty. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::hms_catalog(CatalogKind::Hms)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_list_tables_empty_namespace(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_list_tables_empty_namespace", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let tables = catalog.list_tables(&namespace).await?; + assert!(tables.is_empty()); + + Ok(()) +} + +// Common behavior: created tables expose the requested schema. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::hms_catalog(CatalogKind::Hms)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_create_table_schema(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_create_table_schema", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let table_name = + normalize_test_name_with_parts!("catalog_create_table_schema", harness.label, "table"); + let creation = table_creation(table_name); + let expected_schema = creation.schema.clone(); + + let table = catalog.create_table(&namespace, creation).await?; + assert_eq!(table.identifier().namespace, namespace); + assert_eq!(table.metadata().current_schema().as_ref(), &expected_schema); + + Ok(()) +} + +// Common behavior: updating table properties persists through the catalog. +// HMS is excluded because update_table is not supported yet. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_update_table_properties(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_update_table_properties", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let table_name = + normalize_test_name_with_parts!("catalog_update_table_properties", harness.label, "table"); + let table = catalog + .create_table(&namespace, table_creation(table_name)) + .await?; + + let tx = Transaction::new(&table); + let tx = tx + .update_table_properties() + .set("test_property".to_string(), "test_value".to_string()) + .apply(tx)?; + let updated = tx.commit(catalog.as_ref()).await?; + + assert_eq!( + updated.metadata().properties().get("test_property"), + Some(&"test_value".to_string()) + ); + + Ok(()) +} + +// Common behavior: update_table_properties is rejected when unsupported. +#[rstest] +#[case::hms_catalog(CatalogKind::Hms)] +#[tokio::test] +async fn test_catalog_update_table_properties_unsupported(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_update_table_properties_unsupported", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let table_name = normalize_test_name_with_parts!( + "catalog_update_table_properties_unsupported", + harness.label, + "table" + ); + let table = catalog + .create_table(&namespace, table_creation(table_name)) + .await?; + + let tx = Transaction::new(&table); + let tx = tx + .update_table_properties() + .set("test_property".to_string(), "test_value".to_string()) + .apply(tx)?; + + let err = tx.commit(catalog.as_ref()).await.unwrap_err(); + assert_eq!(err.kind(), ErrorKind::FeatureUnsupported); + + Ok(()) +} + +// Common behavior: dropping a missing table should error. +#[rstest] +#[case::rest_catalog(CatalogKind::Rest)] +#[case::glue_catalog(CatalogKind::Glue)] +#[case::hms_catalog(CatalogKind::Hms)] +#[case::sql_catalog(CatalogKind::Sql)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] +#[case::memory_catalog(CatalogKind::Memory)] +#[tokio::test] +async fn test_catalog_drop_table_missing_errors(#[case] kind: CatalogKind) -> Result<()> { + let Some(harness) = load_catalog(kind).await else { + return Ok(()); + }; + let catalog = harness.catalog; + let namespace = NamespaceIdent::new(normalize_test_name_with_parts!( + "catalog_drop_table_missing_errors", + harness.label + )); + + cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; + catalog.create_namespace(&namespace, HashMap::new()).await?; + + let table_ident = TableIdent::new(namespace.clone(), "missing".to_string()); + assert!(catalog.drop_table(&table_ident).await.is_err()); + Ok(()) +} diff --git a/crates/iceberg/src/catalog/memory/catalog.rs b/crates/iceberg/src/catalog/memory/catalog.rs index df0299acb2..35517e5696 100644 --- a/crates/iceberg/src/catalog/memory/catalog.rs +++ b/crates/iceberg/src/catalog/memory/catalog.rs @@ -305,8 +305,8 @@ impl Catalog for MemoryCatalog { async fn drop_table(&self, table_ident: &TableIdent) -> Result<()> { let mut root_namespace_state = self.root_namespace_state.lock().await; - let metadata_location = root_namespace_state.remove_existing_table(table_ident)?; - self.file_io.delete(&metadata_location).await + root_namespace_state.remove_existing_table(table_ident)?; + Ok(()) } /// Check if a table exists in the catalog. From d59bae100abfd7397ac8cffdf7819e08e7f87ba9 Mon Sep 17 00:00:00 2001 From: Ray Liu Date: Wed, 11 Feb 2026 10:29:24 +0000 Subject: [PATCH 4/5] Address comments --- crates/catalog/glue/src/catalog.rs | 37 ++++- .../catalog/glue/tests/glue_catalog_test.rs | 146 ------------------ crates/catalog/hms/src/catalog.rs | 51 ++++-- .../catalog/loader/tests/namespace_suite.rs | 40 +++-- crates/catalog/rest/src/catalog.rs | 22 +-- .../catalog/rest/tests/rest_catalog_test.rs | 110 ------------- crates/catalog/s3tables/src/catalog.rs | 21 +++ crates/catalog/sql/src/catalog.rs | 24 +-- crates/catalog/sql/src/error.rs | 6 +- 9 files changed, 136 insertions(+), 321 deletions(-) delete mode 100644 crates/catalog/glue/tests/glue_catalog_test.rs delete mode 100644 crates/catalog/rest/tests/rest_catalog_test.rs diff --git a/crates/catalog/glue/src/catalog.rs b/crates/catalog/glue/src/catalog.rs index 37a7996f80..d0bc5de468 100644 --- a/crates/catalog/glue/src/catalog.rs +++ b/crates/catalog/glue/src/catalog.rs @@ -314,6 +314,13 @@ impl Catalog for GlueCatalog { namespace: &NamespaceIdent, properties: HashMap, ) -> Result { + if self.namespace_exists(namespace).await? { + return Err(Error::new( + ErrorKind::NamespaceAlreadyExists, + format!("Namespace {namespace:?} already exists"), + )); + } + let db_input = convert_to_database(namespace, &properties)?; let builder = self.client.0.create_database().database_input(db_input); @@ -340,7 +347,19 @@ impl Catalog for GlueCatalog { let builder = self.client.0.get_database().name(&db_name); let builder = with_catalog_id!(builder, self.config); - let resp = builder.send().await.map_err(from_aws_sdk_error)?; + let resp = builder.send().await.map_err(|err| { + if err + .as_service_error() + .map(|e| e.is_entity_not_found_exception()) + == Some(true) + { + return Error::new( + ErrorKind::NamespaceNotFound, + format!("Namespace {namespace:?} does not exist"), + ); + } + from_aws_sdk_error(err) + })?; match resp.database() { Some(db) => { @@ -348,7 +367,7 @@ impl Catalog for GlueCatalog { Ok(namespace) } None => Err(Error::new( - ErrorKind::DataInvalid, + ErrorKind::NamespaceNotFound, format!("Database with name: {db_name} does not exist"), )), } @@ -404,6 +423,13 @@ impl Catalog for GlueCatalog { namespace: &NamespaceIdent, properties: HashMap, ) -> Result<()> { + if !self.namespace_exists(namespace).await? { + return Err(Error::new( + ErrorKind::NamespaceNotFound, + format!("Namespace {namespace:?} does not exist"), + )); + } + let db_name = validate_namespace(namespace)?; let db_input = convert_to_database(namespace, &properties)?; @@ -431,6 +457,13 @@ impl Catalog for GlueCatalog { /// - `Err(...)` signifies failure to drop the namespace due to validation /// errors, connectivity issues, or Glue Catalog constraints. async fn drop_namespace(&self, namespace: &NamespaceIdent) -> Result<()> { + if !self.namespace_exists(namespace).await? { + return Err(Error::new( + ErrorKind::NamespaceNotFound, + format!("Namespace {namespace:?} does not exist"), + )); + } + let db_name = validate_namespace(namespace)?; let table_list = self.list_tables(namespace).await?; diff --git a/crates/catalog/glue/tests/glue_catalog_test.rs b/crates/catalog/glue/tests/glue_catalog_test.rs deleted file mode 100644 index 55af1a6ad9..0000000000 --- a/crates/catalog/glue/tests/glue_catalog_test.rs +++ /dev/null @@ -1,146 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -//! Integration tests for glue catalog. -//! -//! These tests assume Docker containers are started externally via `make docker-up`. -//! Each test uses unique namespaces based on module path to avoid conflicts. - -use std::collections::HashMap; - -use iceberg::io::{S3_ACCESS_KEY_ID, S3_ENDPOINT, S3_REGION, S3_SECRET_ACCESS_KEY}; -use iceberg::spec::{NestedField, PrimitiveType, Schema, Type}; -use iceberg::{Catalog, CatalogBuilder, NamespaceIdent, Result, TableCreation, TableIdent}; -use iceberg_catalog_glue::{ - AWS_ACCESS_KEY_ID, AWS_REGION_NAME, AWS_SECRET_ACCESS_KEY, GLUE_CATALOG_PROP_URI, - GLUE_CATALOG_PROP_WAREHOUSE, GlueCatalog, GlueCatalogBuilder, -}; -use iceberg_test_utils::{ - cleanup_namespace, get_glue_endpoint, get_minio_endpoint, normalize_test_name_with_parts, - set_up, -}; -use tokio::time::sleep; -use tracing::info; - -async fn get_catalog() -> GlueCatalog { - set_up(); - - let glue_endpoint = get_glue_endpoint(); - let minio_endpoint = get_minio_endpoint(); - - let props = HashMap::from([ - (AWS_ACCESS_KEY_ID.to_string(), "my_access_id".to_string()), - ( - AWS_SECRET_ACCESS_KEY.to_string(), - "my_secret_key".to_string(), - ), - (AWS_REGION_NAME.to_string(), "us-east-1".to_string()), - (S3_ENDPOINT.to_string(), minio_endpoint), - (S3_ACCESS_KEY_ID.to_string(), "admin".to_string()), - (S3_SECRET_ACCESS_KEY.to_string(), "password".to_string()), - (S3_REGION.to_string(), "us-east-1".to_string()), - ]); - - // Wait for bucket to actually exist - let file_io = iceberg::io::FileIO::from_path("s3a://") - .unwrap() - .with_props(props.clone()) - .build() - .unwrap(); - - let mut retries = 0; - while retries < 30 { - if file_io.exists("s3a://warehouse/").await.unwrap_or(false) { - info!("S3 bucket 'warehouse' is ready"); - break; - } - info!("Waiting for bucket creation... (attempt {})", retries + 1); - sleep(std::time::Duration::from_millis(1000)).await; - retries += 1; - } - - let mut glue_props = HashMap::from([ - (GLUE_CATALOG_PROP_URI.to_string(), glue_endpoint), - ( - GLUE_CATALOG_PROP_WAREHOUSE.to_string(), - "s3a://warehouse/hive".to_string(), - ), - ]); - glue_props.extend(props.clone()); - - GlueCatalogBuilder::default() - .load("glue", glue_props) - .await - .unwrap() -} - -async fn set_test_namespace(catalog: &GlueCatalog, namespace: &NamespaceIdent) -> Result<()> { - let properties = HashMap::new(); - catalog.create_namespace(namespace, properties).await?; - - Ok(()) -} - -fn set_table_creation(location: Option, name: impl ToString) -> Result { - let schema = Schema::builder() - .with_schema_id(0) - .with_fields(vec![ - NestedField::required(1, "foo", Type::Primitive(PrimitiveType::Int)).into(), - NestedField::required(2, "bar", Type::Primitive(PrimitiveType::String)).into(), - ]) - .build()?; - - let builder = TableCreation::builder() - .name(name.to_string()) - .properties(HashMap::new()) - .location_opt(location) - .schema(schema); - - Ok(builder.build()) -} - -#[tokio::test] -async fn test_register_table() -> Result<()> { - let catalog = get_catalog().await; - // Use unique namespace to avoid conflicts - let namespace = NamespaceIdent::new(normalize_test_name_with_parts!("test_register_table")); - cleanup_namespace(&catalog, &namespace).await; - set_test_namespace(&catalog, &namespace).await?; - - let location = format!("s3a://warehouse/hive/{namespace}"); - let creation = set_table_creation(Some(location), "my_table")?; - let table = catalog.create_table(&namespace, creation).await?; - let metadata_location = table - .metadata_location() - .expect("Expected metadata location to be set") - .to_string(); - - catalog.drop_table(table.identifier()).await?; - let ident = TableIdent::new(namespace.clone(), "my_table".to_string()); - - let registered = catalog - .register_table(&ident, metadata_location.clone()) - .await?; - - assert_eq!(registered.identifier(), &ident); - assert_eq!( - registered.metadata_location(), - Some(metadata_location.as_str()) - ); - - Ok(()) -} diff --git a/crates/catalog/hms/src/catalog.rs b/crates/catalog/hms/src/catalog.rs index f5909f306a..3c83cccbbf 100644 --- a/crates/catalog/hms/src/catalog.rs +++ b/crates/catalog/hms/src/catalog.rs @@ -260,8 +260,8 @@ impl Catalog for HmsCatalog { ) -> Result { if self.namespace_exists(namespace).await? { return Err(Error::new( - ErrorKind::DataInvalid, - "Namespace already exists", + ErrorKind::NamespaceAlreadyExists, + format!("Namespace {namespace:?} already exists"), )); } let database = convert_to_database(namespace, &properties)?; @@ -288,13 +288,29 @@ impl Catalog for HmsCatalog { async fn get_namespace(&self, namespace: &NamespaceIdent) -> Result { let name = validate_namespace(namespace)?; - let db = self + let resp = self .client .0 .get_database(name.into()) .await - .map(from_thrift_exception) - .map_err(from_thrift_error)??; + .map_err(from_thrift_error)?; + + let db = match resp { + MaybeException::Ok(db) => db, + MaybeException::Exception(ThriftHiveMetastoreGetDatabaseException::O1(_)) => { + return Err(Error::new( + ErrorKind::NamespaceNotFound, + format!("Namespace {:?} not found", namespace), + )); + } + MaybeException::Exception(exception) => { + return Err(Error::new( + ErrorKind::Unexpected, + "Operation failed for hitting thrift error".to_string(), + ) + .with_source(anyhow!("thrift error: {exception:?}"))); + } + }; let ns = convert_to_namespace(&db)?; @@ -349,8 +365,8 @@ impl Catalog for HmsCatalog { ) -> Result<()> { if !self.namespace_exists(namespace).await? { return Err(Error::new( - ErrorKind::DataInvalid, - "Namespace does not exist", + ErrorKind::NamespaceNotFound, + format!("Namespace {namespace:?} does not exist"), )); } let db = convert_to_database(namespace, &properties)?; @@ -386,8 +402,8 @@ impl Catalog for HmsCatalog { if !self.namespace_exists(namespace).await? { return Err(Error::new( - ErrorKind::DataInvalid, - "Namespace does not exist", + ErrorKind::NamespaceNotFound, + format!("Namespace {namespace:?} does not exist"), )); } @@ -413,8 +429,8 @@ impl Catalog for HmsCatalog { let name = validate_namespace(namespace)?; if !self.namespace_exists(namespace).await? { return Err(Error::new( - ErrorKind::DataInvalid, - "Namespace does not exist", + ErrorKind::NamespaceNotFound, + format!("Namespace {namespace:?} does not exist"), )); } @@ -547,12 +563,15 @@ impl Catalog for HmsCatalog { let db_name = validate_namespace(table.namespace())?; if !self.namespace_exists(table.namespace()).await? { return Err(Error::new( - ErrorKind::DataInvalid, - "Namespace does not exist", + ErrorKind::NamespaceNotFound, + format!("Namespace {:?} does not exist", table.namespace()), )); } if !self.table_exists(table).await? { - return Err(Error::new(ErrorKind::DataInvalid, "Table does not exist")); + return Err(Error::new( + ErrorKind::TableNotFound, + format!("Table {table:?} does not exist"), + )); } self.client @@ -604,8 +623,8 @@ impl Catalog for HmsCatalog { let dest_dbname = validate_namespace(dest.namespace())?; if self.table_exists(dest).await? { return Err(Error::new( - ErrorKind::DataInvalid, - "Destination table already exists", + ErrorKind::TableAlreadyExists, + format!("Destination table {dest:?} already exists"), )); } diff --git a/crates/catalog/loader/tests/namespace_suite.rs b/crates/catalog/loader/tests/namespace_suite.rs index e317643033..c99b1a626e 100644 --- a/crates/catalog/loader/tests/namespace_suite.rs +++ b/crates/catalog/loader/tests/namespace_suite.rs @@ -49,7 +49,8 @@ async fn test_catalog_namespace_missing_returns_error(#[case] kind: CatalogKind) cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - assert!(catalog.get_namespace(&namespace).await.is_err()); + let err = catalog.get_namespace(&namespace).await.unwrap_err(); + assert_eq!(err.kind(), ErrorKind::NamespaceNotFound); Ok(()) } @@ -103,7 +104,6 @@ async fn test_catalog_namespace_lifecycle(#[case] kind: CatalogKind) -> Result<( #[case::glue_catalog(CatalogKind::Glue)] #[case::hms_catalog(CatalogKind::Hms)] #[case::sql_catalog(CatalogKind::Sql)] -#[case::s3tables_catalog(CatalogKind::S3Tables)] #[case::memory_catalog(CatalogKind::Memory)] #[tokio::test] async fn test_catalog_update_namespace_supported(#[case] kind: CatalogKind) -> Result<()> { @@ -133,6 +133,7 @@ async fn test_catalog_update_namespace_supported(#[case] kind: CatalogKind) -> R // Common behavior: update_namespace returns FeatureUnsupported when not implemented. #[rstest] #[case::rest_catalog(CatalogKind::Rest)] +#[case::s3tables_catalog(CatalogKind::S3Tables)] #[tokio::test] async fn test_catalog_update_namespace_unsupported(#[case] kind: CatalogKind) -> Result<()> { let Some(harness) = load_catalog(kind).await else { @@ -289,22 +290,19 @@ async fn test_catalog_create_namespace_duplicate_fails(#[case] kind: CatalogKind cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; catalog.create_namespace(&namespace, HashMap::new()).await?; - assert!( - catalog - .create_namespace(&namespace, HashMap::new()) - .await - .is_err() - ); + let err = catalog + .create_namespace(&namespace, HashMap::new()) + .await + .unwrap_err(); + assert_eq!(err.kind(), ErrorKind::NamespaceAlreadyExists); Ok(()) } -// Common behavior: update on a missing namespace should error. +// Common behavior: update on a missing namespace should return NamespaceNotFound. #[rstest] -#[case::rest_catalog(CatalogKind::Rest)] #[case::glue_catalog(CatalogKind::Glue)] #[case::hms_catalog(CatalogKind::Hms)] #[case::sql_catalog(CatalogKind::Sql)] -#[case::s3tables_catalog(CatalogKind::S3Tables)] #[case::memory_catalog(CatalogKind::Memory)] #[tokio::test] async fn test_catalog_update_namespace_missing_errors(#[case] kind: CatalogKind) -> Result<()> { @@ -319,15 +317,14 @@ async fn test_catalog_update_namespace_missing_errors(#[case] kind: CatalogKind) cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - assert!( - catalog - .update_namespace( - &namespace, - HashMap::from([("key".to_string(), "value".to_string())]), - ) - .await - .is_err() - ); + let err = catalog + .update_namespace( + &namespace, + HashMap::from([("key".to_string(), "value".to_string())]), + ) + .await + .unwrap_err(); + assert_eq!(err.kind(), ErrorKind::NamespaceNotFound); Ok(()) } @@ -353,6 +350,7 @@ async fn test_catalog_drop_namespace_missing_errors(#[case] kind: CatalogKind) - cleanup_namespace_dyn(catalog.as_ref(), &namespace).await; - assert!(catalog.drop_namespace(&namespace).await.is_err()); + let err = catalog.drop_namespace(&namespace).await.unwrap_err(); + assert_eq!(err.kind(), ErrorKind::NamespaceNotFound); Ok(()) } diff --git a/crates/catalog/rest/src/catalog.rs b/crates/catalog/rest/src/catalog.rs index ddbf6a4e01..480ac7837d 100644 --- a/crates/catalog/rest/src/catalog.rs +++ b/crates/catalog/rest/src/catalog.rs @@ -475,7 +475,7 @@ impl Catalog for RestCatalog { } StatusCode::NOT_FOUND => { return Err(Error::new( - ErrorKind::Unexpected, + ErrorKind::NamespaceNotFound, "The parent parameter of the namespace provided does not exist", )); } @@ -511,7 +511,7 @@ impl Catalog for RestCatalog { Ok(Namespace::from(response)) } StatusCode::CONFLICT => Err(Error::new( - ErrorKind::Unexpected, + ErrorKind::NamespaceAlreadyExists, "Tried to create a namespace that already exists", )), _ => Err(deserialize_unexpected_catalog_error(http_response).await), @@ -535,7 +535,7 @@ impl Catalog for RestCatalog { Ok(Namespace::from(response)) } StatusCode::NOT_FOUND => Err(Error::new( - ErrorKind::Unexpected, + ErrorKind::NamespaceNotFound, "Tried to get a namespace that does not exist", )), _ => Err(deserialize_unexpected_catalog_error(http_response).await), @@ -583,7 +583,7 @@ impl Catalog for RestCatalog { match http_response.status() { StatusCode::NO_CONTENT | StatusCode::OK => Ok(()), StatusCode::NOT_FOUND => Err(Error::new( - ErrorKind::Unexpected, + ErrorKind::NamespaceNotFound, "Tried to drop a namespace that does not exist", )), _ => Err(deserialize_unexpected_catalog_error(http_response).await), @@ -619,7 +619,7 @@ impl Catalog for RestCatalog { } StatusCode::NOT_FOUND => { return Err(Error::new( - ErrorKind::Unexpected, + ErrorKind::NamespaceNotFound, "Tried to list tables of a namespace that does not exist", )); } @@ -667,13 +667,13 @@ impl Catalog for RestCatalog { } StatusCode::NOT_FOUND => { return Err(Error::new( - ErrorKind::Unexpected, + ErrorKind::NamespaceNotFound, "Tried to create a table under a namespace that does not exist", )); } StatusCode::CONFLICT => { return Err(Error::new( - ErrorKind::Unexpected, + ErrorKind::TableAlreadyExists, "The table already exists", )); } @@ -728,7 +728,7 @@ impl Catalog for RestCatalog { } StatusCode::NOT_FOUND => { return Err(Error::new( - ErrorKind::Unexpected, + ErrorKind::TableNotFound, "Tried to load a table that does not exist", )); } @@ -771,7 +771,7 @@ impl Catalog for RestCatalog { match http_response.status() { StatusCode::NO_CONTENT | StatusCode::OK => Ok(()), StatusCode::NOT_FOUND => Err(Error::new( - ErrorKind::Unexpected, + ErrorKind::TableNotFound, "Tried to drop a table that does not exist", )), _ => Err(deserialize_unexpected_catalog_error(http_response).await), @@ -814,11 +814,11 @@ impl Catalog for RestCatalog { match http_response.status() { StatusCode::NO_CONTENT | StatusCode::OK => Ok(()), StatusCode::NOT_FOUND => Err(Error::new( - ErrorKind::Unexpected, + ErrorKind::TableNotFound, "Tried to rename a table that does not exist (is the namespace correct?)", )), StatusCode::CONFLICT => Err(Error::new( - ErrorKind::Unexpected, + ErrorKind::TableAlreadyExists, "Tried to rename a table to a name that already exists", )), _ => Err(deserialize_unexpected_catalog_error(http_response).await), diff --git a/crates/catalog/rest/tests/rest_catalog_test.rs b/crates/catalog/rest/tests/rest_catalog_test.rs deleted file mode 100644 index 12bb0c8181..0000000000 --- a/crates/catalog/rest/tests/rest_catalog_test.rs +++ /dev/null @@ -1,110 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -//! Integration tests for rest catalog. -//! -//! These tests assume Docker containers are started externally via `make docker-up`. -//! Each test uses unique namespaces based on module path to avoid conflicts. - -use std::collections::HashMap; - -use iceberg::spec::Schema; -use iceberg::{Catalog, CatalogBuilder, NamespaceIdent, TableCreation, TableIdent}; -use iceberg_catalog_rest::{REST_CATALOG_PROP_URI, RestCatalog, RestCatalogBuilder}; -use iceberg_test_utils::{ - cleanup_namespace, get_rest_catalog_endpoint, normalize_test_name_with_parts, set_up, -}; -use tokio::time::sleep; -use tracing::info; - -async fn get_catalog() -> RestCatalog { - set_up(); - - let rest_endpoint = get_rest_catalog_endpoint(); - - // Wait for catalog to be ready - let client = reqwest::Client::new(); - let mut retries = 0; - while retries < 30 { - match client - .get(format!("{rest_endpoint}/v1/config")) - .send() - .await - { - Ok(resp) if resp.status().is_success() => { - info!("REST catalog is ready at {}", rest_endpoint); - break; - } - _ => { - info!( - "Waiting for REST catalog to be ready... (attempt {})", - retries + 1 - ); - sleep(std::time::Duration::from_millis(1000)).await; - retries += 1; - } - } - } - - RestCatalogBuilder::default() - .load( - "rest", - HashMap::from([(REST_CATALOG_PROP_URI.to_string(), rest_endpoint)]), - ) - .await - .unwrap() -} - -#[tokio::test] -async fn test_register_table() { - let catalog = get_catalog().await; - - // Create unique namespace to avoid conflicts - let ns = NamespaceIdent::new(normalize_test_name_with_parts!("test_register_table")); - - // Clean up from any previous test runs - cleanup_namespace(&catalog, &ns).await; - - catalog.create_namespace(&ns, HashMap::new()).await.unwrap(); - - // Create the table, store the metadata location, drop the table - let empty_schema = Schema::builder().build().unwrap(); - let table_creation = TableCreation::builder() - .name("t1".to_string()) - .schema(empty_schema) - .build(); - - let table = catalog.create_table(&ns, table_creation).await.unwrap(); - - let metadata_location = table.metadata_location().unwrap(); - catalog.drop_table(table.identifier()).await.unwrap(); - - let new_table_identifier = TableIdent::new(ns.clone(), "t2".to_string()); - let table_registered = catalog - .register_table(&new_table_identifier, metadata_location.to_string()) - .await - .unwrap(); - - assert_eq!( - table.metadata_location(), - table_registered.metadata_location() - ); - assert_ne!( - table.identifier().to_string(), - table_registered.identifier().to_string() - ); -} diff --git a/crates/catalog/s3tables/src/catalog.rs b/crates/catalog/s3tables/src/catalog.rs index bd3f8d0150..4ca528a903 100644 --- a/crates/catalog/s3tables/src/catalog.rs +++ b/crates/catalog/s3tables/src/catalog.rs @@ -282,6 +282,13 @@ impl Catalog for S3TablesCatalog { namespace: &NamespaceIdent, _properties: HashMap, ) -> Result { + if self.namespace_exists(namespace).await? { + return Err(Error::new( + ErrorKind::NamespaceAlreadyExists, + format!("Namespace {namespace:?} already exists"), + )); + } + let req = self .s3tables_client .create_namespace() @@ -304,6 +311,13 @@ impl Catalog for S3TablesCatalog { /// - If there is an error querying the database, returned by /// `from_aws_sdk_error`. async fn get_namespace(&self, namespace: &NamespaceIdent) -> Result { + if !self.namespace_exists(namespace).await? { + return Err(Error::new( + ErrorKind::NamespaceNotFound, + format!("Namespace {namespace:?} does not exist"), + )); + } + let req = self .s3tables_client .get_namespace() @@ -371,6 +385,13 @@ impl Catalog for S3TablesCatalog { /// - Errors from the underlying database deletion process, converted using /// `from_aws_sdk_error`. async fn drop_namespace(&self, namespace: &NamespaceIdent) -> Result<()> { + if !self.namespace_exists(namespace).await? { + return Err(Error::new( + ErrorKind::NamespaceNotFound, + format!("Namespace {namespace:?} does not exist"), + )); + } + let req = self .s3tables_client .delete_namespace() diff --git a/crates/catalog/sql/src/catalog.rs b/crates/catalog/sql/src/catalog.rs index 1fb21e5fde..d8acb4f206 100644 --- a/crates/catalog/sql/src/catalog.rs +++ b/crates/catalog/sql/src/catalog.rs @@ -412,7 +412,7 @@ impl Catalog for SqlCatalog { if exists { return Err(Error::new( - iceberg::ErrorKind::Unexpected, + iceberg::ErrorKind::NamespaceAlreadyExists, format!("Namespace {namespace:?} already exists"), )); } @@ -1511,7 +1511,7 @@ mod tests { .unwrap_err() .to_string(), format!( - "Unexpected => Namespace {:?} already exists", + "NamespaceAlreadyExists => Namespace {:?} already exists", &namespace_ident ) ); @@ -1773,7 +1773,7 @@ mod tests { .await .unwrap_err() .to_string(), - format!("Unexpected => No such namespace: {non_existent_namespace_ident:?}") + format!("NamespaceNotFound => No such namespace: {non_existent_namespace_ident:?}") ) } @@ -1791,7 +1791,7 @@ mod tests { .await .unwrap_err() .to_string(), - format!("Unexpected => No such namespace: {non_existent_namespace_ident:?}") + format!("NamespaceNotFound => No such namespace: {non_existent_namespace_ident:?}") ) } @@ -1838,7 +1838,7 @@ mod tests { .await .unwrap_err() .to_string(), - format!("Unexpected => No such namespace: {non_existent_namespace_ident:?}"), + format!("NamespaceNotFound => No such namespace: {non_existent_namespace_ident:?}"), ); } @@ -2079,7 +2079,7 @@ mod tests { .await .unwrap_err() .to_string(), - format!("Unexpected => Table {:?} already exists.", &table_ident) + format!("TableAlreadyExists => Table {:?} already exists.", &table_ident) ); } @@ -2195,7 +2195,7 @@ mod tests { .await .unwrap_err() .to_string(), - format!("Unexpected => No such namespace: {non_existent_dst_namespace_ident:?}"), + format!("NamespaceNotFound => No such namespace: {non_existent_dst_namespace_ident:?}"), ); } @@ -2214,7 +2214,7 @@ mod tests { .await .unwrap_err() .to_string(), - format!("Unexpected => No such table: {src_table_ident:?}"), + format!("TableNotFound => No such table: {src_table_ident:?}"), ); } @@ -2234,7 +2234,7 @@ mod tests { .await .unwrap_err() .to_string(), - format!("Unexpected => Table {:?} already exists.", &dst_table_ident), + format!("TableAlreadyExists => Table {:?} already exists.", &dst_table_ident), ); } @@ -2254,7 +2254,7 @@ mod tests { .to_string(); assert_eq!( err, - "Unexpected => No such table: TableIdent { namespace: NamespaceIdent([\"a\"]), name: \"tbl1\" }" + "TableNotFound => No such table: TableIdent { namespace: NamespaceIdent([\"a\"]), name: \"tbl1\" }" ); } @@ -2290,7 +2290,7 @@ mod tests { .to_string(); assert_eq!( err, - "Unexpected => No such table: TableIdent { namespace: NamespaceIdent([\"a\"]), name: \"tbl1\" }" + "TableNotFound => No such table: TableIdent { namespace: NamespaceIdent([\"a\"]), name: \"tbl1\" }" ); } @@ -2310,7 +2310,7 @@ mod tests { .await .unwrap_err() .to_string(), - format!("Unexpected => Table {:?} already exists.", &table_ident) + format!("TableAlreadyExists => Table {:?} already exists.", &table_ident) ); } diff --git a/crates/catalog/sql/src/error.rs b/crates/catalog/sql/src/error.rs index a08f755596..55e0e0a368 100644 --- a/crates/catalog/sql/src/error.rs +++ b/crates/catalog/sql/src/error.rs @@ -28,21 +28,21 @@ pub fn from_sqlx_error(error: sqlx::Error) -> Error { pub fn no_such_namespace_err(namespace: &NamespaceIdent) -> Result { Err(Error::new( - ErrorKind::Unexpected, + ErrorKind::NamespaceNotFound, format!("No such namespace: {namespace:?}"), )) } pub fn no_such_table_err(table_ident: &TableIdent) -> Result { Err(Error::new( - ErrorKind::Unexpected, + ErrorKind::TableNotFound, format!("No such table: {table_ident:?}"), )) } pub fn table_already_exists_err(table_ident: &TableIdent) -> Result { Err(Error::new( - ErrorKind::Unexpected, + ErrorKind::TableAlreadyExists, format!("Table {table_ident:?} already exists."), )) } From 2acfe92c88ea0f5a67f65be3f3c6d0c8d3da17e8 Mon Sep 17 00:00:00 2001 From: Ray Liu Date: Wed, 11 Feb 2026 10:54:50 +0000 Subject: [PATCH 5/5] Fix linter --- Cargo.lock | 2 - crates/catalog/glue/Cargo.toml | 1 - crates/catalog/hms/src/catalog.rs | 2 +- crates/catalog/rest/Cargo.toml | 1 - crates/catalog/sql/src/catalog.rs | 429 +----------------------------- 5 files changed, 5 insertions(+), 430 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b49611977d..1502782c75 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3445,7 +3445,6 @@ dependencies = [ "iceberg_test_utils", "serde_json", "tokio", - "tracing", ] [[package]] @@ -3505,7 +3504,6 @@ dependencies = [ "serde_derive", "serde_json", "tokio", - "tracing", "typed-builder", "uuid", ] diff --git a/crates/catalog/glue/Cargo.toml b/crates/catalog/glue/Cargo.toml index f42fedeae1..403442b1d0 100644 --- a/crates/catalog/glue/Cargo.toml +++ b/crates/catalog/glue/Cargo.toml @@ -36,7 +36,6 @@ aws-sdk-glue = { workspace = true } iceberg = { workspace = true } serde_json = { workspace = true } tokio = { workspace = true } -tracing = { workspace = true } [dev-dependencies] iceberg_test_utils = { path = "../../test_utils", features = ["tests"] } diff --git a/crates/catalog/hms/src/catalog.rs b/crates/catalog/hms/src/catalog.rs index 3c83cccbbf..3127b1e29a 100644 --- a/crates/catalog/hms/src/catalog.rs +++ b/crates/catalog/hms/src/catalog.rs @@ -300,7 +300,7 @@ impl Catalog for HmsCatalog { MaybeException::Exception(ThriftHiveMetastoreGetDatabaseException::O1(_)) => { return Err(Error::new( ErrorKind::NamespaceNotFound, - format!("Namespace {:?} not found", namespace), + format!("Namespace {namespace:?} not found"), )); } MaybeException::Exception(exception) => { diff --git a/crates/catalog/rest/Cargo.toml b/crates/catalog/rest/Cargo.toml index de72b6c61b..40dd70a952 100644 --- a/crates/catalog/rest/Cargo.toml +++ b/crates/catalog/rest/Cargo.toml @@ -39,7 +39,6 @@ serde = { workspace = true } serde_derive = { workspace = true } serde_json = { workspace = true } tokio = { workspace = true, features = ["sync"] } -tracing = { workspace = true } typed-builder = { workspace = true } uuid = { workspace = true, features = ["v4"] } diff --git a/crates/catalog/sql/src/catalog.rs b/crates/catalog/sql/src/catalog.rs index d8acb4f206..45598a1691 100644 --- a/crates/catalog/sql/src/catalog.rs +++ b/crates/catalog/sql/src/catalog.rs @@ -979,7 +979,6 @@ mod tests { use iceberg::spec::{NestedField, PartitionSpec, PrimitiveType, Schema, SortOrder, Type}; use iceberg::table::Table; - use iceberg::transaction::{ApplyTransactionAction, Transaction}; use iceberg::{Catalog, CatalogBuilder, Namespace, NamespaceIdent, TableCreation, TableIdent}; use itertools::Itertools; use regex::Regex; @@ -1497,31 +1496,6 @@ mod tests { ); } - #[tokio::test] - async fn test_create_namespace_throws_error_if_namespace_already_exists() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc, Some("iceberg")).await; - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - - assert_eq!( - catalog - .create_namespace(&namespace_ident, HashMap::new()) - .await - .unwrap_err() - .to_string(), - format!( - "NamespaceAlreadyExists => Namespace {:?} already exists", - &namespace_ident - ) - ); - - assert_eq!( - catalog.get_namespace(&namespace_ident).await.unwrap(), - Namespace::with_properties(namespace_ident, default_properties()) - ); - } - #[tokio::test] async fn test_create_nested_namespace() { let warehouse_loc = temp_path(); @@ -1591,35 +1565,6 @@ mod tests { ) } - #[tokio::test] - async fn test_update_namespace() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc, Some("iceberg")).await; - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - - let mut props = HashMap::from_iter([ - ("prop1".to_string(), "val1".to_string()), - ("prop2".into(), "val2".into()), - ]); - - catalog - .update_namespace(&namespace_ident, props.clone()) - .await - .unwrap(); - - props.insert("exists".into(), "true".into()); - - assert_eq!( - *catalog - .get_namespace(&namespace_ident) - .await - .unwrap() - .properties(), - props - ) - } - #[tokio::test] async fn test_update_nested_namespace() { let warehouse_loc = temp_path(); @@ -1649,28 +1594,6 @@ mod tests { ) } - #[tokio::test] - async fn test_update_namespace_errors_if_namespace_doesnt_exist() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc, Some("iceberg")).await; - let namespace_ident = NamespaceIdent::new("a".into()); - - let props = HashMap::from_iter([ - ("prop1".to_string(), "val1".to_string()), - ("prop2".into(), "val2".into()), - ]); - - let err = catalog - .update_namespace(&namespace_ident, props) - .await - .unwrap_err(); - - assert_eq!( - err.message(), - format!("No such namespace: {namespace_ident:?}") - ); - } - #[tokio::test] async fn test_update_namespace_errors_if_nested_namespace_doesnt_exist() { let warehouse_loc = temp_path(); @@ -1693,18 +1616,6 @@ mod tests { ); } - #[tokio::test] - async fn test_drop_namespace() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc, Some("iceberg")).await; - let namespace_ident = NamespaceIdent::new("abc".into()); - create_namespace(&catalog, &namespace_ident).await; - - catalog.drop_namespace(&namespace_ident).await.unwrap(); - - assert!(!catalog.namespace_exists(&namespace_ident).await.unwrap()) - } - #[tokio::test] async fn test_drop_nested_namespace() { let warehouse_loc = temp_path(); @@ -1761,22 +1672,6 @@ mod tests { assert!(catalog.namespace_exists(&namespace_ident_a).await.unwrap()); } - #[tokio::test] - async fn test_drop_namespace_throws_error_if_namespace_doesnt_exist() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc, Some("iceberg")).await; - - let non_existent_namespace_ident = NamespaceIdent::new("abc".into()); - assert_eq!( - catalog - .drop_namespace(&non_existent_namespace_ident) - .await - .unwrap_err() - .to_string(), - format!("NamespaceNotFound => No such namespace: {non_existent_namespace_ident:?}") - ) - } - #[tokio::test] async fn test_drop_namespace_throws_error_if_nested_namespace_doesnt_exist() { let warehouse_loc = temp_path(); @@ -1815,72 +1710,6 @@ mod tests { ); } - #[tokio::test] - async fn test_list_tables_returns_empty_vector() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc, Some("iceberg")).await; - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - - assert_eq!(catalog.list_tables(&namespace_ident).await.unwrap(), vec![]); - } - - #[tokio::test] - async fn test_list_tables_throws_error_if_namespace_doesnt_exist() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc, Some("iceberg")).await; - - let non_existent_namespace_ident = NamespaceIdent::new("n1".into()); - - assert_eq!( - catalog - .list_tables(&non_existent_namespace_ident) - .await - .unwrap_err() - .to_string(), - format!("NamespaceNotFound => No such namespace: {non_existent_namespace_ident:?}"), - ); - } - - #[tokio::test] - async fn test_create_table_with_location() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc.clone(), Some("iceberg")).await; - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - - let table_name = "abc"; - let location = warehouse_loc.clone(); - let table_creation = TableCreation::builder() - .name(table_name.into()) - .location(location.clone()) - .schema(simple_table_schema()) - .build(); - - let expected_table_ident = TableIdent::new(namespace_ident.clone(), table_name.into()); - - assert_table_eq( - &catalog - .create_table(&namespace_ident, table_creation) - .await - .unwrap(), - &expected_table_ident, - &simple_table_schema(), - ); - - let table = catalog.load_table(&expected_table_ident).await.unwrap(); - - assert_table_eq(&table, &expected_table_ident, &simple_table_schema()); - - assert!( - table - .metadata_location() - .unwrap() - .to_string() - .starts_with(&location) - ) - } - #[tokio::test] async fn test_create_table_falls_back_to_namespace_location_if_table_location_is_missing() { let warehouse_loc = temp_path(); @@ -2079,54 +1908,10 @@ mod tests { .await .unwrap_err() .to_string(), - format!("TableAlreadyExists => Table {:?} already exists.", &table_ident) - ); - } - - #[tokio::test] - async fn test_rename_table_in_same_namespace() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc, Some("iceberg")).await; - let namespace_ident = NamespaceIdent::new("n1".into()); - create_namespace(&catalog, &namespace_ident).await; - let src_table_ident = TableIdent::new(namespace_ident.clone(), "tbl1".into()); - let dst_table_ident = TableIdent::new(namespace_ident.clone(), "tbl2".into()); - create_table(&catalog, &src_table_ident).await; - - catalog - .rename_table(&src_table_ident, &dst_table_ident) - .await - .unwrap(); - - assert_eq!(catalog.list_tables(&namespace_ident).await.unwrap(), vec![ - dst_table_ident - ],); - } - - #[tokio::test] - async fn test_rename_table_across_namespaces() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc, Some("iceberg")).await; - let src_namespace_ident = NamespaceIdent::new("a".into()); - let dst_namespace_ident = NamespaceIdent::new("b".into()); - create_namespaces(&catalog, &vec![&src_namespace_ident, &dst_namespace_ident]).await; - let src_table_ident = TableIdent::new(src_namespace_ident.clone(), "tbl1".into()); - let dst_table_ident = TableIdent::new(dst_namespace_ident.clone(), "tbl2".into()); - create_table(&catalog, &src_table_ident).await; - - catalog - .rename_table(&src_table_ident, &dst_table_ident) - .await - .unwrap(); - - assert_eq!( - catalog.list_tables(&src_namespace_ident).await.unwrap(), - vec![], - ); - - assert_eq!( - catalog.list_tables(&dst_namespace_ident).await.unwrap(), - vec![dst_table_ident], + format!( + "TableAlreadyExists => Table {:?} already exists.", + &table_ident + ) ); } @@ -2198,210 +1983,4 @@ mod tests { format!("NamespaceNotFound => No such namespace: {non_existent_dst_namespace_ident:?}"), ); } - - #[tokio::test] - async fn test_rename_table_throws_error_if_src_table_doesnt_exist() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc, Some("iceberg")).await; - let namespace_ident = NamespaceIdent::new("n1".into()); - create_namespace(&catalog, &namespace_ident).await; - let src_table_ident = TableIdent::new(namespace_ident.clone(), "tbl1".into()); - let dst_table_ident = TableIdent::new(namespace_ident.clone(), "tbl2".into()); - - assert_eq!( - catalog - .rename_table(&src_table_ident, &dst_table_ident) - .await - .unwrap_err() - .to_string(), - format!("TableNotFound => No such table: {src_table_ident:?}"), - ); - } - - #[tokio::test] - async fn test_rename_table_throws_error_if_dst_table_already_exists() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc, Some("iceberg")).await; - let namespace_ident = NamespaceIdent::new("n1".into()); - create_namespace(&catalog, &namespace_ident).await; - let src_table_ident = TableIdent::new(namespace_ident.clone(), "tbl1".into()); - let dst_table_ident = TableIdent::new(namespace_ident.clone(), "tbl2".into()); - create_tables(&catalog, vec![&src_table_ident, &dst_table_ident]).await; - - assert_eq!( - catalog - .rename_table(&src_table_ident, &dst_table_ident) - .await - .unwrap_err() - .to_string(), - format!("TableAlreadyExists => Table {:?} already exists.", &dst_table_ident), - ); - } - - #[tokio::test] - async fn test_drop_table_throws_error_if_table_not_exist() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc.clone(), Some("iceberg")).await; - let namespace_ident = NamespaceIdent::new("a".into()); - let table_name = "tbl1"; - let table_ident = TableIdent::new(namespace_ident.clone(), table_name.into()); - create_namespace(&catalog, &namespace_ident).await; - - let err = catalog - .drop_table(&table_ident) - .await - .unwrap_err() - .to_string(); - assert_eq!( - err, - "TableNotFound => No such table: TableIdent { namespace: NamespaceIdent([\"a\"]), name: \"tbl1\" }" - ); - } - - #[tokio::test] - async fn test_drop_table() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc.clone(), Some("iceberg")).await; - let namespace_ident = NamespaceIdent::new("a".into()); - let table_name = "tbl1"; - let table_ident = TableIdent::new(namespace_ident.clone(), table_name.into()); - create_namespace(&catalog, &namespace_ident).await; - - let location = warehouse_loc.clone(); - let table_creation = TableCreation::builder() - .name(table_name.into()) - .location(location.clone()) - .schema(simple_table_schema()) - .build(); - - catalog - .create_table(&namespace_ident, table_creation) - .await - .unwrap(); - - let table = catalog.load_table(&table_ident).await.unwrap(); - assert_table_eq(&table, &table_ident, &simple_table_schema()); - - catalog.drop_table(&table_ident).await.unwrap(); - let err = catalog - .load_table(&table_ident) - .await - .unwrap_err() - .to_string(); - assert_eq!( - err, - "TableNotFound => No such table: TableIdent { namespace: NamespaceIdent([\"a\"]), name: \"tbl1\" }" - ); - } - - #[tokio::test] - async fn test_register_table_throws_error_if_table_with_same_name_already_exists() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc.clone(), Some("iceberg")).await; - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - let table_name = "tbl1"; - let table_ident = TableIdent::new(namespace_ident.clone(), table_name.into()); - create_table(&catalog, &table_ident).await; - - assert_eq!( - catalog - .register_table(&table_ident, warehouse_loc) - .await - .unwrap_err() - .to_string(), - format!("TableAlreadyExists => Table {:?} already exists.", &table_ident) - ); - } - - #[tokio::test] - async fn test_register_table() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc.clone(), Some("iceberg")).await; - let namespace_ident = NamespaceIdent::new("a".into()); - create_namespace(&catalog, &namespace_ident).await; - - let table_name = "abc"; - let location = warehouse_loc.clone(); - let table_creation = TableCreation::builder() - .name(table_name.into()) - .location(location.clone()) - .schema(simple_table_schema()) - .build(); - - let table_ident = TableIdent::new(namespace_ident.clone(), table_name.into()); - let expected_table = catalog - .create_table(&namespace_ident, table_creation) - .await - .unwrap(); - - let metadata_location = expected_table - .metadata_location() - .expect("Expected metadata location to be set") - .to_string(); - - assert_table_eq(&expected_table, &table_ident, &simple_table_schema()); - - let _ = catalog.drop_table(&table_ident).await; - - let table = catalog - .register_table(&table_ident, metadata_location.clone()) - .await - .unwrap(); - - assert_eq!(table.identifier(), expected_table.identifier()); - assert_eq!(table.metadata_location(), Some(metadata_location.as_str())); - } - - #[tokio::test] - async fn test_update_table() { - let warehouse_loc = temp_path(); - let catalog = new_sql_catalog(warehouse_loc, Some("iceberg")).await; - - // Create a test namespace and table - let namespace_ident = NamespaceIdent::new("ns1".into()); - create_namespace(&catalog, &namespace_ident).await; - let table_ident = TableIdent::new(namespace_ident.clone(), "tbl1".into()); - create_table(&catalog, &table_ident).await; - - let table = catalog.load_table(&table_ident).await.unwrap(); - - // Store the original metadata location for comparison - let original_metadata_location = table.metadata_location().unwrap().to_string(); - - // Create a transaction to update the table - let tx = Transaction::new(&table); - let tx = tx - .update_table_properties() - .set("test_property".to_string(), "test_value".to_string()) - .apply(tx) - .unwrap(); - - // Commit the transaction to the catalog - let updated_table = tx.commit(&catalog).await.unwrap(); - - // Verify the update was successful - assert_eq!( - updated_table.metadata().properties().get("test_property"), - Some(&"test_value".to_string()) - ); - // Verify the metadata location has been updated - assert_ne!( - updated_table.metadata_location().unwrap(), - original_metadata_location.as_str() - ); - - // Load the table again from the catalog to verify changes were persisted - let reloaded = catalog.load_table(&table_ident).await.unwrap(); - - // Verify the reloaded table matches the updated table - assert_eq!( - reloaded.metadata().properties().get("test_property"), - Some(&"test_value".to_string()) - ); - assert_eq!( - reloaded.metadata_location(), - updated_table.metadata_location() - ); - } }