mchades commented on code in PR #11926:
URL: https://github.com/apache/gravitino/pull/11926#discussion_r3774349202
##########
trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/hive/00019_view.sql:
##########
@@ -0,0 +1,47 @@
+CREATE SCHEMA gt_hive.gt_hive_view_db;
+CREATE TABLE gt_hive.gt_hive_view_db.t01 (id integer, name varchar, salary
integer);
+INSERT INTO gt_hive.gt_hive_view_db.t01 VALUES (1, 'alice', 100), (2, 'bob',
200);
+
+-- create + query + show tables
+CREATE VIEW gt_hive.gt_hive_view_db.v01 AS SELECT id, name FROM
gt_hive.gt_hive_view_db.t01 WHERE salary > 100;
+SHOW TABLES FROM gt_hive.gt_hive_view_db;
+SELECT * FROM gt_hive.gt_hive_view_db.v01 ORDER BY id;
+
+-- show create view
+SHOW CREATE VIEW gt_hive.gt_hive_view_db.v01;
+
+-- create or replace
+CREATE OR REPLACE VIEW gt_hive.gt_hive_view_db.v01 AS SELECT id, name, salary
FROM gt_hive.gt_hive_view_db.t01;
+SELECT * FROM gt_hive.gt_hive_view_db.v01 ORDER BY id;
+
+-- rename + drop
+ALTER VIEW gt_hive.gt_hive_view_db.v01 RENAME TO gt_hive.gt_hive_view_db.v02;
+SHOW TABLES FROM gt_hive.gt_hive_view_db;
+DROP VIEW gt_hive.gt_hive_view_db.v02;
+SHOW TABLES FROM gt_hive.gt_hive_view_db;
+
+-- default catalog/schema round trip: create a view via USE with an
unqualified source table
+-- reference, then switch the session default elsewhere before querying it, so
the query can only
+-- succeed if the view's own stored default catalog/schema (not the ambient
session) is used to
+-- resolve "t01".
+USE gt_hive.gt_hive_view_db;
+CREATE VIEW v03 AS SELECT id, name FROM t01 WHERE salary > 100;
+USE gt_hive.information_schema;
+SELECT * FROM gt_hive.gt_hive_view_db.v03 ORDER BY id;
+DROP VIEW gt_hive.gt_hive_view_db.v03;
+
+-- error cases: drop nonexistent view; create view colliding with existing
table name (HMS natural rejection)
+DROP VIEW gt_hive.gt_hive_view_db.nonexistent_view;
+CREATE VIEW gt_hive.gt_hive_view_db.t01 AS SELECT 1;
+
+-- native Trino view interop: a view created directly through Trino's own
native Hive connector
+-- (bypassing Gravitino) is encoded using Trino's own native "Presto View"
format; Gravitino's
+-- Trino connector recognizes this format directly, so the view is visible and
queryable through
+-- gt_hive too, without going through Gravitino at all to create it.
+CREATE VIEW native_hive.gt_hive_view_db.native_v01 AS SELECT id, name FROM
native_hive.gt_hive_view_db.t01;
Review Comment:
**[P2] This covers native Hive → Gravitino, but not the reverse direction.**
Could we also create a view through `gt_hive`, then run `SHOW CREATE VIEW` and
`SELECT` through `native_hive`, including a `timestamp(3)` output column?
Otherwise the new encoder is only validated by Gravitino’s own decoder, so a
native-incompatible payload could still pass this test.
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadataAdapter.java:
##########
@@ -148,6 +157,116 @@ public GravitinoTable createTable(ConnectorTableMetadata
tableMetadata) {
return new GravitinoTable(schemaName, tableName, columns, comment,
properties);
}
+ /**
+ * Transform Gravitino view metadata to Trino ConnectorViewDefinition. Owner
is not supported by
+ * Gravitino views, so the resulting definition always has an empty owner;
since Trino requires an
+ * owner for run-as-definer views, {@code runAsInvoker} is always {@code
true}.
+ *
+ * <p>{@link ConnectorViewDefinition} requires a catalog to be present
whenever a schema is
+ * present. Some catalogs (e.g. Iceberg) can store a default schema without
a default catalog; in
+ * single-metalake mode the current Trino catalog is used as a fallback,
since the schema is
+ * implicitly relative to it. In multi-metalake mode the bare Gravitino
catalog name is not the
+ * name Trino actually resolves catalogs by, so this fallback cannot be
applied and the view is
+ * rejected instead of being exposed with a wrong or unresolvable catalog.
+ *
+ * @param view the Gravitino view
+ * @param catalogName the name of the Trino catalog this view belongs to
+ * @param singleMetalakeMode whether the connector is running in
single-metalake mode
+ * @return the Trino ConnectorViewDefinition
+ */
+ public ConnectorViewDefinition getViewDefinition(
+ GravitinoView view, String catalogName, boolean singleMetalakeMode) {
+ Preconditions.checkArgument(
+ view.getSql() != null,
+ "View %s.%s has no Trino dialect SQL representation",
+ view.getSchemaName(),
+ view.getName());
+ List<ViewColumn> columns =
+ view.getColumns().stream()
+ .map(
+ column ->
+ new ViewColumn(
+ column.getName(),
+
dataTypeTransformer.getTrinoType(column.getType()).getTypeId(),
+ Optional.ofNullable(column.getComment())))
+ .collect(Collectors.toList());
+
+ String defaultCatalog = view.getDefaultCatalog();
+ if (defaultCatalog == null && view.getDefaultSchema() != null) {
+ if (!singleMetalakeMode) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION,
+ String.format(
+ "View %s.%s has a default schema without a default catalog,
which is not "
+ + "supported in multi-metalake mode",
+ view.getSchemaName(), view.getName()));
+ }
+ defaultCatalog = catalogName;
+ }
+
+ return new ConnectorViewDefinition(
+ view.getSql(),
+ Optional.ofNullable(defaultCatalog),
+ Optional.ofNullable(view.getDefaultSchema()),
+ columns,
+ Optional.ofNullable(view.getComment()),
+ Optional.empty(),
Review Comment:
**[P1] Could we round-trip the native Trino security and resolution context
through reserved internal view properties?** `createView()` drops the
owner/security mode, native view loading also discards these fields and the SQL
path, while this method always reconstructs an ownerless `SECURITY INVOKER`
definition with an empty path. Trino creates views as `SECURITY DEFINER` by
default, so this silently changes semantics after reload. Please namespace and
protect these properties from direct mutation, use the decoded native payload
as the source of truth on load, and cover DEFINER, INVOKER, and non-empty-path
round trips.
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java:
##########
@@ -454,4 +498,297 @@ 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.
+ LOG.debug(
+ "View {}.{} in catalog {} has no Trino dialect SQL representation,
hiding it from"
+ + " Trino",
+ schemaName,
+ viewName,
+ catalogName);
+ return Optional.empty();
+ }
+ return Optional.of(gravitinoView);
+ } catch (NoSuchViewException e) {
+ return Optional.empty();
+ } catch (UnsupportedOperationException e) {
+ LOG.debug(
+ "Catalog {} does not support loading view {}.{}", catalogName,
schemaName, viewName, 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)
+ .filter(viewName -> getViewIfPresent(schemaName,
viewName).isPresent())
+ .toList();
+ } catch (UnsupportedOperationException e) {
+ LOG.debug(
+ "Catalog {} does not support listing views for schema {}",
catalogName, schemaName, e);
+ return List.of();
+ } catch (NoSuchSchemaException e) {
Review Comment:
**[P2] `ConnectorMetadata.listViews()` must return an empty list when the
requested schema does not exist, but this converts `NoSuchSchemaException` into
an error.** Could we return `List.of()` for this case and add a missing-schema
regression test?
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java:
##########
@@ -454,4 +498,297 @@ 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.
+ LOG.debug(
+ "View {}.{} in catalog {} has no Trino dialect SQL representation,
hiding it from"
+ + " Trino",
+ schemaName,
+ viewName,
+ catalogName);
+ return Optional.empty();
+ }
+ return Optional.of(gravitinoView);
+ } catch (NoSuchViewException e) {
+ return Optional.empty();
+ } catch (UnsupportedOperationException e) {
+ LOG.debug(
+ "Catalog {} does not support loading view {}.{}", catalogName,
schemaName, viewName, 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)
+ .filter(viewName -> getViewIfPresent(schemaName,
viewName).isPresent())
+ .toList();
+ } catch (UnsupportedOperationException e) {
+ LOG.debug(
+ "Catalog {} does not support listing views for schema {}",
catalogName, schemaName, e);
+ return List.of();
+ } 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.
+ *
+ * <p>Only views with a Trino dialect SQL representation are considered
visible to Trino; if an
+ * entity with the same name already exists but has no Trino representation
(e.g. a view created
+ * by another engine), it is never silently replaced.
+ *
+ * @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");
+ }
+ Preconditions.checkArgument(
+ view.getSql() != null,
+ "View %s.%s has no Trino dialect SQL representation",
+ view.getSchemaName(),
+ view.getName());
+ 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) {
+ View existingView = viewCatalog.loadView(identifier);
+ if (!existingView.sqlFor(Dialects.TRINO).isPresent()) {
+ // An entity with this name already exists but is not visible to
Trino (e.g. a view
+ // created by another engine), so it must not be treated as
replaceable.
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_ALREADY_EXISTS, "View already
exists");
+ }
+ if (!replace) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_ALREADY_EXISTS, "View already
exists");
+ }
+ if (hasNonTrinoRepresentation(existingView.representations())) {
+ // Trino cannot regenerate SQL for other engines' dialects, so
replacing here would
+ // leave those representations referring to a schema/body that no
longer matches.
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION,
+ "Cannot replace view "
+ + view.getSchemaName()
+ + "."
+ + view.getName()
+ + " because it has SQL representations in other dialects");
+ }
+ List<ViewChange> changes = new ArrayList<>();
+ changes.add(
+ ViewChange.replaceView(
+ view.getRawColumns(),
+ representations,
+ view.getDefaultCatalog(),
+ view.getDefaultSchema(),
+ view.getComment()));
+ changes.addAll(computePropertyChanges(existingView.properties(),
view.getProperties()));
+ viewCatalog.alterView(identifier, changes.toArray(new ViewChange[0]));
+ } 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);
+ } catch (UnsupportedOperationException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION,
+ "Catalog does not support this view operation",
+ 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) {
+ if (!supportsViews()) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION, "Catalog does
not support views");
+ }
+ // 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;
+ try {
+ dropped = viewCatalog.dropView(NameIdentifier.of(schemaName, viewName));
+ } catch (UnsupportedOperationException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION,
+ "Catalog does not support this view operation",
+ e);
+ }
+ 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 (!supportsViews()) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION, "Catalog does
not support views");
+ }
+ 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());
+ if (oldViewName.getTableName().equals(newViewName.getTableName())) {
+ return;
+ }
+ try {
+ viewCatalog.alterView(
+ NameIdentifier.of(oldViewName.getSchemaName(),
oldViewName.getTableName()),
+ ViewChange.rename(newViewName.getTableName()));
+ } catch (NoSuchViewException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_NOT_EXISTS, "View does not exist",
e);
+ } catch (ViewAlreadyExistsException e) {
Review Comment:
**[P2] This catch is not reached for Iceberg rename collisions because
`IcebergViewCatalogOperations` lets
`org.apache.iceberg.exceptions.AlreadyExistsException` escape instead of
translating it to Gravitino’s `ViewAlreadyExistsException`.** Could we
normalize that exception in the Iceberg adapter and add an occupied-target
regression test?
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java:
##########
@@ -454,4 +498,297 @@ 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.
+ LOG.debug(
+ "View {}.{} in catalog {} has no Trino dialect SQL representation,
hiding it from"
+ + " Trino",
+ schemaName,
+ viewName,
+ catalogName);
+ return Optional.empty();
+ }
+ return Optional.of(gravitinoView);
+ } catch (NoSuchViewException e) {
+ return Optional.empty();
+ } catch (UnsupportedOperationException e) {
+ LOG.debug(
+ "Catalog {} does not support loading view {}.{}", catalogName,
schemaName, viewName, 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)
+ .filter(viewName -> getViewIfPresent(schemaName,
viewName).isPresent())
+ .toList();
+ } catch (UnsupportedOperationException e) {
+ LOG.debug(
+ "Catalog {} does not support listing views for schema {}",
catalogName, schemaName, e);
+ return List.of();
+ } 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.
+ *
+ * <p>Only views with a Trino dialect SQL representation are considered
visible to Trino; if an
+ * entity with the same name already exists but has no Trino representation
(e.g. a view created
+ * by another engine), it is never silently replaced.
+ *
+ * @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");
+ }
+ Preconditions.checkArgument(
+ view.getSql() != null,
+ "View %s.%s has no Trino dialect SQL representation",
+ view.getSchemaName(),
+ view.getName());
+ 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) {
+ View existingView = viewCatalog.loadView(identifier);
+ if (!existingView.sqlFor(Dialects.TRINO).isPresent()) {
+ // An entity with this name already exists but is not visible to
Trino (e.g. a view
+ // created by another engine), so it must not be treated as
replaceable.
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_ALREADY_EXISTS, "View already
exists");
+ }
+ if (!replace) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_ALREADY_EXISTS, "View already
exists");
+ }
+ if (hasNonTrinoRepresentation(existingView.representations())) {
+ // Trino cannot regenerate SQL for other engines' dialects, so
replacing here would
+ // leave those representations referring to a schema/body that no
longer matches.
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION,
+ "Cannot replace view "
+ + view.getSchemaName()
+ + "."
+ + view.getName()
+ + " because it has SQL representations in other dialects");
+ }
+ List<ViewChange> changes = new ArrayList<>();
+ changes.add(
+ ViewChange.replaceView(
+ view.getRawColumns(),
+ representations,
+ view.getDefaultCatalog(),
+ view.getDefaultSchema(),
+ view.getComment()));
+ changes.addAll(computePropertyChanges(existingView.properties(),
view.getProperties()));
Review Comment:
**[P2] For Trino 445+, how can a non-empty view-property map reach this code
from SQL?** No connector implementation exposes `getViewProperties()`, so Trino
rejects `CREATE VIEW ... WITH (...)` before `createViewInternal()`, while the
current unit test injects the property map directly. Could we delegate the
internal connector’s view-property metadata in the applicable version shim(s),
splitting the 440–445 range if necessary, and add an SQL-level IT?
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java:
##########
@@ -454,4 +498,297 @@ 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.
+ LOG.debug(
+ "View {}.{} in catalog {} has no Trino dialect SQL representation,
hiding it from"
+ + " Trino",
+ schemaName,
+ viewName,
+ catalogName);
+ return Optional.empty();
+ }
+ return Optional.of(gravitinoView);
+ } catch (NoSuchViewException e) {
+ return Optional.empty();
+ } catch (UnsupportedOperationException e) {
+ LOG.debug(
+ "Catalog {} does not support loading view {}.{}", catalogName,
schemaName, viewName, 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)
+ .filter(viewName -> getViewIfPresent(schemaName,
viewName).isPresent())
+ .toList();
+ } catch (UnsupportedOperationException e) {
+ LOG.debug(
+ "Catalog {} does not support listing views for schema {}",
catalogName, schemaName, e);
+ return List.of();
+ } 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.
+ *
+ * <p>Only views with a Trino dialect SQL representation are considered
visible to Trino; if an
+ * entity with the same name already exists but has no Trino representation
(e.g. a view created
+ * by another engine), it is never silently replaced.
+ *
+ * @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");
+ }
+ Preconditions.checkArgument(
+ view.getSql() != null,
+ "View %s.%s has no Trino dialect SQL representation",
+ view.getSchemaName(),
+ view.getName());
+ 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) {
+ View existingView = viewCatalog.loadView(identifier);
+ if (!existingView.sqlFor(Dialects.TRINO).isPresent()) {
+ // An entity with this name already exists but is not visible to
Trino (e.g. a view
+ // created by another engine), so it must not be treated as
replaceable.
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_ALREADY_EXISTS, "View already
exists");
+ }
+ if (!replace) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_ALREADY_EXISTS, "View already
exists");
+ }
+ if (hasNonTrinoRepresentation(existingView.representations())) {
+ // Trino cannot regenerate SQL for other engines' dialects, so
replacing here would
+ // leave those representations referring to a schema/body that no
longer matches.
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION,
+ "Cannot replace view "
+ + view.getSchemaName()
+ + "."
+ + view.getName()
+ + " because it has SQL representations in other dialects");
+ }
+ List<ViewChange> changes = new ArrayList<>();
+ changes.add(
+ ViewChange.replaceView(
+ view.getRawColumns(),
+ representations,
+ view.getDefaultCatalog(),
+ view.getDefaultSchema(),
+ view.getComment()));
+ changes.addAll(computePropertyChanges(existingView.properties(),
view.getProperties()));
+ viewCatalog.alterView(identifier, changes.toArray(new ViewChange[0]));
+ } 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);
+ } catch (UnsupportedOperationException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION,
+ "Catalog does not support this view operation",
+ 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) {
+ if (!supportsViews()) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION, "Catalog does
not support views");
+ }
+ // 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;
+ try {
+ dropped = viewCatalog.dropView(NameIdentifier.of(schemaName, viewName));
+ } catch (UnsupportedOperationException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION,
+ "Catalog does not support this view operation",
+ e);
+ }
+ 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 (!supportsViews()) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION, "Catalog does
not support views");
+ }
+ 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());
+ if (oldViewName.getTableName().equals(newViewName.getTableName())) {
+ return;
+ }
+ try {
+ viewCatalog.alterView(
+ NameIdentifier.of(oldViewName.getSchemaName(),
oldViewName.getTableName()),
+ ViewChange.rename(newViewName.getTableName()));
+ } catch (NoSuchViewException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_NOT_EXISTS, "View does not exist",
e);
+ } catch (ViewAlreadyExistsException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_VIEW_ALREADY_EXISTS, "View already
exists", e);
+ } catch (UnsupportedOperationException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION,
+ "Catalog does not support this view operation",
+ e);
+ }
+ }
+
+ /**
+ * Checks whether any of the given representations belongs to a dialect
other than {@link
+ * Dialects#TRINO}.
+ *
+ * @param representations the representations to check
+ * @return true if a non-Trino dialect representation is present
+ */
+ private static boolean hasNonTrinoRepresentation(Representation[]
representations) {
+ return Arrays.stream(representations)
+ .anyMatch(
+ representation ->
+ representation instanceof SQLRepresentation
+ && !Dialects.TRINO.equalsIgnoreCase(
+ ((SQLRepresentation) representation).dialect()));
+ }
+
+ /**
+ * Computes the {@link ViewChange} operations needed to apply {@code
desired} on top of a view's
+ * current properties, so that {@code CREATE OR REPLACE VIEW ... WITH (...)}
applies the new
+ * properties instead of silently leaving them unset ({@link
ViewChange#replaceView} does not
+ * affect properties).
+ *
+ * <p>Only additions/updates are applied, never removals: {@code current} is
the view's full
Review Comment:
**[P2] `CREATE OR REPLACE VIEW` should replace caller-managed properties,
but this only adds or updates them, so a property omitted from the new `WITH
(...)` clause remains stale.** Could we preserve backend/internal reserved keys
while emitting `RemoveProperty` for omitted caller-owned keys, and test
removing a user property while retaining reserved Hive and Trino metadata?
--
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]
