Copilot commented on code in PR #11926:
URL: https://github.com/apache/gravitino/pull/11926#discussion_r3537082096
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/util/json/JsonCodec.java:
##########
@@ -45,6 +45,7 @@
import io.trino.spi.connector.ConnectorTransactionHandle;
import io.trino.spi.type.StandardTypes;
import io.trino.spi.type.Type;
+import io.trino.spi.type.TypeId;
Review Comment:
`TypeId` is imported but only referenced in Javadoc. Most Java style checks
treat that as an unused import, which will fail compilation/checkstyle. Either
remove the import (and fully-qualify the Javadoc link if desired) or reference
`TypeId` in code.
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java:
##########
@@ -454,4 +471,179 @@ public Function getFunction(String schemaName, String
functionName) {
}
return functionCatalog.getFunction(NameIdentifier.of(schemaName,
functionName));
}
+
+ /**
+ * Checks whether the catalog supports view operations.
+ *
+ * @return true if the catalog supports view operations, false otherwise
+ */
+ public boolean supportsViews() {
+ return viewCatalog != null;
+ }
+
+ /**
+ * Retrieves the Gravitino view for the specified name, if it exists and has
a Trino dialect SQL
+ * representation.
+ *
+ * @param schemaName the name of the schema
+ * @param viewName the name of the view
+ * @return an {@link Optional} containing the Gravitino view, or {@link
Optional#empty()} if the
+ * view does not exist or has no Trino dialect SQL representation
+ */
+ public Optional<GravitinoView> getViewIfPresent(String schemaName, String
viewName) {
+ if (!supportsViews()) {
+ return Optional.empty();
+ }
+ try {
+ View view = viewCatalog.loadView(NameIdentifier.of(schemaName,
viewName));
+ GravitinoView gravitinoView = new GravitinoView(schemaName, viewName,
view);
+ if (gravitinoView.getSql() == null) {
+ // The view exists but has no Trino dialect SQL representation, so it
is not visible to
+ // Trino.
+ return Optional.empty();
+ }
+ return Optional.of(gravitinoView);
+ } catch (NoSuchViewException e) {
+ return Optional.empty();
+ }
+ }
+
+ /**
+ * Retrieves the Gravitino view for the specified name.
+ *
+ * @param schemaName the name of the schema
+ * @param viewName the name of the view
+ * @return the Gravitino view
+ * @throws TrinoException if the view does not exist or has no Trino dialect
SQL representation
+ */
+ public GravitinoView getView(String schemaName, String viewName) {
+ return getViewIfPresent(schemaName, viewName)
+ .orElseThrow(
+ () ->
+ new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_NOT_EXISTS, "View does
not exist"));
+ }
+
+ /**
+ * Lists the names of all views in the specified schema.
+ *
+ * @param schemaName the name of the schema
+ * @return a list of view names, or an empty list if the catalog does not
support views
+ */
+ public List<String> listViews(String schemaName) {
+ if (!supportsViews()) {
+ return List.of();
+ }
+ try {
+ NameIdentifier[] views = viewCatalog.listViews(Namespace.of(schemaName));
+ return Arrays.stream(views).map(NameIdentifier::name).toList();
+ } catch (NoSuchSchemaException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_SCHEMA_NOT_EXISTS,
SCHEMA_DOES_NOT_EXIST_MSG, e);
+ }
+ }
+
+ /**
+ * Creates or replaces a view in the catalog.
+ *
+ * @param view the Gravitino view, with the Trino dialect SQL definition set
+ * @param replace whether to replace the view if it already exists
+ */
+ public void createView(GravitinoView view, boolean replace) {
+ if (!supportsViews()) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION, "Catalog does
not support views");
+ }
+ NameIdentifier identifier = NameIdentifier.of(view.getSchemaName(),
view.getName());
+ SQLRepresentation[] representations = {
+
SQLRepresentation.builder().withDialect(Dialects.TRINO).withSql(view.getSql()).build()
+ };
+ try {
+ boolean exists = viewCatalog.viewExists(identifier);
+ if (exists && replace) {
+ viewCatalog.alterView(
+ identifier,
+ ViewChange.replaceView(
+ view.getRawColumns(),
+ representations,
+ view.getDefaultCatalog(),
+ view.getDefaultSchema(),
+ view.getComment()));
+ } else if (exists) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_ALREADY_EXISTS, "View already
exists");
+ } else {
+ viewCatalog.createView(
+ identifier,
+ view.getComment(),
+ view.getRawColumns(),
+ representations,
+ view.getDefaultCatalog(),
+ view.getDefaultSchema(),
+ view.getProperties());
+ }
+ } catch (NoSuchSchemaException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_SCHEMA_NOT_EXISTS,
SCHEMA_DOES_NOT_EXIST_MSG, e);
+ } catch (ViewAlreadyExistsException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_ALREADY_EXISTS, "View already
exists", e);
+ } catch (NoSuchViewException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_NOT_EXISTS, "View does not exist",
e);
+ }
+ }
+
+ /**
+ * Drops a view from the catalog.
+ *
+ * <p>Only views with a Trino dialect SQL representation are considered
visible to Trino; views
+ * created by other engines without one are treated as not existing, so this
method never drops
+ * them.
+ *
+ * @param schemaName the name of the schema
+ * @param viewName the name of the view
+ */
+ public void dropView(String schemaName, String viewName) {
+ // Ensures the view is visible to Trino (exists and has a Trino dialect
representation) before
+ // dropping it, so views created by other engines without a Trino
representation are never
+ // silently dropped.
+ getView(schemaName, viewName);
+ boolean dropped = viewCatalog.dropView(NameIdentifier.of(schemaName,
viewName));
+ if (!dropped) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_OPERATION_FAILED,
+ "Failed to drop view " + schemaName + "." + viewName);
+ }
+ }
+
+ /**
+ * Renames a view in the catalog.
+ *
+ * <p>Only views with a Trino dialect SQL representation are considered
visible to Trino; views
+ * created by other engines without one are treated as not existing, so this
method never renames
+ * them.
+ *
+ * @param oldViewName the old name of the view
+ * @param newViewName the new name of the view
+ */
+ public void renameView(SchemaTableName oldViewName, SchemaTableName
newViewName) {
+ if (!oldViewName.getSchemaName().equals(newViewName.getSchemaName())) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION, "Cannot rename
view across schemas");
+ }
+ // Ensures the view is visible to Trino before renaming it, for the same
reason as dropView.
+ getView(oldViewName.getSchemaName(), oldViewName.getTableName());
Review Comment:
`renameView()` also relies on `getView()` to fail when views are
unsupported, which reports `GRAVITINO_VIEW_NOT_EXISTS` rather than
`GRAVITINO_UNSUPPORTED_OPERATION`. Adding an explicit `supportsViews()` guard
makes the error consistent with `createView()` and avoids implying the view is
missing when the feature is unsupported.
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java:
##########
@@ -454,4 +471,179 @@ public Function getFunction(String schemaName, String
functionName) {
}
return functionCatalog.getFunction(NameIdentifier.of(schemaName,
functionName));
}
+
+ /**
+ * Checks whether the catalog supports view operations.
+ *
+ * @return true if the catalog supports view operations, false otherwise
+ */
+ public boolean supportsViews() {
+ return viewCatalog != null;
+ }
+
+ /**
+ * Retrieves the Gravitino view for the specified name, if it exists and has
a Trino dialect SQL
+ * representation.
+ *
+ * @param schemaName the name of the schema
+ * @param viewName the name of the view
+ * @return an {@link Optional} containing the Gravitino view, or {@link
Optional#empty()} if the
+ * view does not exist or has no Trino dialect SQL representation
+ */
+ public Optional<GravitinoView> getViewIfPresent(String schemaName, String
viewName) {
+ if (!supportsViews()) {
+ return Optional.empty();
+ }
+ try {
+ View view = viewCatalog.loadView(NameIdentifier.of(schemaName,
viewName));
+ GravitinoView gravitinoView = new GravitinoView(schemaName, viewName,
view);
+ if (gravitinoView.getSql() == null) {
+ // The view exists but has no Trino dialect SQL representation, so it
is not visible to
+ // Trino.
+ return Optional.empty();
+ }
+ return Optional.of(gravitinoView);
+ } catch (NoSuchViewException e) {
+ return Optional.empty();
+ }
+ }
+
+ /**
+ * Retrieves the Gravitino view for the specified name.
+ *
+ * @param schemaName the name of the schema
+ * @param viewName the name of the view
+ * @return the Gravitino view
+ * @throws TrinoException if the view does not exist or has no Trino dialect
SQL representation
+ */
+ public GravitinoView getView(String schemaName, String viewName) {
+ return getViewIfPresent(schemaName, viewName)
+ .orElseThrow(
+ () ->
+ new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_NOT_EXISTS, "View does
not exist"));
+ }
+
+ /**
+ * Lists the names of all views in the specified schema.
+ *
+ * @param schemaName the name of the schema
+ * @return a list of view names, or an empty list if the catalog does not
support views
+ */
+ public List<String> listViews(String schemaName) {
+ if (!supportsViews()) {
+ return List.of();
+ }
+ try {
+ NameIdentifier[] views = viewCatalog.listViews(Namespace.of(schemaName));
+ return Arrays.stream(views).map(NameIdentifier::name).toList();
Review Comment:
`listViews()` currently returns *all* view names from
`viewCatalog.listViews(...)` without filtering out views that lack a Trino SQL
representation. That contradicts this PR's stated behavior (views without a
Trino representation should be invisible to Trino) and can lead to `SHOW
TABLES`/`listViews` returning entries that `getView()` cannot load.
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java:
##########
@@ -454,4 +471,179 @@ public Function getFunction(String schemaName, String
functionName) {
}
return functionCatalog.getFunction(NameIdentifier.of(schemaName,
functionName));
}
+
+ /**
+ * Checks whether the catalog supports view operations.
+ *
+ * @return true if the catalog supports view operations, false otherwise
+ */
+ public boolean supportsViews() {
+ return viewCatalog != null;
+ }
+
+ /**
+ * Retrieves the Gravitino view for the specified name, if it exists and has
a Trino dialect SQL
+ * representation.
+ *
+ * @param schemaName the name of the schema
+ * @param viewName the name of the view
+ * @return an {@link Optional} containing the Gravitino view, or {@link
Optional#empty()} if the
+ * view does not exist or has no Trino dialect SQL representation
+ */
+ public Optional<GravitinoView> getViewIfPresent(String schemaName, String
viewName) {
+ if (!supportsViews()) {
+ return Optional.empty();
+ }
+ try {
+ View view = viewCatalog.loadView(NameIdentifier.of(schemaName,
viewName));
+ GravitinoView gravitinoView = new GravitinoView(schemaName, viewName,
view);
+ if (gravitinoView.getSql() == null) {
+ // The view exists but has no Trino dialect SQL representation, so it
is not visible to
+ // Trino.
+ return Optional.empty();
+ }
+ return Optional.of(gravitinoView);
+ } catch (NoSuchViewException e) {
+ return Optional.empty();
+ }
+ }
+
+ /**
+ * Retrieves the Gravitino view for the specified name.
+ *
+ * @param schemaName the name of the schema
+ * @param viewName the name of the view
+ * @return the Gravitino view
+ * @throws TrinoException if the view does not exist or has no Trino dialect
SQL representation
+ */
+ public GravitinoView getView(String schemaName, String viewName) {
+ return getViewIfPresent(schemaName, viewName)
+ .orElseThrow(
+ () ->
+ new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_NOT_EXISTS, "View does
not exist"));
+ }
+
+ /**
+ * Lists the names of all views in the specified schema.
+ *
+ * @param schemaName the name of the schema
+ * @return a list of view names, or an empty list if the catalog does not
support views
+ */
+ public List<String> listViews(String schemaName) {
+ if (!supportsViews()) {
+ return List.of();
+ }
+ try {
+ NameIdentifier[] views = viewCatalog.listViews(Namespace.of(schemaName));
+ return Arrays.stream(views).map(NameIdentifier::name).toList();
+ } catch (NoSuchSchemaException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_SCHEMA_NOT_EXISTS,
SCHEMA_DOES_NOT_EXIST_MSG, e);
+ }
+ }
+
+ /**
+ * Creates or replaces a view in the catalog.
+ *
+ * @param view the Gravitino view, with the Trino dialect SQL definition set
+ * @param replace whether to replace the view if it already exists
+ */
+ public void createView(GravitinoView view, boolean replace) {
+ if (!supportsViews()) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION, "Catalog does
not support views");
+ }
+ NameIdentifier identifier = NameIdentifier.of(view.getSchemaName(),
view.getName());
+ SQLRepresentation[] representations = {
+
SQLRepresentation.builder().withDialect(Dialects.TRINO).withSql(view.getSql()).build()
+ };
+ try {
+ boolean exists = viewCatalog.viewExists(identifier);
+ if (exists && replace) {
+ viewCatalog.alterView(
+ identifier,
+ ViewChange.replaceView(
+ view.getRawColumns(),
+ representations,
+ view.getDefaultCatalog(),
+ view.getDefaultSchema(),
+ view.getComment()));
+ } else if (exists) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_ALREADY_EXISTS, "View already
exists");
+ } else {
+ viewCatalog.createView(
+ identifier,
+ view.getComment(),
+ view.getRawColumns(),
+ representations,
+ view.getDefaultCatalog(),
+ view.getDefaultSchema(),
+ view.getProperties());
+ }
+ } catch (NoSuchSchemaException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_SCHEMA_NOT_EXISTS,
SCHEMA_DOES_NOT_EXIST_MSG, e);
+ } catch (ViewAlreadyExistsException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_ALREADY_EXISTS, "View already
exists", e);
+ } catch (NoSuchViewException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_NOT_EXISTS, "View does not exist",
e);
+ }
+ }
+
+ /**
+ * Drops a view from the catalog.
+ *
+ * <p>Only views with a Trino dialect SQL representation are considered
visible to Trino; views
+ * created by other engines without one are treated as not existing, so this
method never drops
+ * them.
+ *
+ * @param schemaName the name of the schema
+ * @param viewName the name of the view
+ */
+ public void dropView(String schemaName, String viewName) {
+ // Ensures the view is visible to Trino (exists and has a Trino dialect
representation) before
+ // dropping it, so views created by other engines without a Trino
representation are never
+ // silently dropped.
+ getView(schemaName, viewName);
Review Comment:
`dropView()` relies on `getView()` to fail when the catalog doesn't support
views, which surfaces `GRAVITINO_VIEW_NOT_EXISTS` instead of
`GRAVITINO_UNSUPPORTED_OPERATION`. For catalogs without `ViewCatalog`, this
should consistently report an unsupported operation (like `createView()` does).
--
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]
