Merge branch 'hms-location' into dev

Author: sundy-li

crates/catalog/glue/src/catalog.rs

@@ -172,7 +172,7 @@ impl Catalog for GlueCatalog {
     ///
     /// - Errors from `validate_namespace` if the namespace identifier does not
     /// meet validation criteria.
-    /// - Errors from `convert_to_database` if the properties cannot be  
+    /// - Errors from `convert_to_database` if the properties cannot be
     /// successfully converted into a database configuration.
     /// - Errors from the underlying database creation process, converted using
     /// `from_sdk_error`.
@@ -259,7 +259,7 @@ impl Catalog for GlueCatalog {
     /// Asynchronously updates properties of an existing namespace.
     ///
     /// Converts the given namespace identifier and properties into a database
-    /// representation and then attempts to update the corresponding namespace  
+    /// representation and then attempts to update the corresponding namespace
     /// in the Glue Catalog.
     ///
     /// # Returns
@@ -295,7 +295,7 @@ impl Catalog for GlueCatalog {
     /// # Returns
     /// A `Result<()>` indicating the outcome:
     /// - `Ok(())` signifies successful namespace deletion.
-    /// - `Err(...)` signifies failure to drop the namespace due to validation  
+    /// - `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<()> {
         let db_name = validate_namespace(namespace)?;
@@ -322,7 +322,7 @@ impl Catalog for GlueCatalog {
     /// A `Result<Vec<TableIdent>>`, which is:
     /// - `Ok(vec![...])` containing a vector of `TableIdent` instances, each
     /// representing a table within the specified namespace.
-    /// - `Err(...)` if an error occurs during namespace validation or while  
+    /// - `Err(...)` if an error occurs during namespace validation or while
     /// querying the database.
     async fn list_tables(&self, namespace: &NamespaceIdent) -> Result<Vec<TableIdent>> {
         let db_name = validate_namespace(namespace)?;
@@ -375,7 +375,7 @@ impl Catalog for GlueCatalog {
     async fn create_table(
         &self,
         namespace: &NamespaceIdent,
-        creation: TableCreation,
+        mut creation: TableCreation,
     ) -> Result<Table> {
         let db_name = validate_namespace(namespace)?;
         let table_name = creation.name.clone();
@@ -387,7 +387,7 @@ impl Catalog for GlueCatalog {
                 get_default_table_location(&ns, &db_name, &table_name, &self.config.warehouse)
             }
         };
-
+        creation.location = Some(location.clone());
         let metadata = TableMetadataBuilder::from_table_creation(creation)?
             .build()?
             .metadata;

crates/catalog/glue/tests/glue_catalog_test.rs

@@ -105,7 +105,7 @@ async fn set_test_namespace(catalog: &GlueCatalog, namespace: &NamespaceIdent) -
     Ok(())
 }
 
-fn set_table_creation(location: impl ToString, name: impl ToString) -> Result<TableCreation> {
+fn set_table_creation(name: impl ToString) -> Result<TableCreation> {
     let schema = Schema::builder()
         .with_schema_id(0)
         .with_fields(vec![
@@ -115,7 +115,6 @@ fn set_table_creation(location: impl ToString, name: impl ToString) -> Result<Ta
         .build()?;
 
     let creation = TableCreation::builder()
-        .location(location.to_string())
         .name(name.to_string())
         .properties(HashMap::new())
         .schema(schema)
@@ -127,7 +126,7 @@ fn set_table_creation(location: impl ToString, name: impl ToString) -> Result<Ta
 #[tokio::test]
 async fn test_rename_table() -> Result<()> {
     let catalog = get_catalog().await;
-    let creation = set_table_creation("s3a://warehouse/hive", "my_table")?;
+    let creation = set_table_creation("my_table")?;
     let namespace = Namespace::new(NamespaceIdent::new("test_rename_table".into()));
 
     catalog
@@ -154,7 +153,7 @@ async fn test_rename_table() -> Result<()> {
 #[tokio::test]
 async fn test_table_exists() -> Result<()> {
     let catalog = get_catalog().await;
-    let creation = set_table_creation("s3a://warehouse/hive", "my_table")?;
+    let creation = set_table_creation("my_table")?;
     let namespace = Namespace::new(NamespaceIdent::new("test_table_exists".into()));
 
     catalog
@@ -178,7 +177,7 @@ async fn test_table_exists() -> Result<()> {
 #[tokio::test]
 async fn test_drop_table() -> Result<()> {
     let catalog = get_catalog().await;
-    let creation = set_table_creation("s3a://warehouse/hive", "my_table")?;
+    let creation = set_table_creation("my_table")?;
     let namespace = Namespace::new(NamespaceIdent::new("test_drop_table".into()));
 
     catalog
@@ -199,7 +198,7 @@ async fn test_drop_table() -> Result<()> {
 #[tokio::test]
 async fn test_load_table() -> Result<()> {
     let catalog = get_catalog().await;
-    let creation = set_table_creation("s3a://warehouse/hive", "my_table")?;
+    let creation = set_table_creation("my_table")?;
     let namespace = Namespace::new(NamespaceIdent::new("test_load_table".into()));
 
     catalog
@@ -227,7 +226,9 @@ async fn test_create_table() -> Result<()> {
     let catalog = get_catalog().await;
     let namespace = NamespaceIdent::new("test_create_table".to_string());
     set_test_namespace(&catalog, &namespace).await?;
-    let creation = set_table_creation("s3a://warehouse/hive", "my_table")?;
+    let mut creation = set_table_creation("my_table")?;
+    // inject custom location, ignore the namespace prefix
+    creation.location = Some("s3a://warehouse/hive".to_string());
 
     let result = catalog.create_table(&namespace, creation).await?;
 

crates/catalog/hms/src/catalog.rs

@@ -338,16 +338,14 @@ impl Catalog for HmsCatalog {
         let db_name = validate_namespace(namespace)?;
         let table_name = creation.name.clone();
 
-        if creation.location.is_none() {
-            let ns = self.get_namespace(namespace).await?;
-            creation.location = Some(get_default_table_location(
-                &ns,
-                &table_name,
-                &self.config.warehouse,
-            ));
-        }
-
-        let location = creation.location.clone().unwrap();
+        let location = match &creation.location {
+            Some(location) => location.clone(),
+            None => {
+                let ns = self.get_namespace(namespace).await?;
+                get_default_table_location(&ns, &table_name, &self.config.warehouse)
+            }
+        };
+        creation.location = Some(location.clone());
         let metadata = TableMetadataBuilder::from_table_creation(creation)?
             .build()?
             .metadata;