[#12996] fix(catalog-glue): Derive table location from the Glue database LocationUri

catalogs/catalog-common/src/main/java/org/apache/gravitino/catalog/glue/GlueConstants.java

@@ -66,8 +66,10 @@ public final class GlueConstants {
 
   /**
    * Base storage path used as a warehouse when no explicit {@code location} is given at table
-   * creation time. The table location is derived as {@code warehouse/database/table}. Example:
-   * {@code s3://my-bucket/gravitino-warehouse}.
+   * creation time and the Glue database declares no {@code LocationUri}. The table location is
+   * derived as {@code warehouse/database/table}.
+   *
+   * <p>Example warehouse: {@code s3://my-bucket/gravitino-warehouse}.
    */
   public static final String WAREHOUSE = "warehouse";
 

catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogOperations.java

@@ -122,7 +122,10 @@ public class GlueCatalogOperations implements CatalogOperations, SupportsSchemas
 
   @VisibleForTesting String defaultTableFormat;
 
-  /** Warehouse storage path. Table location is derived as {@code warehouse/db/table}. */
+  /**
+   * Warehouse storage path. Table location is derived as {@code warehouse/db/table} for databases
+   * that declare no {@code LocationUri}.
+   */
   @VisibleForTesting String warehouseLocation;
 
   /** Iceberg SDK Glue catalog used for creating Iceberg-format tables. */
@@ -824,16 +827,23 @@ private software.amazon.awssdk.services.glue.model.Column toGlueColumn(Column co
         .build();
   }
 
+  /**
+   * Resolves the storage location of a table, in order of precedence: the explicit {@code location}
+   * property, the {@code LocationUri} the Glue database declares, and finally the catalog warehouse
+   * path. A database location already identifies the database, so the table name is appended
+   * directly to it, whereas the warehouse path is shared by all databases and needs the database
+   * name in between.
+   */
   private String resolveTableLocation(String explicitLocation, String dbName, String tableName) {
     if (explicitLocation != null) {
       return explicitLocation;
     }
+    String databaseLocation = databaseLocationUri(dbName);
+    if (StringUtils.isNotBlank(databaseLocation)) {
+      return StringUtils.stripEnd(databaseLocation, "/") + "/" + tableName;
+    }
     if (StringUtils.isNotBlank(warehouseLocation)) {
-      String base =
-          warehouseLocation.endsWith("/")
-              ? warehouseLocation.substring(0, warehouseLocation.length() - 1)
-              : warehouseLocation;
-      return base + "/" + dbName + "/" + tableName;
+      return StringUtils.stripEnd(warehouseLocation, "/") + "/" + dbName + "/" + tableName;
     }
     throw new IllegalArgumentException(
         "Table location is required: either set the '"
@@ -843,6 +853,17 @@ private String resolveTableLocation(String explicitLocation, String dbName, Stri
             + "' on the catalog.");
   }
 
+  /** Returns the {@code LocationUri} of the given Glue database, or null when it declares none. */
+  private String databaseLocationUri(String dbName) {
+    GetDatabaseRequest.Builder req = GetDatabaseRequest.builder().name(dbName);
+    applyCatalogId(catalogId, req::catalogId);
+    try {
+      return glueClient.getDatabase(req.build()).database().locationUri();
+    } catch (GlueException e) {
+      throw GlueExceptionConverter.toSchemaException(e, "schema " + dbName);
+    }
+  }
+
   private static void applyColumnChange(
       List<Column> dataCols, List<Column> partCols, TableChange.ColumnChange change) {
 

catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogPropertiesMetadata.java

@@ -105,8 +105,9 @@ public class GlueCatalogPropertiesMetadata extends BaseCatalogPropertiesMetadata
               stringRequiredPropertyEntry(
                   WAREHOUSE,
                   "Base storage path used as warehouse when no explicit location is set"
-                      + " at table creation time (e.g. s3://my-bucket/warehouse)."
-                      + " Table location is derived as warehouse/database/table.",
+                      + " at table creation time and the database declares no LocationUri"
+                      + " (e.g. s3://my-bucket/warehouse)."
+                      + " Table location is then derived as warehouse/database/table.",
                   false /* immutable */,
                   false /* hidden */))
           .build();

catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogOperationsForIceberg.java

@@ -47,6 +47,9 @@
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mockito;
 import software.amazon.awssdk.services.glue.GlueClient;
+import software.amazon.awssdk.services.glue.model.Database;
+import software.amazon.awssdk.services.glue.model.GetDatabaseRequest;
+import software.amazon.awssdk.services.glue.model.GetDatabaseResponse;
 import software.amazon.awssdk.services.glue.model.GetPartitionsRequest;
 import software.amazon.awssdk.services.glue.model.GetPartitionsResponse;
 import software.amazon.awssdk.services.glue.model.GetTableRequest;
@@ -193,6 +196,10 @@ void testCreateTable_icebergMissingLocationThrows() {
     GlueColumn[] cols = {
       GlueColumn.builder().withName("id").withType(Types.LongType.get()).withNullable(false).build()
     };
+    // The database declares no location either, so there is nothing to derive the location from.
+    when(mockClient.getDatabase(any(GetDatabaseRequest.class)))
+        .thenReturn(
+            GetDatabaseResponse.builder().database(Database.builder().name(DB).build()).build());
 
     assertThrows(
         IllegalArgumentException.class,

catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogTableOperations.java

@@ -24,6 +24,7 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -52,8 +53,11 @@
 import software.amazon.awssdk.services.glue.GlueClient;
 import software.amazon.awssdk.services.glue.model.AlreadyExistsException;
 import software.amazon.awssdk.services.glue.model.CreateTableRequest;
+import software.amazon.awssdk.services.glue.model.Database;
 import software.amazon.awssdk.services.glue.model.DeleteTableRequest;
 import software.amazon.awssdk.services.glue.model.EntityNotFoundException;
+import software.amazon.awssdk.services.glue.model.GetDatabaseRequest;
+import software.amazon.awssdk.services.glue.model.GetDatabaseResponse;
 import software.amazon.awssdk.services.glue.model.GetTableRequest;
 import software.amazon.awssdk.services.glue.model.GetTableResponse;
 import software.amazon.awssdk.services.glue.model.GetTablesRequest;
@@ -74,6 +78,14 @@ void setup() {
     ops = new GlueCatalogOperations();
     ops.glueClient = mockClient;
     ops.warehouseLocation = "s3://test-bucket/warehouse";
+    stubDatabaseLocation(null);
+  }
+
+  /** Stubs the database lookup used to resolve table locations. */
+  private void stubDatabaseLocation(String locationUri) {
+    Database database = Database.builder().name("mydb").locationUri(locationUri).build();
+    when(mockClient.getDatabase(any(GetDatabaseRequest.class)))
+        .thenReturn(GetDatabaseResponse.builder().database(database).build());
   }
 
   // -------------------------------------------------------------------------
@@ -274,6 +286,163 @@ void testCreateTableStorageDescriptorProperties() {
     assertFalse(req.tableInput().parameters().containsKey(GlueConstants.LOCATION));
   }
 
+  @Test
+  void testCreateTableLocationFromDatabaseLocationUri() {
+    stubDatabaseLocation("s3://test-bucket/gravprobe");
+    NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb", "mytable");
+
+    ArgumentCaptor<CreateTableRequest> captor = ArgumentCaptor.forClass(CreateTableRequest.class);
+
+    ops.createTable(
+        ident,
+        new Column[0],
+        "comment",
+        Collections.emptyMap(),
+        Transforms.EMPTY_TRANSFORM,
+        Distributions.NONE,
+        SortOrders.NONE,
+        Indexes.EMPTY_INDEXES);
+
+    verify(mockClient).createTable(captor.capture());
+    assertEquals(
+        "s3://test-bucket/gravprobe/mytable",
+        captor.getValue().tableInput().storageDescriptor().location());
+  }
+
+  @Test
+  void testCreateTableLocationFromDatabaseLocationUriWithTrailingSlashes() {
+    stubDatabaseLocation("s3://test-bucket/gravprobe///");
+    NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb", "mytable");
+
+    ArgumentCaptor<CreateTableRequest> captor = ArgumentCaptor.forClass(CreateTableRequest.class);
+
+    ops.createTable(
+        ident,
+        new Column[0],
+        "comment",
+        Collections.emptyMap(),
+        Transforms.EMPTY_TRANSFORM,
+        Distributions.NONE,
+        SortOrders.NONE,
+        Indexes.EMPTY_INDEXES);
+
+    verify(mockClient).createTable(captor.capture());
+    assertEquals(
+        "s3://test-bucket/gravprobe/mytable",
+        captor.getValue().tableInput().storageDescriptor().location());
+  }
+
+  @Test
+  void testCreateTableLocationWhenWarehouseEqualsDatabaseLocation() {
+    ops.warehouseLocation = "s3://test-bucket/gravprobe";
+    stubDatabaseLocation("s3://test-bucket/gravprobe");
+    NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb", "mytable");
+
+    ArgumentCaptor<CreateTableRequest> captor = ArgumentCaptor.forClass(CreateTableRequest.class);
+
+    ops.createTable(
+        ident,
+        new Column[0],
+        "comment",
+        Collections.emptyMap(),
+        Transforms.EMPTY_TRANSFORM,
+        Distributions.NONE,
+        SortOrders.NONE,
+        Indexes.EMPTY_INDEXES);
+
+    verify(mockClient).createTable(captor.capture());
+    assertEquals(
+        "s3://test-bucket/gravprobe/mytable",
+        captor.getValue().tableInput().storageDescriptor().location());
+  }
+
+  @Test
+  void testCreateTableLocationFallsBackToWarehouse() {
+    NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb", "mytable");
+
+    ArgumentCaptor<CreateTableRequest> captor = ArgumentCaptor.forClass(CreateTableRequest.class);
+
+    ops.createTable(
+        ident,
+        new Column[0],
+        "comment",
+        Collections.emptyMap(),
+        Transforms.EMPTY_TRANSFORM,
+        Distributions.NONE,
+        SortOrders.NONE,
+        Indexes.EMPTY_INDEXES);
+
+    verify(mockClient).createTable(captor.capture());
+    assertEquals(
+        "s3://test-bucket/warehouse/mydb/mytable",
+        captor.getValue().tableInput().storageDescriptor().location());
+  }
+
+  @Test
+  void testCreateTableLocationFromWarehouseWithTrailingSlashes() {
+    ops.warehouseLocation = "s3://test-bucket/warehouse///";
+    NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb", "mytable");
+
+    ArgumentCaptor<CreateTableRequest> captor = ArgumentCaptor.forClass(CreateTableRequest.class);
+
+    ops.createTable(
+        ident,
+        new Column[0],
+        "comment",
+        Collections.emptyMap(),
+        Transforms.EMPTY_TRANSFORM,
+        Distributions.NONE,
+        SortOrders.NONE,
+        Indexes.EMPTY_INDEXES);
+
+    verify(mockClient).createTable(captor.capture());
+    assertEquals(
+        "s3://test-bucket/warehouse/mydb/mytable",
+        captor.getValue().tableInput().storageDescriptor().location());
+  }
+
+  @Test
+  void testCreateTableExplicitLocationWinsOverDatabaseLocationUri() {
+    stubDatabaseLocation("s3://test-bucket/gravprobe");
+    NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb", "mytable");
+
+    ArgumentCaptor<CreateTableRequest> captor = ArgumentCaptor.forClass(CreateTableRequest.class);
+
+    ops.createTable(
+        ident,
+        new Column[0],
+        "comment",
+        Map.of(GlueConstants.LOCATION, "s3://my-bucket/path"),
+        Transforms.EMPTY_TRANSFORM,
+        Distributions.NONE,
+        SortOrders.NONE,
+        Indexes.EMPTY_INDEXES);
+
+    verify(mockClient).createTable(captor.capture());
+    assertEquals(
+        "s3://my-bucket/path", captor.getValue().tableInput().storageDescriptor().location());
+  }
+
+  @Test
+  void testCreateTableSchemaNotFoundWhileResolvingLocation() {
+    NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "missing", "mytable");
+    when(mockClient.getDatabase(any(GetDatabaseRequest.class)))
+        .thenThrow(EntityNotFoundException.builder().message("not found").build());
+
+    assertThrows(
+        NoSuchSchemaException.class,
+        () ->
+            ops.createTable(
+                ident,
+                new Column[0],
+                "comment",
+                Collections.emptyMap(),
+                Transforms.EMPTY_TRANSFORM,
+                Distributions.NONE,
+                SortOrders.NONE,
+                Indexes.EMPTY_INDEXES));
+  }
+
   // -------------------------------------------------------------------------
   // alterTable
   // -------------------------------------------------------------------------
@@ -299,6 +468,32 @@ void testAlterTableRenameAndComment() {
     assertEquals("new comment", result.comment());
   }
 
+  @Test
+  void testAlterTableKeepsExistingLocation() {
+    stubDatabaseLocation("s3://test-bucket/gravprobe");
+    NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb", "t");
+    Table glueTable =
+        Table.builder()
+            .name("t")
+            .storageDescriptor(
+                StorageDescriptor.builder().location("s3://other-bucket/existing/t").build())
+            .createTime(Instant.now())
+            .build();
+    when(mockClient.getTable(any(GetTableRequest.class)))
+        .thenReturn(GetTableResponse.builder().table(glueTable).build());
+    when(mockClient.updateTable(any(UpdateTableRequest.class)))
+        .thenReturn(UpdateTableResponse.builder().build());
+
+    ArgumentCaptor<UpdateTableRequest> captor = ArgumentCaptor.forClass(UpdateTableRequest.class);
+    ops.alterTable(ident, TableChange.updateComment("new comment"));
+
+    verify(mockClient).updateTable(captor.capture());
+    assertEquals(
+        "s3://other-bucket/existing/t",
+        captor.getValue().tableInput().storageDescriptor().location());
+    verify(mockClient, never()).getDatabase(any(GetDatabaseRequest.class));
+  }
+
   @Test
   void testAlterTableSetProperty() {
     NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb", "t");

docs/aws-glue-catalog.md

@@ -42,7 +42,7 @@ Besides the [common catalog properties](./gravitino-server-config.md#catalog-pro
 | `aws-access-key-id`     | AWS access key ID for static credential authentication. When omitted, the default credential chain is used.                                                                                         | (none)        | No       | No        |
 | `aws-secret-access-key` | AWS secret access key paired with `aws-access-key-id`. When omitted, the default credential chain is used.                                                                                          | (none)        | No       | No        |
 | `aws-glue-endpoint`     | Custom Glue endpoint URL for VPC endpoints or LocalStack testing (e.g. `http://localhost:4566`).                                                                                                    | (none)        | No       | No        |
-| `warehouse`             | Base storage path used as the warehouse when no explicit `location` is specified at table creation time (e.g. `s3://my-bucket/warehouse`). Table location is derived as `warehouse/database/table`. | (none)        | Yes      | No        |
+| `warehouse`             | Base storage path used as the warehouse when no explicit `location` is specified at table creation time and the Glue database declares no `LocationUri` (e.g. `s3://my-bucket/warehouse`). Table location is then derived as `warehouse/database/table`. | (none)        | Yes      | No        |
 | `default-table-format`  | Default format for tables created via Gravitino's `createTable()` API. Accepted values: `iceberg`, `hive`.                                                                                          | `hive`        | No       | No        |
 | `table-format-filter`   | Comma-separated list of table formats exposed by `listTables()` and `loadTable()`. Accepted values: `all`, `hive`, `iceberg`, `delta`, `parquet`. Use to restrict visible table types.              | `all`         | No       | No        |
 
@@ -148,7 +148,7 @@ The following table lists predefined properties for Glue tables. Additional key-
 
 | Property Name       | Description                                                                                                                                                                                                  | Default Value                                                | Required | Reserved | Immutable |
 |---------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|--------------------------------------------------------------|----------|----------|-----------|
-| `location`          | The location for table storage, such as `s3://bucket/prefix/test_table`. Derived from `warehouse/database/table` when not specified.                                                                         | (derived from warehouse)                                     | No       | No       | No        |
+| `location`          | The location for table storage, such as `s3://bucket/prefix/test_table`. When not specified, it is derived from the `LocationUri` of the Glue database as `database-location/table`, falling back to `warehouse/database/table` when the database declares no location.                                                                         | (derived from database location or warehouse)                | No       | No       | No        |
 | `format`            | The table file format (`parquet`, `orc`, `textfile`, etc.). When set, `input-format`, `output-format`, and `serde-lib` are derived automatically. Used primarily when creating Hive-format tables via Trino. | (none)                                                       | No       | No       | Yes       |
 | `input-format`      | The input format class for the table, such as `org.apache.hadoop.hive.ql.io.orc.OrcInputFormat`.                                                                                                             | `org.apache.hadoop.mapred.TextInputFormat`                   | No       | No       | Yes       |
 | `output-format`     | The output format class for the table, such as `org.apache.hadoop.hive.ql.io.orc.OrcOutputFormat`.                                                                                                           | `org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat` | No       | No       | Yes       |
@@ -214,7 +214,7 @@ The Glue catalog supports creating and managing Iceberg-format tables through th
 
 Set `table-format=ICEBERG` in the table properties, or configure `default-table-format=iceberg` on the catalog to make all tables Iceberg by default.
 
-The `warehouse` catalog property must be configured. The table location is derived as `warehouse/database/table` when no explicit `location` is specified.
+The `warehouse` catalog property is required, but it is used only as a fallback when no explicit `location` is specified and the Glue database declares no `LocationUri`. Without an explicit `location`, the table location is derived as `database-location/table` when the database declares a `LocationUri`, or as `warehouse/database/table` otherwise.
 
 ### Register an Existing Iceberg Table