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 417b582a76 [Cherry-pick to branch-1.3] [#13010] fix(catalog-glue): Do
not expose Glue VIRTUAL_VIEW objects as tables (#13011) (#13039)
417b582a76 is described below
commit 417b582a7663a5af7ad56559e883a684c3e26777
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Sep 10 09:10:51 2026 +0800
[Cherry-pick to branch-1.3] [#13010] fix(catalog-glue): Do not expose Glue
VIRTUAL_VIEW objects as tables (#13011) (#13039)
**Cherry-pick Information:**
- Original commit: d648735761c3b46eb6c1c5d0f334cb293969edd0
- 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]>
Co-authored-by: diqiu50 <[email protected]>
---
.../gravitino/catalog/glue/GlueConstants.java | 7 ++
.../catalog/glue/GlueCatalogOperations.java | 37 ++++++++-
.../glue/TestGlueCatalogOperationsForIceberg.java | 39 ++++++++++
.../glue/TestGlueCatalogTableOperations.java | 86 +++++++++++++++++++++
.../integration/test/AbstractGlueCatalogIT.java | 87 ++++++++++++++++++++++
5 files changed, 255 insertions(+), 1 deletion(-)
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 da78dfe24c..2cd1f32cf3 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
@@ -148,5 +148,12 @@ public final class GlueConstants {
/** Glue {@code tableType} value for external tables. */
public static final String EXTERNAL_TABLE_TYPE = "EXTERNAL_TABLE";
+ /**
+ * Glue {@code tableType} value for views. Both Hive-compatible views (whose
definition lives in
+ * {@code Table.viewOriginalText()}) and Glue multi-dialect views (whose
definition lives in
+ * {@code Table.viewDefinition()}) carry this type.
+ */
+ public static final String VIRTUAL_VIEW_TABLE_TYPE = "VIRTUAL_VIEW";
+
private GlueConstants() {}
}
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 67a8a80738..99f205e203 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
@@ -346,6 +346,7 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
if (nextToken != null) req.nextToken(nextToken);
GetTablesResponse resp = glueClient.getTables(req.build());
resp.tableList().stream()
+ .filter(t -> !isView(t))
.filter(this::matchesFormatFilter)
.map(t -> NameIdentifier.of(namespace, t.name()))
.forEach(result::add);
@@ -367,6 +368,7 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
try {
software.amazon.awssdk.services.glue.model.Table rawGlueTable =
glueClient.getTable(req.build()).table();
+ rejectIfView(rawGlueTable, ident, dbName);
GlueTable table = GlueTable.fromGlueTable(rawGlueTable, typeConverter);
// Recover Iceberg-specific partitioning and sort orders from the
Iceberg metadata.
@@ -509,6 +511,8 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
throw GlueExceptionConverter.toTableException(e, "table " +
ident.name());
}
+ rejectIfView(rawGlueTable, ident, dbName);
+
if (GlueIcebergTableHelper.isIcebergTable(rawGlueTable)) {
return alterIcebergTable(ident, dbName, rawGlueTable, changes);
}
@@ -588,7 +592,7 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
&&
rawGlueTable.parameters().containsKey(GlueConstants.METADATA_LOCATION);
boolean isSdkManaged =
rawGlueTable.hasParameters()
- && GlueConstants.ICEBERG_TABLE_TYPE_VALUE.equals(
+ && GlueConstants.ICEBERG_TABLE_TYPE_VALUE.equalsIgnoreCase(
rawGlueTable.parameters().get(GlueConstants.TABLE_TYPE_PARAM));
if (hasMetadataLocation && !isSdkManaged) {
return alterRegisterModeIcebergTable(ident, dbName, rawGlueTable,
changes);
@@ -641,6 +645,20 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
@Override
public boolean dropTable(NameIdentifier ident) {
String dbName = schemaName(ident.namespace());
+
+ // Glue stores views as objects of type VIRTUAL_VIEW in the same namespace
as tables, so the
+ // object has to be fetched first to avoid dropping a view through the
table API.
+ GetTableRequest.Builder getReq =
+ GetTableRequest.builder().databaseName(dbName).name(ident.name());
+ applyCatalogId(catalogId, getReq::catalogId);
+ try {
+ rejectIfView(glueClient.getTable(getReq.build()).table(), ident, dbName);
+ } catch (EntityNotFoundException e) {
+ return false;
+ } catch (GlueException e) {
+ throw GlueExceptionConverter.toTableException(e, "table " +
ident.name());
+ }
+
DeleteTableRequest.Builder req =
DeleteTableRequest.builder().databaseName(dbName).name(ident.name());
applyCatalogId(catalogId, req::catalogId);
@@ -662,6 +680,23 @@ public class GlueCatalogOperations implements
CatalogOperations, SupportsSchemas
return levels[levels.length - 1];
}
+ /**
+ * Returns whether the Glue object is a view rather than a table. Glue keeps
views and tables in
+ * the same namespace and returns both from the table APIs, so every table
entry point has to
+ * screen them out.
+ */
+ private static boolean isView(Table table) {
+ return
GlueConstants.VIRTUAL_VIEW_TABLE_TYPE.equalsIgnoreCase(table.tableType());
+ }
+
+ /** Rejects a Glue object that is a view, so that it is not acted on through
the table API. */
+ private static void rejectIfView(Table table, NameIdentifier ident, String
dbName) {
+ if (isView(table)) {
+ throw new NoSuchTableException(
+ "No table named %s in schema %s (it is a view, not a table)",
ident.name(), dbName);
+ }
+ }
+
// NOTE: parameter type is the Glue SDK Table, not GlueTable (our domain
class).
// The Glue SDK's Column model is also referenced by FQN throughout this
class because its
// simple name conflicts with the imported org.apache.gravitino.rel.Column.
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 3fe0f11d96..96e950c3f2 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
@@ -26,6 +26,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;
@@ -55,6 +56,7 @@ import
software.amazon.awssdk.services.glue.model.GetPartitionsResponse;
import software.amazon.awssdk.services.glue.model.GetTableRequest;
import software.amazon.awssdk.services.glue.model.GetTableResponse;
import software.amazon.awssdk.services.glue.model.StorageDescriptor;
+import software.amazon.awssdk.services.glue.model.UpdateTableRequest;
class TestGlueCatalogOperationsForIceberg {
@@ -312,6 +314,43 @@ class TestGlueCatalogOperationsForIceberg {
verify(mockBuilder).create();
}
+ /**
+ * Glue stores table_type as a free-form string and different writers use
different casing, so the
+ * register-mode check must not treat a lower-case "iceberg" as an
externally registered table.
+ */
+ @Test
+ void testAlterIcebergTableWithLowerCaseTableTypeRoutesToIcebergSdk() {
+ String newName = "ice1_renamed";
+ software.amazon.awssdk.services.glue.model.Table rawTable =
+ software.amazon.awssdk.services.glue.model.Table.builder()
+ .name(TABLE)
+ .parameters(Map.of(TABLE_TYPE_PARAM, "iceberg",
"metadata_location", LOCATION))
+ .storageDescriptor(StorageDescriptor.builder().build())
+ .build();
+ software.amazon.awssdk.services.glue.model.Table renamedTable =
+ software.amazon.awssdk.services.glue.model.Table.builder()
+ .name(newName)
+ .parameters(Map.of(TABLE_TYPE_PARAM, "iceberg"))
+ .storageDescriptor(StorageDescriptor.builder().build())
+ .build();
+
+ when(mockClient.getTable(any(GetTableRequest.class)))
+ .thenReturn(GetTableResponse.builder().table(rawTable).build())
+ .thenReturn(GetTableResponse.builder().table(renamedTable).build());
+ when(mockIcebergCatalog.loadTable(TableIdentifier.of(DB, newName)))
+ .thenThrow(new RuntimeException("no iceberg metadata"));
+ when(mockClient.getPartitions(any(GetPartitionsRequest.class)))
+ .thenReturn(GetPartitionsResponse.builder().build());
+
+ NameIdentifier ident = NameIdentifier.of("cat", "ns", DB, TABLE);
+ GlueTable result = ops.alterTable(ident, TableChange.rename(newName));
+
+ verify(mockIcebergCatalog)
+ .renameTable(TableIdentifier.of(DB, TABLE), TableIdentifier.of(DB,
newName));
+ verify(mockClient, never()).updateTable(any(UpdateTableRequest.class));
+ assertEquals(newName, result.name());
+ }
+
@Test
void testMatchesFormatFilter_icebergFallbackViaTableType() {
// Table has table_type=ICEBERG but no table-format property (e.g. created
by external tooling)
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 d2da93aafc..ffc9babbb7 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
@@ -563,6 +563,7 @@ class TestGlueCatalogTableOperations {
@Test
void testDropTableSuccess() {
NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb",
"t");
+
stubGetTable(Table.builder().name("t").tableType(GlueConstants.EXTERNAL_TABLE_TYPE).build());
boolean result = ops.dropTable(ident);
@@ -573,6 +574,18 @@ class TestGlueCatalogTableOperations {
@Test
void testDropTableNotFound() {
NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb",
"missing");
+ when(mockClient.getTable(any(GetTableRequest.class)))
+ .thenThrow(EntityNotFoundException.builder().message("not
found").build());
+
+ assertFalse(ops.dropTable(ident));
+ }
+
+ /** The table is dropped by another client between the type check and the
delete. */
+ @Test
+ void testDropTableVanishesBeforeDelete() {
+ NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb",
"missing");
+ stubGetTable(
+
Table.builder().name("missing").tableType(GlueConstants.EXTERNAL_TABLE_TYPE).build());
when(mockClient.deleteTable(any(DeleteTableRequest.class)))
.thenThrow(EntityNotFoundException.builder().message("not
found").build());
@@ -583,6 +596,7 @@ class TestGlueCatalogTableOperations {
void testDropTableWithCatalogId() {
ops.catalogId = "123456789012";
NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb",
"t");
+
stubGetTable(Table.builder().name("t").tableType(GlueConstants.EXTERNAL_TABLE_TYPE).build());
ArgumentCaptor<DeleteTableRequest> captor =
ArgumentCaptor.forClass(DeleteTableRequest.class);
ops.dropTable(ident);
@@ -592,4 +606,76 @@ class TestGlueCatalogTableOperations {
assertEquals("mydb", captor.getValue().databaseName());
assertEquals("t", captor.getValue().name());
}
+
+ // -------------------------------------------------------------------------
+ // views are not tables
+ // -------------------------------------------------------------------------
+
+ @Test
+ void testListTablesExcludesViews() {
+ Namespace ns = Namespace.of("metalake", "catalog", "mydb");
+ Table table = Table.builder().name("iceberg_test").build();
+
+ when(mockClient.getTables(any(GetTablesRequest.class)))
+ .thenReturn(
+ GetTablesResponse.builder()
+ .tableList(table, viewObject("iceberg_view"))
+ .nextToken(null)
+ .build());
+
+ NameIdentifier[] result = ops.listTables(ns);
+
+ assertEquals(1, result.length);
+ assertEquals("iceberg_test", result[0].name());
+ }
+
+ @Test
+ void testLoadTableRejectsView() {
+ NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb",
"iceberg_view");
+ stubGetTable(viewObject("iceberg_view"));
+
+ NoSuchTableException e = assertThrows(NoSuchTableException.class, () ->
ops.loadTable(ident));
+ assertTrue(e.getMessage().contains("it is a view, not a table"));
+ }
+
+ @Test
+ void testAlterTableRejectsView() {
+ NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb",
"iceberg_view");
+ stubGetTable(viewObject("iceberg_view"));
+
+ assertThrows(
+ NoSuchTableException.class, () -> ops.alterTable(ident,
TableChange.updateComment("x")));
+ }
+
+ @Test
+ void testDropTableRejectsView() {
+ NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb",
"iceberg_view");
+ stubGetTable(viewObject("iceberg_view"));
+
+ assertThrows(NoSuchTableException.class, () -> ops.dropTable(ident));
+ verify(mockClient, never()).deleteTable(any(DeleteTableRequest.class));
+ }
+
+ /** Glue stores tableType as a free-form string; casing must not decide
whether it is a view. */
+ @Test
+ void testViewDetectionIsCaseInsensitive() {
+ NameIdentifier ident = NameIdentifier.of("metalake", "catalog", "mydb",
"v");
+ stubGetTable(Table.builder().name("v").tableType("virtual_view").build());
+
+ assertThrows(NoSuchTableException.class, () -> ops.loadTable(ident));
+ }
+
+ private void stubGetTable(Table table) {
+ when(mockClient.getTable(any(GetTableRequest.class)))
+ .thenReturn(GetTableResponse.builder().table(table).build());
+ }
+
+ private static Table viewObject(String name) {
+ return Table.builder()
+ .name(name)
+ .tableType(GlueConstants.VIRTUAL_VIEW_TABLE_TYPE)
+ .viewOriginalText("/* Presto View: abc */")
+ .parameters(Map.of("presto_view", "true"))
+ .build();
+ }
}
diff --git
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/integration/test/AbstractGlueCatalogIT.java
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/integration/test/AbstractGlueCatalogIT.java
index 946502567a..55e0164b57 100644
---
a/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/integration/test/AbstractGlueCatalogIT.java
+++
b/catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/integration/test/AbstractGlueCatalogIT.java
@@ -36,8 +36,10 @@ import org.apache.gravitino.Namespace;
import org.apache.gravitino.Schema;
import org.apache.gravitino.SchemaChange;
import org.apache.gravitino.catalog.glue.GlueCatalogOperations;
+import org.apache.gravitino.catalog.glue.GlueClientProvider;
import org.apache.gravitino.catalog.glue.GlueConstants;
import org.apache.gravitino.catalog.hive.HiveStorageConstants;
+import org.apache.gravitino.exceptions.NoSuchTableException;
import org.apache.gravitino.exceptions.NonEmptySchemaException;
import org.apache.gravitino.rel.Column;
import org.apache.gravitino.rel.SupportsPartitions;
@@ -61,6 +63,11 @@ import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
+import software.amazon.awssdk.services.glue.GlueClient;
+import software.amazon.awssdk.services.glue.model.CreateTableRequest;
+import software.amazon.awssdk.services.glue.model.GetTableRequest;
+import software.amazon.awssdk.services.glue.model.StorageDescriptor;
+import software.amazon.awssdk.services.glue.model.TableInput;
/**
* Abstract base class for Glue catalog integration tests.
@@ -73,6 +80,7 @@ abstract class AbstractGlueCatalogIT {
protected GlueCatalogOperations ops;
private String currentSchema;
+ private GlueClient rawGlueClient;
private static final Namespace SCHEMA_NS = Namespace.of("ml", "cat");
@@ -86,6 +94,9 @@ abstract class AbstractGlueCatalogIT {
@AfterAll
void closeOps() throws Exception {
+ if (rawGlueClient != null) {
+ rawGlueClient.close();
+ }
if (ops != null) {
ops.close();
}
@@ -173,6 +184,38 @@ abstract class AbstractGlueCatalogIT {
return ops.loadTable(tableIdent(schema, table)).supportPartitions();
}
+ private GlueClient glueClient() {
+ if (rawGlueClient == null) {
+ rawGlueClient = GlueClientProvider.buildClient(catalogConfig());
+ }
+ return rawGlueClient;
+ }
+
+ /** Creates a Presto/Athena style view directly through the Glue API. */
+ private void createGlueView(String schema, String name) {
+ glueClient()
+ .createTable(
+ CreateTableRequest.builder()
+ .databaseName(schema)
+ .tableInput(
+ TableInput.builder()
+ .name(name)
+ .tableType(GlueConstants.VIRTUAL_VIEW_TABLE_TYPE)
+ .viewOriginalText("/* Presto View: dGVzdA== */")
+ .viewExpandedText("/* Presto View */")
+ .parameters(Map.of("presto_view", "true"))
+ .storageDescriptor(
+ StorageDescriptor.builder()
+ .columns(
+
software.amazon.awssdk.services.glue.model.Column.builder()
+ .name("id")
+ .type("int")
+ .build())
+ .build())
+ .build())
+ .build());
+ }
+
private Partition identityPartition(String dateValue) {
return Partitions.identity(
"dt=" + dateValue,
@@ -332,6 +375,50 @@ abstract class AbstractGlueCatalogIT {
assertFalse(ops.dropTable(tableIdent(schema, "droptbl")));
}
+ // -------------------------------------------------------------------------
+ // View tests: Glue keeps views alongside tables, the table APIs must skip
them
+ // -------------------------------------------------------------------------
+
+ @Test
+ void testListTablesExcludesViews() {
+ String schema = newSchema();
+ ops.createSchema(schemaIdent(schema), null, Collections.emptyMap());
+ createHiveTable(schema, "base_tbl");
+ createGlueView(schema, "a_view");
+
+ List<String> names =
+ Arrays.stream(ops.listTables(tableNs(schema)))
+ .map(NameIdentifier::name)
+ .collect(Collectors.toList());
+ assertTrue(names.contains("base_tbl"));
+ assertFalse(names.contains("a_view"));
+ }
+
+ @Test
+ void testLoadTableRejectsView() {
+ String schema = newSchema();
+ ops.createSchema(schemaIdent(schema), null, Collections.emptyMap());
+ createGlueView(schema, "load_view");
+
+ assertThrows(NoSuchTableException.class, () ->
ops.loadTable(tableIdent(schema, "load_view")));
+ }
+
+ @Test
+ void testDropTableRejectsView() {
+ String schema = newSchema();
+ ops.createSchema(schemaIdent(schema), null, Collections.emptyMap());
+ createGlueView(schema, "drop_view");
+
+ assertThrows(NoSuchTableException.class, () ->
ops.dropTable(tableIdent(schema, "drop_view")));
+ assertEquals(
+ GlueConstants.VIRTUAL_VIEW_TABLE_TYPE,
+ glueClient()
+
.getTable(GetTableRequest.builder().databaseName(schema).name("drop_view").build())
+ .table()
+ .tableType(),
+ "the view must survive the rejected drop");
+ }
+
// -------------------------------------------------------------------------
// Iceberg table tests
// -------------------------------------------------------------------------
