mchades commented on code in PR #11926:
URL: https://github.com/apache/gravitino/pull/11926#discussion_r4120289933
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadataAdapter.java:
##########
@@ -147,6 +164,123 @@ public GravitinoTable createTable(ConnectorTableMetadata
tableMetadata) {
return new GravitinoTable(schemaName, tableName, columns, comment,
properties);
}
+ /**
+ * Transform Gravitino view metadata to Trino ConnectorViewDefinition. The
view's {@code SECURITY
+ * DEFINER} owner, if any, is read back from the {@link
#RESERVED_VIEW_OWNER_PROPERTY} reserved
+ * property (set by {@link #createView}); its absence means the view is
{@code SECURITY INVOKER},
+ * matching Trino's own invariant that {@code runAsInvoker} and a present
owner are mutually
+ * exclusive.
+ *
+ * <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;
+ }
+
+ String ownerProperty =
view.getProperties().get(RESERVED_VIEW_OWNER_PROPERTY);
+ return new ConnectorViewDefinition(
+ view.getSql(),
+ Optional.ofNullable(defaultCatalog),
+ Optional.ofNullable(view.getDefaultSchema()),
+ columns,
+ Optional.ofNullable(view.getComment()),
+ Optional.ofNullable(ownerProperty),
+ ownerProperty == null,
+ List.of());
+ }
+
+ /**
+ * Transform Trino ConnectorViewDefinition to Gravitino view metadata. The
{@code viewProperties}
+ * are merged as-is into the resulting view's generic properties; the caller
cannot set {@link
+ * #RESERVED_VIEW_OWNER_PROPERTY} directly through them since it is reserved
to round-trip the
+ * definition's own owner/{@code runAsInvoker}.
+ *
+ * <p>Gravitino views have no field to persist the view's {@code path} (the
catalogs/schemas used
+ * to resolve unqualified function names, set via {@code SET PATH}), so a
definition with a
+ * non-empty path is rejected rather than silently discarding it; loading a
view therefore always
+ * returns an empty path, which is safe because no view with a non-empty
path is ever stored.
+ *
+ * @param viewName the schema-qualified view name
+ * @param definition the Trino ConnectorViewDefinition
+ * @param viewProperties the Trino view properties
+ * @return the Gravitino view metadata
+ */
+ public GravitinoView createView(
+ SchemaTableName viewName,
+ ConnectorViewDefinition definition,
+ Map<String, Object> viewProperties) {
+ if (!definition.getPath().isEmpty()) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION,
+ "View " + viewName + " has a non-empty path (SET PATH), which
Gravitino cannot persist");
+ }
+ TypeManager typeManager =
JsonCodec.getTypeManager(getClass().getClassLoader());
+ List<GravitinoColumn> columns = new ArrayList<>();
+ List<ViewColumn> viewColumns = definition.getColumns();
+ for (int i = 0; i < viewColumns.size(); i++) {
+ ViewColumn column = viewColumns.get(i);
+ Type trinoType = typeManager.getType(column.getType());
+ columns.add(
+ new GravitinoColumn(
+ column.getName(),
+ dataTypeTransformer.getGravitinoType(trinoType),
Review Comment:
**[P2] View output types should not use the physical-table type
restrictions.** This transformer is catalog-specific: Iceberg rejects
`varchar(1)` (e.g. the output of `SELECT 'x' AS c`), while Hive accepts
`timestamp(3) with time zone` during creation but rejects it in
`getViewDefinition()`. I reproduced both with the current adapter methods, so
otherwise valid views either cannot be created or cannot be reloaded.
Please use view-specific type conversion that preserves the query's output
types, and cover these cases with the actual Hive/Iceberg adapters rather than
only `GeneralDataTypeTransformer`.
--
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]
