Copilot commented on code in PR #13011:
URL: https://github.com/apache/gravitino/pull/13011#discussion_r3958697103
##########
catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogOperations.java:
##########
@@ -364,6 +365,10 @@ public GlueTable loadTable(NameIdentifier ident) throws
NoSuchTableException {
try {
software.amazon.awssdk.services.glue.model.Table rawGlueTable =
glueClient.getTable(req.build()).table();
+ if (isView(rawGlueTable)) {
+ throw new NoSuchTableException(
+ "No table named %s in schema %s (it is a view, not a table)",
ident.name(), dbName);
+ }
Review Comment:
The same view-rejection logic and error message is duplicated in
`loadTable()`, `alterTable()`, and `dropTable()`. Consider extracting a small
helper (e.g., `rejectIfView(Table, NameIdentifier, String dbName)`) to
centralize the condition and message, reducing the chance of future
inconsistencies if the wording/behavior changes.
##########
catalogs/catalog-glue/src/test/java/org/apache/gravitino/catalog/glue/integration/test/AbstractGlueCatalogIT.java:
##########
@@ -173,6 +185,38 @@ private SupportsPartitions createPartitionedTable(String
schema, String table) {
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(ImmutableMap.of("presto_view", "true"))
+ .storageDescriptor(
Review Comment:
This test-only code pulls in Guava solely to build a single-entry map.
Prefer using `Map.of(\"presto_view\", \"true\")` here to avoid
introducing/strengthening a Guava dependency and to match the style already
used in other tests in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
