This is an automated email from the ASF dual-hosted git repository.
diqiu50 pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new b2b15b65b3 [Cherry-pick to branch-1.3] [#12996] fix(catalog-glue):
Derive table location from the Glue database LocationUri (#12997) (#13038)
b2b15b65b3 is described below
commit b2b15b65b38ce9642d6456360edf78e0ee08e413
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Sep 10 09:10:21 2026 +0800
[Cherry-pick to branch-1.3] [#12996] fix(catalog-glue): Derive table
location from the Glue database LocationUri (#12997) (#13038)
**Cherry-pick Information:**
- Original commit: 0e38553643676c3cbd38f39d7392d9af09f8e76e
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)
Co-authored-by: Yuhui <[email protected]>
Co-authored-by: Claude Opus 5 <[email protected]>
---
.../gravitino/catalog/glue/GlueConstants.java | 6 +-
.../catalog/glue/GlueCatalogOperations.java | 33 +++-
.../glue/GlueCatalogPropertiesMetadata.java | 5 +-
.../glue/TestGlueCatalogOperationsForIceberg.java | 7 +
.../glue/TestGlueCatalogTableOperations.java | 195 +++++++++++++++++++++
docs/aws-glue-catalog.md | 6 +-
6 files changed, 239 insertions(+), 13 deletions(-)
diff --git
a/catalogs/catalog-common/src/main/java/org/apache/gravitino/catalog/glue/GlueConstants.java
b/catalogs/catalog-common/src/main/java/org/apache/gravitino/catalog/glue/GlueConstants.java
index 76de4483d5..da78dfe24c 100644
---
a/catalogs/catalog-common/src/main/java/org/apache/gravitino/catalog/glue/GlueConstants.java
+++
b/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";
diff --git
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogOperations.java
b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogOperations.java
index e7ce0ea8b0..67a8a80738 100644
---
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogOperations.java
+++
b/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 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
.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 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
+ "' 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) {
diff --git
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogPropertiesMetadata.java
b/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogPropertiesMetadata.java
index 47e5bf31c6..7275861ab7 100644
---
a/catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogPropertiesMetadata.java
+++
b/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();
diff --git
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogOperationsForIceberg.java
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogOperationsForIceberg.java
index 8970b74a4a..3fe0f11d96 100644
---
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogOperationsForIceberg.java
+++
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogOperationsForIceberg.java
@@ -47,6 +47,9 @@ import org.junit.jupiter.api.Test;
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 @@ class TestGlueCatalogOperationsForIceberg {
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,
diff --git
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogTableOperations.java
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogTableOperations.java
index b8ed3206e8..d2da93aafc 100644
---
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogTableOperations.java
+++
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/TestGlueCatalogTableOperations.java
@@ -24,6 +24,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
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 org.mockito.ArgumentCaptor;
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 @@ class TestGlueCatalogTableOperations {
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 @@ class TestGlueCatalogTableOperations {
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 @@ class TestGlueCatalogTableOperations {
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");
diff --git a/docs/aws-glue-catalog.md b/docs/aws-glue-catalog.md
index 590f5bae2e..6e1b7fb70a 100644
--- a/docs/aws-glue-catalog.md
+++ b/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
