diff --git a/core/trino-main/src/main/java/io/trino/execution/CommentTask.java b/core/trino-main/src/main/java/io/trino/execution/CommentTask.java index cd29320675dd..3448089fd3bc 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CommentTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CommentTask.java @@ -20,6 +20,8 @@ import io.trino.metadata.QualifiedObjectName; import io.trino.metadata.RedirectionAwareTableHandle; import io.trino.metadata.TableHandle; +import io.trino.metadata.ViewColumn; +import io.trino.metadata.ViewDefinition; import io.trino.security.AccessControl; import io.trino.spi.connector.ColumnHandle; import io.trino.sql.tree.Comment; @@ -135,21 +137,37 @@ private void commentOnColumn(Comment statement, Session session) QualifiedName prefix = statement.getName().getPrefix() .orElseThrow(() -> semanticException(MISSING_TABLE, statement, "Table must be specified")); - QualifiedObjectName originalTableName = createQualifiedObjectName(session, statement, prefix); - RedirectionAwareTableHandle redirectionAwareTableHandle = metadata.getRedirectionAwareTableHandle(session, originalTableName); - if (redirectionAwareTableHandle.getTableHandle().isEmpty()) { - throw semanticException(TABLE_NOT_FOUND, statement, "Table does not exist: " + originalTableName); + QualifiedObjectName originalObjectName = createQualifiedObjectName(session, statement, prefix); + if (metadata.isView(session, originalObjectName)) { + String columnName = statement.getName().getSuffix(); + ViewDefinition viewDefinition = metadata.getView(session, originalObjectName).get(); + ViewColumn viewColumn = viewDefinition.getColumns().stream() + .filter(column -> column.getName().equals(columnName)) + .findAny() + .orElseThrow(() -> semanticException(COLUMN_NOT_FOUND, statement, "Column does not exist: " + columnName)); + + accessControl.checkCanSetColumnComment(session.toSecurityContext(), originalObjectName); + metadata.setViewColumnComment(session, originalObjectName, viewColumn.getName(), statement.getComment()); } - TableHandle tableHandle = redirectionAwareTableHandle.getTableHandle().get(); - - String columnName = statement.getName().getSuffix(); - Map columnHandles = metadata.getColumnHandles(session, tableHandle); - if (!columnHandles.containsKey(columnName)) { - throw semanticException(COLUMN_NOT_FOUND, statement, "Column does not exist: " + columnName); + else if (metadata.isMaterializedView(session, originalObjectName)) { + throw semanticException(TABLE_NOT_FOUND, statement, "Setting comments on the columns of materialized views is unsupported"); } + else { + RedirectionAwareTableHandle redirectionAwareTableHandle = metadata.getRedirectionAwareTableHandle(session, originalObjectName); + if (redirectionAwareTableHandle.getTableHandle().isEmpty()) { + throw semanticException(TABLE_NOT_FOUND, statement, "Table does not exist: " + originalObjectName); + } + TableHandle tableHandle = redirectionAwareTableHandle.getTableHandle().get(); + + String columnName = statement.getName().getSuffix(); + Map columnHandles = metadata.getColumnHandles(session, tableHandle); + if (!columnHandles.containsKey(columnName)) { + throw semanticException(COLUMN_NOT_FOUND, statement, "Column does not exist: " + columnName); + } - accessControl.checkCanSetColumnComment(session.toSecurityContext(), redirectionAwareTableHandle.getRedirectedTableName().orElse(originalTableName)); + accessControl.checkCanSetColumnComment(session.toSecurityContext(), redirectionAwareTableHandle.getRedirectedTableName().orElse(originalObjectName)); - metadata.setColumnComment(session, tableHandle, columnHandles.get(columnName), statement.getComment()); + metadata.setColumnComment(session, tableHandle, columnHandles.get(columnName), statement.getComment()); + } } } diff --git a/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java b/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java index 9e912659a452..cd0c01b93165 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java @@ -93,7 +93,7 @@ public ListenableFuture execute( List columns = analysis.getOutputDescriptor(statement.getQuery()) .getVisibleFields().stream() - .map(field -> new ViewColumn(field.getName().get(), field.getType().getTypeId())) + .map(field -> new ViewColumn(field.getName().get(), field.getType().getTypeId(), Optional.empty())) .collect(toImmutableList()); String catalogName = name.getCatalogName(); diff --git a/core/trino-main/src/main/java/io/trino/execution/CreateViewTask.java b/core/trino-main/src/main/java/io/trino/execution/CreateViewTask.java index a55e791d9848..cd5b35b994d8 100644 --- a/core/trino-main/src/main/java/io/trino/execution/CreateViewTask.java +++ b/core/trino-main/src/main/java/io/trino/execution/CreateViewTask.java @@ -97,7 +97,7 @@ else if (metadata.getTableHandle(session, name).isPresent()) { List columns = analysis.getOutputDescriptor(statement.getQuery()) .getVisibleFields().stream() - .map(field -> new ViewColumn(field.getName().get(), field.getType().getTypeId())) + .map(field -> new ViewColumn(field.getName().get(), field.getType().getTypeId(), Optional.empty())) .collect(toImmutableList()); // use DEFINER security by default diff --git a/core/trino-main/src/main/java/io/trino/metadata/MaterializedViewDefinition.java b/core/trino-main/src/main/java/io/trino/metadata/MaterializedViewDefinition.java index 034dcbb30e3c..f4523471505e 100644 --- a/core/trino-main/src/main/java/io/trino/metadata/MaterializedViewDefinition.java +++ b/core/trino-main/src/main/java/io/trino/metadata/MaterializedViewDefinition.java @@ -54,7 +54,7 @@ public MaterializedViewDefinition(ConnectorMaterializedViewDefinition view, Iden view.getCatalog(), view.getSchema(), view.getColumns().stream() - .map(column -> new ViewColumn(column.getName(), column.getType())) + .map(column -> new ViewColumn(column.getName(), column.getType(), Optional.empty())) .collect(toImmutableList()), view.getComment(), Optional.of(runAsIdentity)); diff --git a/core/trino-main/src/main/java/io/trino/metadata/Metadata.java b/core/trino-main/src/main/java/io/trino/metadata/Metadata.java index 453826b74717..c6a9c61056c8 100644 --- a/core/trino-main/src/main/java/io/trino/metadata/Metadata.java +++ b/core/trino-main/src/main/java/io/trino/metadata/Metadata.java @@ -219,6 +219,11 @@ Optional getTableHandleForExecute( */ void setViewComment(Session session, QualifiedObjectName viewName, Optional comment); + /** + * Comments to the specified view column. + */ + void setViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional comment); + /** * Comments to the specified column. */ diff --git a/core/trino-main/src/main/java/io/trino/metadata/MetadataManager.java b/core/trino-main/src/main/java/io/trino/metadata/MetadataManager.java index 326c8a264b2a..0319ae5a9c64 100644 --- a/core/trino-main/src/main/java/io/trino/metadata/MetadataManager.java +++ b/core/trino-main/src/main/java/io/trino/metadata/MetadataManager.java @@ -566,7 +566,11 @@ public List listTableColumns(Session session, QualifiedTab ImmutableList.Builder columns = ImmutableList.builder(); for (ViewColumn column : entry.getValue().getColumns()) { try { - columns.add(new ColumnMetadata(column.getName(), typeManager.getType(column.getType()))); + columns.add(ColumnMetadata.builder() + .setName(column.getName()) + .setType(typeManager.getType(column.getType())) + .setComment(column.getComment()) + .build()); } catch (TypeNotFoundException e) { throw new TrinoException(INVALID_VIEW, format("Unknown type '%s' for column '%s' in view: %s", column.getType(), column.getName(), entry.getKey())); @@ -698,6 +702,15 @@ public void setViewComment(Session session, QualifiedObjectName viewName, Option metadata.setViewComment(session.toConnectorSession(catalogHandle), viewName.asSchemaTableName(), comment); } + @Override + public void setViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional comment) + { + CatalogMetadata catalogMetadata = getCatalogMetadataForWrite(session, viewName.getCatalogName()); + CatalogHandle catalogHandle = catalogMetadata.getCatalogHandle(); + ConnectorMetadata metadata = catalogMetadata.getMetadata(session); + metadata.setViewColumnComment(session.toConnectorSession(catalogHandle), viewName.asSchemaTableName(), columnName, comment); + } + @Override public void setColumnComment(Session session, TableHandle tableHandle, ColumnHandle column, Optional comment) { diff --git a/core/trino-main/src/main/java/io/trino/metadata/ViewColumn.java b/core/trino-main/src/main/java/io/trino/metadata/ViewColumn.java index 7dc2215c3b12..ce03e472d80c 100644 --- a/core/trino-main/src/main/java/io/trino/metadata/ViewColumn.java +++ b/core/trino-main/src/main/java/io/trino/metadata/ViewColumn.java @@ -16,7 +16,9 @@ import io.trino.spi.type.TypeId; import java.util.Objects; +import java.util.Optional; +import static com.google.common.base.MoreObjects.toStringHelper; import static java.util.Objects.requireNonNull; public final class ViewColumn @@ -24,10 +26,13 @@ public final class ViewColumn private final String name; private final TypeId type; - public ViewColumn(String name, TypeId type) + private final Optional comment; + + public ViewColumn(String name, TypeId type, Optional comment) { this.name = requireNonNull(name, "name is null"); this.type = requireNonNull(type, "type is null"); + this.comment = requireNonNull(comment, "comment is null"); } public String getName() @@ -40,10 +45,19 @@ public TypeId getType() return type; } + public Optional getComment() + { + return comment; + } + @Override public String toString() { - return name + " " + type; + return toStringHelper(this).omitNullValues() + .add("name", name) + .add("type", type) + .add("comment", comment.orElse(null)) + .toString(); } @Override @@ -56,12 +70,12 @@ public boolean equals(Object o) return false; } ViewColumn that = (ViewColumn) o; - return Objects.equals(name, that.name) && Objects.equals(type, that.type); + return Objects.equals(name, that.name) && Objects.equals(type, that.type) && Objects.equals(comment, that.comment); } @Override public int hashCode() { - return Objects.hash(name, type); + return Objects.hash(name, type, comment); } } diff --git a/core/trino-main/src/main/java/io/trino/metadata/ViewDefinition.java b/core/trino-main/src/main/java/io/trino/metadata/ViewDefinition.java index c30914f58ad9..025f4883c893 100644 --- a/core/trino-main/src/main/java/io/trino/metadata/ViewDefinition.java +++ b/core/trino-main/src/main/java/io/trino/metadata/ViewDefinition.java @@ -70,7 +70,7 @@ private ViewDefinition(QualifiedObjectName viewName, ConnectorViewDefinition vie this.catalog = view.getCatalog(); this.schema = view.getSchema(); this.columns = view.getColumns().stream() - .map(column -> new ViewColumn(column.getName(), column.getType())) + .map(column -> new ViewColumn(column.getName(), column.getType(), column.getComment())) .collect(toImmutableList()); this.comment = view.getComment(); this.runAsIdentity = runAsIdentity; @@ -124,7 +124,7 @@ public ConnectorViewDefinition toConnectorViewDefinition() catalog, schema, columns.stream() - .map(column -> new ConnectorViewDefinition.ViewColumn(column.getName(), column.getType())) + .map(column -> new ConnectorViewDefinition.ViewColumn(column.getName(), column.getType(), column.getComment())) .collect(toImmutableList()), comment, runAsIdentity.map(Identity::getUser), diff --git a/core/trino-main/src/main/java/io/trino/metadata/ViewInfo.java b/core/trino-main/src/main/java/io/trino/metadata/ViewInfo.java index fef0211fad31..91e2755585f2 100644 --- a/core/trino-main/src/main/java/io/trino/metadata/ViewInfo.java +++ b/core/trino-main/src/main/java/io/trino/metadata/ViewInfo.java @@ -34,7 +34,7 @@ public ViewInfo(ConnectorViewDefinition viewDefinition) { this.originalSql = viewDefinition.getOriginalSql(); this.columns = viewDefinition.getColumns().stream() - .map(column -> new ViewColumn(column.getName(), column.getType())) + .map(column -> new ViewColumn(column.getName(), column.getType(), column.getComment())) .collect(toImmutableList()); this.comment = viewDefinition.getComment(); this.storageTable = Optional.empty(); @@ -44,7 +44,7 @@ public ViewInfo(ConnectorMaterializedViewDefinition viewDefinition) { this.originalSql = viewDefinition.getOriginalSql(); this.columns = viewDefinition.getColumns().stream() - .map(column -> new ViewColumn(column.getName(), column.getType())) + .map(column -> new ViewColumn(column.getName(), column.getType(), Optional.empty())) .collect(toImmutableList()); this.comment = viewDefinition.getComment(); this.storageTable = viewDefinition.getStorageTable(); diff --git a/core/trino-main/src/test/java/io/trino/connector/MockConnector.java b/core/trino-main/src/test/java/io/trino/connector/MockConnector.java index 7d0ad697080f..2fc1722d21f8 100644 --- a/core/trino-main/src/test/java/io/trino/connector/MockConnector.java +++ b/core/trino-main/src/test/java/io/trino/connector/MockConnector.java @@ -509,6 +509,9 @@ public void setTableComment(ConnectorSession session, ConnectorTableHandle table @Override public void setViewComment(ConnectorSession session, SchemaTableName viewName, Optional comment) {} + @Override + public void setViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) {} + @Override public void setColumnComment(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnHandle column, Optional comment) {} diff --git a/core/trino-main/src/test/java/io/trino/execution/BaseDataDefinitionTaskTest.java b/core/trino-main/src/test/java/io/trino/execution/BaseDataDefinitionTaskTest.java index 02e9f1d3c0bf..6760e79b3468 100644 --- a/core/trino-main/src/test/java/io/trino/execution/BaseDataDefinitionTaskTest.java +++ b/core/trino-main/src/test/java/io/trino/execution/BaseDataDefinitionTaskTest.java @@ -158,7 +158,7 @@ protected static QualifiedName asQualifiedName(QualifiedObjectName qualifiedObje protected MaterializedViewDefinition someMaterializedView() { - return someMaterializedView("select * from some_table", ImmutableList.of(new ViewColumn("test", BIGINT.getTypeId()))); + return someMaterializedView("select * from some_table", ImmutableList.of(new ViewColumn("test", BIGINT.getTypeId(), Optional.empty()))); } protected MaterializedViewDefinition someMaterializedView(String sql, List columns) @@ -181,7 +181,7 @@ protected static ConnectorTableMetadata someTable(QualifiedObjectName tableName) protected static ViewDefinition someView() { - return viewDefinition("SELECT 1", ImmutableList.of(new ViewColumn("test", BIGINT.getTypeId()))); + return viewDefinition("SELECT 1", ImmutableList.of(new ViewColumn("test", BIGINT.getTypeId(), Optional.empty()))); } protected static ViewDefinition viewDefinition(String sql, ImmutableList columns) @@ -442,6 +442,23 @@ public void setColumnComment(Session session, TableHandle tableHandle, ColumnHan tables.put(tableMetadata.getTable(), newTableMetadata); } + @Override + public void setViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional comment) + { + ViewDefinition view = views.get(viewName.asSchemaTableName()); + views.put( + viewName.asSchemaTableName(), + new ViewDefinition( + view.getOriginalSql(), + view.getCatalog(), + view.getSchema(), + view.getColumns().stream() + .map(currentViewColumn -> columnName.equals(currentViewColumn.getName()) ? new ViewColumn(currentViewColumn.getName(), currentViewColumn.getType(), comment) : currentViewColumn) + .collect(toImmutableList()), + view.getComment(), + view.getRunAsIdentity())); + } + @Override public void renameMaterializedView(Session session, QualifiedObjectName source, QualifiedObjectName target) { diff --git a/core/trino-main/src/test/java/io/trino/execution/TestCommentTask.java b/core/trino-main/src/test/java/io/trino/execution/TestCommentTask.java index b1eaad0f4748..c71a49171fc6 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestCommentTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestCommentTask.java @@ -27,6 +27,7 @@ import java.util.Optional; import static io.airlift.concurrent.MoreFutures.getFutureValue; +import static io.trino.spi.StandardErrorCode.COLUMN_NOT_FOUND; import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND; import static io.trino.sql.tree.Comment.Type.COLUMN; import static io.trino.sql.tree.Comment.Type.TABLE; @@ -121,6 +122,24 @@ public void testCommentTableColumn() .isEqualTo(Optional.of("new test column comment")); } + @Test + public void testCommentViewColumn() + { + QualifiedObjectName viewName = qualifiedObjectName("existing_view"); + QualifiedName columnName = qualifiedColumnName("existing_view", "test"); + QualifiedName missingColumnName = qualifiedColumnName("existing_view", "missing"); + metadata.createView(testSession, viewName, someView(), false); + assertThat(metadata.isView(testSession, viewName)).isTrue(); + + getFutureValue(setComment(COLUMN, columnName, Optional.of("new test column comment"))); + assertThat(metadata.getView(testSession, viewName).get().getColumns().stream().filter(column -> "test".equals(column.getName())).findAny().get().getComment()) + .isEqualTo(Optional.of("new test column comment")); + + assertTrinoExceptionThrownBy(() -> getFutureValue(setComment(COLUMN, missingColumnName, Optional.of("comment for missing column")))) + .hasErrorCode(COLUMN_NOT_FOUND) + .hasMessage("Column does not exist: %s", missingColumnName.getSuffix()); + } + private ListenableFuture setComment(Comment.Type type, QualifiedName viewName, Optional comment) { return new CommentTask(metadata, new AllowAllAccessControl()).execute(new Comment(type, viewName, comment), queryStateMachine, ImmutableList.of(), WarningCollector.NOOP); diff --git a/core/trino-main/src/test/java/io/trino/metadata/AbstractMockMetadata.java b/core/trino-main/src/test/java/io/trino/metadata/AbstractMockMetadata.java index b5bee784985d..f1df76ebe437 100644 --- a/core/trino-main/src/test/java/io/trino/metadata/AbstractMockMetadata.java +++ b/core/trino-main/src/test/java/io/trino/metadata/AbstractMockMetadata.java @@ -281,6 +281,12 @@ public void setViewComment(Session session, QualifiedObjectName viewName, Option throw new UnsupportedOperationException(); } + @Override + public void setViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional comment) + { + throw new UnsupportedOperationException(); + } + @Override public void setColumnComment(Session session, TableHandle tableHandle, ColumnHandle column, Optional comment) { diff --git a/core/trino-main/src/test/java/io/trino/metadata/CountingAccessMetadata.java b/core/trino-main/src/test/java/io/trino/metadata/CountingAccessMetadata.java index 6a9a87bc2553..8315fbb660f1 100644 --- a/core/trino-main/src/test/java/io/trino/metadata/CountingAccessMetadata.java +++ b/core/trino-main/src/test/java/io/trino/metadata/CountingAccessMetadata.java @@ -285,6 +285,12 @@ public void setViewComment(Session session, QualifiedObjectName viewName, Option delegate.setViewComment(session, viewName, comment); } + @Override + public void setViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional comment) + { + delegate.setViewColumnComment(session, viewName, columnName, comment); + } + @Override public void setColumnComment(Session session, TableHandle tableHandle, ColumnHandle column, Optional comment) { diff --git a/core/trino-main/src/test/java/io/trino/metadata/TestInformationSchemaMetadata.java b/core/trino-main/src/test/java/io/trino/metadata/TestInformationSchemaMetadata.java index b0cd1410f518..45b596a65112 100644 --- a/core/trino-main/src/test/java/io/trino/metadata/TestInformationSchemaMetadata.java +++ b/core/trino-main/src/test/java/io/trino/metadata/TestInformationSchemaMetadata.java @@ -72,7 +72,7 @@ public TestInformationSchemaMetadata() "select 1", Optional.of("test_catalog"), Optional.of("test_schema"), - ImmutableList.of(new ViewColumn("test", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("test", BIGINT.getTypeId(), Optional.of("test column comment"))), Optional.of("comment"), Optional.empty(), true); diff --git a/core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java b/core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java index 9b4387a133e4..d3a1c4d632d6 100644 --- a/core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java +++ b/core/trino-main/src/test/java/io/trino/sql/analyzer/TestAnalyzer.java @@ -6715,7 +6715,7 @@ public void setup() "select a from t1", Optional.of(TPCH_CATALOG), Optional.of("s1"), - ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId(), Optional.empty())), Optional.of("comment"), Identity.ofUser("user"), Optional.empty(), @@ -6727,7 +6727,7 @@ public void setup() "select a from t1", Optional.of(TPCH_CATALOG), Optional.of("s1"), - ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId(), Optional.empty())), Optional.of("comment"), Optional.of(Identity.ofUser("user"))); inSetupTransaction(session -> metadata.createView(session, new QualifiedObjectName(TPCH_CATALOG, "s1", "v1"), viewData1, false)); @@ -6737,7 +6737,7 @@ public void setup() "select a from t1", Optional.of(TPCH_CATALOG), Optional.of("s1"), - ImmutableList.of(new ViewColumn("a", VARCHAR.getTypeId())), + ImmutableList.of(new ViewColumn("a", VARCHAR.getTypeId(), Optional.empty())), Optional.of("comment"), Optional.of(Identity.ofUser("user"))); inSetupTransaction(session -> metadata.createView(session, new QualifiedObjectName(TPCH_CATALOG, "s1", "v2"), viewData2, false)); @@ -6747,7 +6747,7 @@ public void setup() "select a from t4", Optional.of(SECOND_CATALOG), Optional.of("s2"), - ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId(), Optional.empty())), Optional.of("comment"), Optional.of(Identity.ofUser("owner"))); inSetupTransaction(session -> metadata.createView(session, new QualifiedObjectName(THIRD_CATALOG, "s3", "v3"), viewData3, false)); @@ -6757,7 +6757,7 @@ public void setup() "select A from t1", Optional.of("tpch"), Optional.of("s1"), - ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId(), Optional.empty())), Optional.of("comment"), Optional.of(Identity.ofUser("user"))); inSetupTransaction(session -> metadata.createView(session, new QualifiedObjectName("tpch", "s1", "v4"), viewData4, false)); @@ -6767,7 +6767,7 @@ public void setup() "select * from v5", Optional.of(TPCH_CATALOG), Optional.of("s1"), - ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId(), Optional.empty())), Optional.of("comment"), Optional.of(Identity.ofUser("user"))); inSetupTransaction(session -> metadata.createView(session, new QualifiedObjectName(TPCH_CATALOG, "s1", "v5"), viewData5, false)); @@ -6843,7 +6843,7 @@ public void setup() "SELECT a FROM t1", Optional.of(TPCH_CATALOG), Optional.of("s1"), - ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId(), Optional.empty())), Optional.empty(), Identity.ofUser("some user"), Optional.of(new CatalogSchemaTableName(TPCH_CATALOG, "s1", "t1")), @@ -6854,7 +6854,7 @@ public void setup() "SELECT a FROM t2", Optional.of(TPCH_CATALOG), Optional.of("s1"), - ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId(), Optional.empty())), Optional.empty(), Optional.empty()); inSetupTransaction(session -> metadata.createView( @@ -6892,7 +6892,7 @@ public void setup() "SELECT a, b FROM t1", Optional.of(TPCH_CATALOG), Optional.of("s1"), - ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId()), new ViewColumn("b", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId(), Optional.empty()), new ViewColumn("b", BIGINT.getTypeId(), Optional.empty())), Optional.empty(), Identity.ofUser("some user"), // t3 has a, b column and hidden column x @@ -6910,7 +6910,7 @@ public void setup() "SELECT a FROM t1", Optional.of(TPCH_CATALOG), Optional.of("s1"), - ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId(), Optional.empty())), Optional.empty(), Identity.ofUser("some user"), Optional.of(new CatalogSchemaTableName(TPCH_CATALOG, "s1", "t2")), @@ -6927,7 +6927,7 @@ public void setup() "SELECT a, b as c FROM t1", Optional.of(TPCH_CATALOG), Optional.of("s1"), - ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId()), new ViewColumn("c", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId(), Optional.empty()), new ViewColumn("c", BIGINT.getTypeId(), Optional.empty())), Optional.empty(), Identity.ofUser("some user"), Optional.of(new CatalogSchemaTableName(TPCH_CATALOG, "s1", "t2")), @@ -6944,7 +6944,7 @@ public void setup() "SELECT a, null b FROM t1", Optional.of(TPCH_CATALOG), Optional.of("s1"), - ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId()), new ViewColumn("b", RowType.anonymousRow(TINYINT).getTypeId())), + ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId(), Optional.empty()), new ViewColumn("b", RowType.anonymousRow(TINYINT).getTypeId(), Optional.empty())), Optional.empty(), Identity.ofUser("some user"), Optional.of(new CatalogSchemaTableName(TPCH_CATALOG, "s1", "t2")), diff --git a/core/trino-main/src/test/java/io/trino/sql/planner/TestMaterializedViews.java b/core/trino-main/src/test/java/io/trino/sql/planner/TestMaterializedViews.java index 56c82b8e6216..93b1ca435574 100644 --- a/core/trino-main/src/test/java/io/trino/sql/planner/TestMaterializedViews.java +++ b/core/trino-main/src/test/java/io/trino/sql/planner/TestMaterializedViews.java @@ -116,7 +116,7 @@ protected LocalQueryRunner createLocalQueryRunner() "SELECT a, b FROM test_table", Optional.of(TEST_CATALOG_NAME), Optional.of(SCHEMA), - ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId()), new ViewColumn("b", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId(), Optional.empty()), new ViewColumn("b", BIGINT.getTypeId(), Optional.empty())), Optional.empty(), Identity.ofUser("some user"), Optional.of(new CatalogSchemaTableName(TEST_CATALOG_NAME, SCHEMA, "storage_table")), @@ -147,7 +147,7 @@ protected LocalQueryRunner createLocalQueryRunner() "SELECT a, b FROM test_table", Optional.of(TEST_CATALOG_NAME), Optional.of(SCHEMA), - ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId()), new ViewColumn("b", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("a", BIGINT.getTypeId(), Optional.empty()), new ViewColumn("b", BIGINT.getTypeId(), Optional.empty())), Optional.empty(), Identity.ofUser("some user"), Optional.of(new CatalogSchemaTableName(TEST_CATALOG_NAME, SCHEMA, "storage_table_with_casts")), diff --git a/core/trino-main/src/test/java/io/trino/sql/query/TestColumnMask.java b/core/trino-main/src/test/java/io/trino/sql/query/TestColumnMask.java index 5cd540c43b14..0754dddf1ca1 100644 --- a/core/trino-main/src/test/java/io/trino/sql/query/TestColumnMask.java +++ b/core/trino-main/src/test/java/io/trino/sql/query/TestColumnMask.java @@ -79,7 +79,9 @@ public void init() "SELECT nationkey, name FROM local.tiny.nation", Optional.empty(), Optional.empty(), - ImmutableList.of(new ConnectorViewDefinition.ViewColumn("nationkey", BigintType.BIGINT.getTypeId()), new ConnectorViewDefinition.ViewColumn("name", VarcharType.createVarcharType(25).getTypeId())), + ImmutableList.of( + new ConnectorViewDefinition.ViewColumn("nationkey", BigintType.BIGINT.getTypeId(), Optional.empty()), + new ConnectorViewDefinition.ViewColumn("name", VarcharType.createVarcharType(25).getTypeId(), Optional.empty())), Optional.empty(), Optional.of(VIEW_OWNER), false); diff --git a/core/trino-main/src/test/java/io/trino/sql/query/TestRowFilter.java b/core/trino-main/src/test/java/io/trino/sql/query/TestRowFilter.java index c02623be53bc..416a3820681d 100644 --- a/core/trino-main/src/test/java/io/trino/sql/query/TestRowFilter.java +++ b/core/trino-main/src/test/java/io/trino/sql/query/TestRowFilter.java @@ -79,8 +79,9 @@ public void init() "SELECT nationkey, name FROM local.tiny.nation", Optional.empty(), Optional.empty(), - ImmutableList.of(new ConnectorViewDefinition.ViewColumn("nationkey", BigintType.BIGINT.getTypeId()), new ConnectorViewDefinition.ViewColumn("name", VarcharType.createVarcharType(25) - .getTypeId())), + ImmutableList.of( + new ConnectorViewDefinition.ViewColumn("nationkey", BigintType.BIGINT.getTypeId(), Optional.empty()), + new ConnectorViewDefinition.ViewColumn("name", VarcharType.createVarcharType(25).getTypeId(), Optional.empty())), Optional.empty(), Optional.of(VIEW_OWNER), false); diff --git a/core/trino-spi/pom.xml b/core/trino-spi/pom.xml index 9fa906c5b04d..5a7547d816fc 100644 --- a/core/trino-spi/pom.xml +++ b/core/trino-spi/pom.xml @@ -199,6 +199,12 @@ method void io.trino.spi.eventlistener.QueryStatistics::<init>(java.time.Duration, java.time.Duration, java.time.Duration, java.time.Duration, java.util.Optional<java.time.Duration>, java.util.Optional<java.time.Duration>, java.util.Optional<java.time.Duration>, java.util.Optional<java.time.Duration>, java.util.Optional<java.time.Duration>, java.util.Optional<java.time.Duration>, java.util.Optional<java.time.Duration>, java.util.Optional<java.time.Duration>, java.util.Optional<java.time.Duration>, java.util.Optional<java.time.Duration>, long, long, long, long, long, long, long, long, long, long, long, long, long, long, long, double, double, java.util.List<io.trino.spi.eventlistener.StageGcStatistics>, int, boolean, java.util.List<io.trino.spi.eventlistener.StageCpuDistribution>, java.util.List<io.trino.spi.eventlistener.StageOutputBufferUtilization>, java.util.List<java.lang.String>, java.util.Optional<java.lang.String>) + + java.method.numberOfParametersChanged + method void io.trino.spi.connector.ConnectorViewDefinition.ViewColumn::<init>(java.lang.String, io.trino.spi.type.TypeId) + method void io.trino.spi.connector.ConnectorViewDefinition.ViewColumn::<init>(java.lang.String, io.trino.spi.type.TypeId, java.util.Optional<java.lang.String>) + Add support for view column comments + diff --git a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java index a4de58348b3b..344c746da5a5 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java +++ b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java @@ -403,6 +403,14 @@ default void setViewComment(ConnectorSession session, SchemaTableName viewName, throw new TrinoException(NOT_SUPPORTED, "This connector does not support setting view comments"); } + /** + * Comments to the specified view column. + */ + default void setViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + throw new TrinoException(NOT_SUPPORTED, "This connector does not support setting view column comments"); + } + /** * Comments to the specified column */ diff --git a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorViewDefinition.java b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorViewDefinition.java index db5e1a20b012..2bc653b14066 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorViewDefinition.java +++ b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorViewDefinition.java @@ -133,14 +133,18 @@ public static final class ViewColumn { private final String name; private final TypeId type; + private final Optional comment; @JsonCreator public ViewColumn( @JsonProperty("name") String name, - @JsonProperty("type") TypeId type) + @JsonProperty("type") TypeId type, + @JsonProperty("comment") Optional comment) + { this.name = requireNonNull(name, "name is null"); this.type = requireNonNull(type, "type is null"); + this.comment = requireNonNull(comment, "comment is null"); } @JsonProperty @@ -155,10 +159,20 @@ public TypeId getType() return type; } + @JsonProperty + public Optional getComment() + { + return comment; + } + @Override public String toString() { - return name + " " + type; + StringJoiner joiner = new StringJoiner(", ", "[", "]"); + joiner.add("name=" + name); + joiner.add("type=" + type); + comment.ifPresent(value -> joiner.add("comment=" + value)); + return getClass().getSimpleName() + joiner; } } } diff --git a/core/trino-spi/src/test/java/io/trino/spi/connector/TestConnectorViewDefinition.java b/core/trino-spi/src/test/java/io/trino/spi/connector/TestConnectorViewDefinition.java index 90c8b33607aa..5bc16832bf93 100644 --- a/core/trino-spi/src/test/java/io/trino/spi/connector/TestConnectorViewDefinition.java +++ b/core/trino-spi/src/test/java/io/trino/spi/connector/TestConnectorViewDefinition.java @@ -104,8 +104,8 @@ public void testRoundTrip() Optional.of("test_catalog"), Optional.of("test_schema"), ImmutableList.of( - new ViewColumn("abc", BIGINT.getTypeId()), - new ViewColumn("xyz", new ArrayType(createVarcharType(32)).getTypeId())), + new ViewColumn("abc", BIGINT.getTypeId(), Optional.of("abc description")), + new ViewColumn("xyz", new ArrayType(createVarcharType(32)).getTypeId(), Optional.empty())), Optional.of("comment"), Optional.of("test_owner"), false)); diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java index 34475ef4b750..14c9d0a19cdc 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java @@ -439,6 +439,14 @@ public void setViewComment(ConnectorSession session, SchemaTableName viewName, O } } + @Override + public void setViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) { + delegate.setViewColumnComment(session, viewName, columnName, comment); + } + } + @Override public void setColumnComment(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnHandle column, Optional comment) { diff --git a/plugin/trino-blackhole/src/main/java/io/trino/plugin/blackhole/BlackHoleMetadata.java b/plugin/trino-blackhole/src/main/java/io/trino/plugin/blackhole/BlackHoleMetadata.java index 93bf09e58631..a7682ec6173b 100644 --- a/plugin/trino-blackhole/src/main/java/io/trino/plugin/blackhole/BlackHoleMetadata.java +++ b/plugin/trino-blackhole/src/main/java/io/trino/plugin/blackhole/BlackHoleMetadata.java @@ -39,6 +39,7 @@ import io.trino.spi.connector.SchemaNotFoundException; import io.trino.spi.connector.SchemaTableName; import io.trino.spi.connector.SchemaTablePrefix; +import io.trino.spi.connector.ViewNotFoundException; import io.trino.spi.security.TrinoPrincipal; import io.trino.spi.statistics.ColumnStatistics; import io.trino.spi.statistics.ComputedStatistics; @@ -51,10 +52,12 @@ import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableMap.toImmutableMap; import static io.trino.plugin.blackhole.BlackHoleConnector.DISTRIBUTED_ON; import static io.trino.plugin.blackhole.BlackHoleConnector.FIELD_LENGTH_PROPERTY; @@ -396,4 +399,20 @@ private void checkSchemaExists(String schemaName) throw new SchemaNotFoundException(schemaName); } } + + @Override + public void setViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + ConnectorViewDefinition view = getView(session, viewName).orElseThrow(() -> new ViewNotFoundException(viewName)); + views.put(viewName, new ConnectorViewDefinition( + view.getOriginalSql(), + view.getCatalog(), + view.getSchema(), + view.getColumns().stream() + .map(currentViewColumn -> Objects.equals(columnName, currentViewColumn.getName()) ? new ConnectorViewDefinition.ViewColumn(currentViewColumn.getName(), currentViewColumn.getType(), comment) : currentViewColumn) + .collect(toImmutableList()), + view.getComment(), + view.getOwner(), + view.isRunAsInvoker())); + } } diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java index 2c23b97c1f5b..603f51066ce0 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java @@ -1325,11 +1325,7 @@ public void setTableComment(ConnectorSession session, ConnectorTableHandle table @Override public void setViewComment(ConnectorSession session, SchemaTableName viewName, Optional comment) { - Table view = metastore.getTable(viewName.getSchemaName(), viewName.getTableName()) - .orElseThrow(() -> new ViewNotFoundException(viewName)); - if (translateHiveViews && !isPrestoView(view)) { - throw new HiveViewNotSupportedException(viewName); - } + Table view = getView(viewName); ConnectorViewDefinition definition = toConnectorViewDefinition(session, viewName, Optional.of(view)) .orElseThrow(() -> new ViewNotFoundException(viewName)); @@ -1342,8 +1338,44 @@ public void setViewComment(ConnectorSession session, SchemaTableName viewName, O definition.getOwner(), definition.isRunAsInvoker()); + replaceView(session, viewName, view, newDefinition); + } + + @Override + public void setViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + Table view = getView(viewName); + + ConnectorViewDefinition definition = toConnectorViewDefinition(session, viewName, Optional.of(view)) + .orElseThrow(() -> new ViewNotFoundException(viewName)); + ConnectorViewDefinition newDefinition = new ConnectorViewDefinition( + definition.getOriginalSql(), + definition.getCatalog(), + definition.getSchema(), + definition.getColumns().stream() + .map(currentViewColumn -> columnName.equals(currentViewColumn.getName()) ? new ConnectorViewDefinition.ViewColumn(currentViewColumn.getName(), currentViewColumn.getType(), comment) : currentViewColumn) + .collect(toImmutableList()), + definition.getComment(), + definition.getOwner(), + definition.isRunAsInvoker()); + + replaceView(session, viewName, view, newDefinition); + } + + private Table getView(SchemaTableName viewName) + { + Table view = metastore.getTable(viewName.getSchemaName(), viewName.getTableName()) + .orElseThrow(() -> new ViewNotFoundException(viewName)); + if (translateHiveViews && !isPrestoView(view)) { + throw new HiveViewNotSupportedException(viewName); + } + return view; + } + + private void replaceView(ConnectorSession session, SchemaTableName viewName, Table view, ConnectorViewDefinition newViewDefinition) + { Table.Builder viewBuilder = Table.builder(view) - .setViewOriginalText(Optional.of(encodeViewData(newDefinition))); + .setViewOriginalText(Optional.of(encodeViewData(newViewDefinition))); PrincipalPrivileges principalPrivileges = accessControlMetadata.isUsingSystemSecurity() ? NO_PRIVILEGES : buildInitialPrivilegeSet(session.getUser()); diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/LegacyHiveViewReader.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/LegacyHiveViewReader.java index 6c6906de30d0..6da51a5c8729 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/LegacyHiveViewReader.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/LegacyHiveViewReader.java @@ -47,7 +47,7 @@ public ConnectorViewDefinition decodeViewData(String viewData, Table table, Cata Optional.of(catalogName.toString()), Optional.ofNullable(table.getDatabaseName()), Stream.concat(table.getDataColumns().stream(), table.getPartitionColumns().stream()) - .map(column -> new ConnectorViewDefinition.ViewColumn(column.getName(), TypeId.of(column.getType().getTypeSignature().toString()))) + .map(column -> new ConnectorViewDefinition.ViewColumn(column.getName(), TypeId.of(column.getType().getTypeSignature().toString()), column.getComment())) .collect(toImmutableList()), Optional.ofNullable(table.getParameters().get(TABLE_COMMENT)), Optional.empty(), // will be filled in later by HiveMetadata diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/ViewReaderUtil.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/ViewReaderUtil.java index 0689c6c5440f..888133755d1a 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/ViewReaderUtil.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/ViewReaderUtil.java @@ -219,7 +219,8 @@ public ConnectorViewDefinition decodeViewData(String viewSql, Table table, Catal List columns = rowType.getFieldList().stream() .map(field -> new ViewColumn( field.getName(), - typeManager.fromSqlType(getTypeString(field.getType())).getTypeId())) + typeManager.fromSqlType(getTypeString(field.getType())).getTypeId(), + Optional.empty())) .collect(toImmutableList()); return new ConnectorViewDefinition( trinoSql, diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHive.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHive.java index 01fc4a664143..631f58a102c8 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHive.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHive.java @@ -3924,7 +3924,7 @@ private void doCreateView(SchemaTableName viewName, boolean replace) viewData, Optional.empty(), Optional.empty(), - ImmutableList.of(new ViewColumn("test", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("test", BIGINT.getTypeId(), Optional.empty())), Optional.empty(), Optional.empty(), true); diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java index 65bf92e85f79..eafe3a2ae5c1 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java @@ -229,6 +229,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior) return false; case SUPPORTS_COMMENT_ON_VIEW: + case SUPPORTS_COMMENT_ON_VIEW_COLUMN: return true; case SUPPORTS_CREATE_VIEW: diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java index ab1e4adedd5a..48ae6964b355 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java @@ -665,6 +665,12 @@ public void setViewComment(ConnectorSession session, SchemaTableName viewName, O catalog.updateViewComment(session, viewName, comment); } + @Override + public void setViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + catalog.updateViewColumnComment(session, viewName, columnName, comment); + } + @Override public Optional getNewTableLayout(ConnectorSession session, ConnectorTableMetadata tableMetadata) { diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/TrinoCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/TrinoCatalog.java index 807d07bb2611..8a6ab955d2dd 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/TrinoCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/TrinoCatalog.java @@ -91,6 +91,8 @@ Transaction newCreateTableTransaction( void updateViewComment(ConnectorSession session, SchemaTableName schemaViewName, Optional comment); + void updateViewColumnComment(ConnectorSession session, SchemaTableName schemaViewName, String columnName, Optional comment); + String defaultTableLocation(ConnectorSession session, SchemaTableName schemaTableName); void setTablePrincipal(ConnectorSession session, SchemaTableName schemaTableName, TrinoPrincipal principal); diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java index aac15f8ddd61..107bfa6fa446 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/glue/TrinoGlueCatalog.java @@ -70,6 +70,7 @@ import java.time.Duration; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Stream; @@ -690,6 +691,30 @@ public void updateViewComment(ConnectorSession session, SchemaTableName viewName definition.getOwner(), definition.isRunAsInvoker()); + updateView(session, viewName, newDefinition); + } + + @Override + public void updateViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + ConnectorViewDefinition definition = getView(session, viewName) + .orElseThrow(() -> new ViewNotFoundException(viewName)); + ConnectorViewDefinition newDefinition = new ConnectorViewDefinition( + definition.getOriginalSql(), + definition.getCatalog(), + definition.getSchema(), + definition.getColumns().stream() + .map(currentViewColumn -> Objects.equals(columnName, currentViewColumn.getName()) ? new ConnectorViewDefinition.ViewColumn(currentViewColumn.getName(), currentViewColumn.getType(), comment) : currentViewColumn) + .collect(toImmutableList()), + definition.getComment(), + definition.getOwner(), + definition.isRunAsInvoker()); + + updateView(session, viewName, newDefinition); + } + + private void updateView(ConnectorSession session, SchemaTableName viewName, ConnectorViewDefinition newDefinition) + { TableInput viewTableInput = getViewTableInput(viewName.getTableName(), encodeViewData(newDefinition), session.getUser(), createViewProperties(session)); try { diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/TrinoHiveCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/TrinoHiveCatalog.java index b975e03cbfe0..a7f925f7b62a 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/TrinoHiveCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/hms/TrinoHiveCatalog.java @@ -52,6 +52,7 @@ import java.io.IOException; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; @@ -329,6 +330,33 @@ public void updateViewComment(ConnectorSession session, SchemaTableName viewName definition.getOwner(), definition.isRunAsInvoker()); + replaceView(session, viewName, view, newDefinition); + } + + @Override + public void updateViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + io.trino.plugin.hive.metastore.Table view = metastore.getTable(viewName.getSchemaName(), viewName.getTableName()) + .orElseThrow(() -> new ViewNotFoundException(viewName)); + + ConnectorViewDefinition definition = getView(viewName, view.getViewOriginalText(), view.getTableType(), view.getParameters(), view.getOwner()) + .orElseThrow(() -> new ViewNotFoundException(viewName)); + ConnectorViewDefinition newDefinition = new ConnectorViewDefinition( + definition.getOriginalSql(), + definition.getCatalog(), + definition.getSchema(), + definition.getColumns().stream() + .map(currentViewColumn -> Objects.equals(columnName, currentViewColumn.getName()) ? new ConnectorViewDefinition.ViewColumn(currentViewColumn.getName(), currentViewColumn.getType(), comment) : currentViewColumn) + .collect(toImmutableList()), + definition.getComment(), + definition.getOwner(), + definition.isRunAsInvoker()); + + replaceView(session, viewName, view, newDefinition); + } + + private void replaceView(ConnectorSession session, SchemaTableName viewName, io.trino.plugin.hive.metastore.Table view, ConnectorViewDefinition newDefinition) + { io.trino.plugin.hive.metastore.Table.Builder viewBuilder = io.trino.plugin.hive.metastore.Table.builder(view) .setViewOriginalText(Optional.of(encodeViewData(newDefinition))); diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java index e493f6723160..61c91dd4cfbe 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java @@ -172,6 +172,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior) return false; case SUPPORTS_COMMENT_ON_VIEW: + case SUPPORTS_COMMENT_ON_VIEW_COLUMN: return true; case SUPPORTS_CREATE_VIEW: diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java index a151f5affa87..00d6b48b5f03 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/BaseTrinoCatalogTest.java @@ -273,7 +273,7 @@ public void testView() Optional.empty(), Optional.empty(), ImmutableList.of( - new ConnectorViewDefinition.ViewColumn("name", VarcharType.createVarcharType(25).getTypeId())), + new ConnectorViewDefinition.ViewColumn("name", VarcharType.createVarcharType(25).getTypeId(), Optional.empty())), Optional.empty(), Optional.of(SESSION.getUser()), false); diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogConnectorSmokeTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogConnectorSmokeTest.java index 72fb45cb935f..cad44550c4d8 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogConnectorSmokeTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/catalog/glue/TestIcebergGlueCatalogConnectorSmokeTest.java @@ -160,11 +160,40 @@ public void testCommentView() } } + @Test + public void testCommentViewColumn() + { + // TODO: Consider moving to BaseConnectorSmokeTest + String viewColumnName = "regionkey"; + try (TestView view = new TestView(getQueryRunner()::execute, "test_comment_view", "SELECT * FROM region")) { + // comment set + assertUpdate("COMMENT ON COLUMN " + view.getName() + "." + viewColumnName + " IS 'new region key comment'"); + assertThat(getColumnComment(view.getName(), viewColumnName)).isEqualTo("new region key comment"); + + // comment updated + assertUpdate("COMMENT ON COLUMN " + view.getName() + "." + viewColumnName + " IS 'updated region key comment'"); + assertThat(getColumnComment(view.getName(), viewColumnName)).isEqualTo("updated region key comment"); + + // comment set to empty + assertUpdate("COMMENT ON COLUMN " + view.getName() + "." + viewColumnName + " IS ''"); + assertThat(getColumnComment(view.getName(), viewColumnName)).isEqualTo(""); + + // comment deleted + assertUpdate("COMMENT ON COLUMN " + view.getName() + "." + viewColumnName + " IS NULL"); + assertThat(getColumnComment(view.getName(), viewColumnName)).isEqualTo(null); + } + } + private String getTableComment(String tableName) { return (String) computeScalar("SELECT comment FROM system.metadata.table_comments WHERE catalog_name = 'iceberg' AND schema_name = '" + schemaName + "' AND table_name = '" + tableName + "'"); } + protected String getColumnComment(String tableName, String columnName) + { + return (String) computeScalar("SELECT comment FROM information_schema.columns WHERE table_schema = '" + getSession().getSchema().orElseThrow() + "' AND table_name = '" + tableName + "' AND column_name = '" + columnName + "'"); + } + private String schemaPath() { return format("s3://%s/%s", bucketName, schemaName); diff --git a/plugin/trino-memory/src/main/java/io/trino/plugin/memory/MemoryMetadata.java b/plugin/trino-memory/src/main/java/io/trino/plugin/memory/MemoryMetadata.java index 4c4719b010ea..ab19f25f75f5 100644 --- a/plugin/trino-memory/src/main/java/io/trino/plugin/memory/MemoryMetadata.java +++ b/plugin/trino-memory/src/main/java/io/trino/plugin/memory/MemoryMetadata.java @@ -336,6 +336,22 @@ public synchronized void setViewComment(ConnectorSession session, SchemaTableNam view.isRunAsInvoker())); } + @Override + public synchronized void setViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + ConnectorViewDefinition view = getView(session, viewName).orElseThrow(() -> new ViewNotFoundException(viewName)); + views.put(viewName, new ConnectorViewDefinition( + view.getOriginalSql(), + view.getCatalog(), + view.getSchema(), + view.getColumns().stream() + .map(currentViewColumn -> Objects.equals(columnName, currentViewColumn.getName()) ? new ConnectorViewDefinition.ViewColumn(currentViewColumn.getName(), currentViewColumn.getType(), comment) : currentViewColumn) + .collect(toImmutableList()), + view.getComment(), + view.getOwner(), + view.isRunAsInvoker())); + } + @Override public synchronized void renameView(ConnectorSession session, SchemaTableName viewName, SchemaTableName newViewName) { diff --git a/plugin/trino-memory/src/test/java/io/trino/plugin/memory/TestMemoryConnectorTest.java b/plugin/trino-memory/src/test/java/io/trino/plugin/memory/TestMemoryConnectorTest.java index 3dca1cd93725..35e098df0e14 100644 --- a/plugin/trino-memory/src/test/java/io/trino/plugin/memory/TestMemoryConnectorTest.java +++ b/plugin/trino-memory/src/test/java/io/trino/plugin/memory/TestMemoryConnectorTest.java @@ -98,6 +98,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior) return false; case SUPPORTS_COMMENT_ON_VIEW: + case SUPPORTS_COMMENT_ON_VIEW_COLUMN: return true; case SUPPORTS_CREATE_VIEW: diff --git a/plugin/trino-memory/src/test/java/io/trino/plugin/memory/TestMemoryMetadata.java b/plugin/trino-memory/src/test/java/io/trino/plugin/memory/TestMemoryMetadata.java index 23803a28072c..8da0ac175d82 100644 --- a/plugin/trino-memory/src/test/java/io/trino/plugin/memory/TestMemoryMetadata.java +++ b/plugin/trino-memory/src/test/java/io/trino/plugin/memory/TestMemoryMetadata.java @@ -346,7 +346,7 @@ private static ConnectorViewDefinition testingViewDefinition(String sql) sql, Optional.empty(), Optional.empty(), - ImmutableList.of(new ViewColumn("test", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("test", BIGINT.getTypeId(), Optional.empty())), Optional.empty(), Optional.empty(), true); diff --git a/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/metadata/TestRaptorMetadata.java b/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/metadata/TestRaptorMetadata.java index 3fbb0be89051..803a24521a39 100644 --- a/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/metadata/TestRaptorMetadata.java +++ b/plugin/trino-raptor-legacy/src/test/java/io/trino/plugin/raptor/legacy/metadata/TestRaptorMetadata.java @@ -829,7 +829,7 @@ private static ConnectorViewDefinition testingViewDefinition(String sql) sql, Optional.empty(), Optional.empty(), - ImmutableList.of(new ViewColumn("test", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("test", BIGINT.getTypeId(), Optional.empty())), Optional.empty(), Optional.empty(), true); diff --git a/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestComments.java b/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestComments.java index e5c6ede8464f..54c5081bc1fa 100644 --- a/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestComments.java +++ b/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestComments.java @@ -102,6 +102,40 @@ private static String getTableComment(String catalogName, String schemaName, Str return (String) onTrino().executeQuery(sql).getOnlyValue(); } + @Test(groups = COMMENT) + public void testCommentViewColumn() + { + String columnName = "col"; + String createViewSql = format("" + + "CREATE VIEW hive.default.%s " + + "AS SELECT 1 AS %s", + COMMENT_VIEW_NAME, + columnName); + onTrino().executeQuery(createViewSql); + + assertThat(getColumnComment("default", COMMENT_VIEW_NAME, columnName)).isNull(); + + onTrino().executeQuery(format("COMMENT ON COLUMN %s.%s IS 'new col comment'", COMMENT_VIEW_NAME, columnName)); + assertThat(getColumnComment("default", COMMENT_VIEW_NAME, columnName)).isEqualTo("new col comment"); + + onTrino().executeQuery(format("COMMENT ON COLUMN %s.%s IS ''", COMMENT_VIEW_NAME, columnName)); + assertThat(getColumnComment("default", COMMENT_VIEW_NAME, columnName)).isEmpty(); + + onTrino().executeQuery(format("COMMENT ON COLUMN %s.%s IS NULL", COMMENT_VIEW_NAME, columnName)); + assertThat(getColumnComment("default", COMMENT_VIEW_NAME, columnName)).isNull(); + + onTrino().executeQuery(format("CREATE TABLE hive.default.%s (col int)", COMMENT_TABLE_NAME)); + onHive().executeQuery(format("CREATE VIEW default.%s AS SELECT * FROM default.%s", COMMENT_HIVE_VIEW_NAME, COMMENT_TABLE_NAME)); + assertThatThrownBy(() -> onTrino().executeQuery(format("COMMENT ON COLUMN %s.%s IS NULL", COMMENT_HIVE_VIEW_NAME, columnName))) + .hasMessageContaining("Hive views are not supported"); + } + + protected String getColumnComment(String schemaName, String tableName, String columnName) + { + String sql = "SELECT comment FROM information_schema.columns WHERE table_schema = '" + schemaName + "' AND table_name = '" + tableName + "' AND column_name = '" + columnName + "'"; + return (String) onTrino().executeQuery(sql).getOnlyValue(); + } + @Test(groups = COMMENT) public void testCommentColumn() { diff --git a/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestHiveViewCompatibility.java b/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestHiveViewCompatibility.java index 5fd67f885db8..2a011fe138e3 100644 --- a/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestHiveViewCompatibility.java +++ b/testing/trino-product-tests/src/main/java/io/trino/tests/product/hive/TestHiveViewCompatibility.java @@ -73,6 +73,26 @@ public void testExistingView() .hasMessageContaining("View already exists"); } + @Test(groups = {HIVE_VIEW_COMPATIBILITY, PROFILE_SPECIFIC_TESTS}) + public void testCommentOnViewColumn() + { + onTrino().executeQuery("DROP VIEW IF EXISTS hive_test_view_comment_column"); + onTrino().executeQuery("CREATE VIEW hive_test_view_comment_column AS SELECT * FROM nation"); + onCompatibilityTestServer().executeQuery("DROP VIEW IF EXISTS hive_test_view_comment_column_compatibility"); + onCompatibilityTestServer().executeQuery("CREATE VIEW hive_test_view_comment_column_compatibility AS SELECT * FROM nation"); + + assertViewQuery(onCompatibilityTestServer(), "SELECT * FROM hive_test_view_comment_column", queryAssert -> queryAssert.hasRowsCount(25)); + assertViewQuery(onTrino(), "SELECT * FROM hive_test_view_comment_column_compatibility", queryAssert -> queryAssert.hasRowsCount(25)); + + // Verify that the views are still readable after adding a comment on one of their columns + onTrino().executeQuery("COMMENT ON COLUMN hive_test_view_comment_column.n_nationkey IS 'ID of the nation'"); + assertViewQuery(onCompatibilityTestServer(), "SELECT * FROM hive_test_view_comment_column", queryAssert -> queryAssert.hasRowsCount(25)); + + onTrino().executeQuery("COMMENT ON COLUMN hive_test_view_comment_column_compatibility.n_nationkey IS 'ID of the nation'"); + assertViewQuery(onCompatibilityTestServer(), "SELECT * FROM hive_test_view_comment_column_compatibility", queryAssert -> queryAssert.hasRowsCount(25)); + assertViewQuery(onTrino(), "SELECT * FROM hive_test_view_comment_column_compatibility", queryAssert -> queryAssert.hasRowsCount(25)); + } + protected static void assertViewQuery(QueryExecutor queryExecutor, String query, Consumer assertion) { // Ensure view compatibility by comparing the results diff --git a/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java b/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java index bff9b7520c7f..25ec5b6ee518 100644 --- a/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java +++ b/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java @@ -83,6 +83,7 @@ import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_COMMENT_ON_COLUMN; import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_COMMENT_ON_TABLE; import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_COMMENT_ON_VIEW; +import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_COMMENT_ON_VIEW_COLUMN; import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_CREATE_MATERIALIZED_VIEW; import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_CREATE_SCHEMA; import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_CREATE_TABLE; @@ -2829,6 +2830,39 @@ public void testCommentColumn() } } + @Test + public void testCommentViewColumn() + { + if (!hasBehavior(SUPPORTS_COMMENT_ON_VIEW_COLUMN)) { + if (hasBehavior(SUPPORTS_CREATE_VIEW)) { + try (TestView view = new TestView(getQueryRunner()::execute, "test_comment_view_column", "SELECT * FROM region")) { + assertQueryFails("COMMENT ON COLUMN " + view.getName() + ".regionkey IS 'new region key comment'", "This connector does not support setting view column comments"); + } + return; + } + throw new SkipException("Skipping as connector does not support CREATE VIEW"); + } + + String viewColumnName = "regionkey"; + try (TestView view = new TestView(getQueryRunner()::execute, "test_comment_view_column", "SELECT * FROM region")) { + // comment set + assertUpdate("COMMENT ON COLUMN " + view.getName() + "." + viewColumnName + " IS 'new region key comment'"); + assertThat(getColumnComment(view.getName(), viewColumnName)).isEqualTo("new region key comment"); + + // comment updated + assertUpdate("COMMENT ON COLUMN " + view.getName() + "." + viewColumnName + " IS 'updated region key comment'"); + assertThat(getColumnComment(view.getName(), viewColumnName)).isEqualTo("updated region key comment"); + + // comment set to empty + assertUpdate("COMMENT ON COLUMN " + view.getName() + "." + viewColumnName + " IS ''"); + assertThat(getColumnComment(view.getName(), viewColumnName)).isEqualTo(""); + + // comment deleted + assertUpdate("COMMENT ON COLUMN " + view.getName() + "." + viewColumnName + " IS NULL"); + assertThat(getColumnComment(view.getName(), viewColumnName)).isEqualTo(null); + } + } + protected String getColumnComment(String tableName, String columnName) { return (String) computeScalar(format( diff --git a/testing/trino-testing/src/main/java/io/trino/testing/TestingConnectorBehavior.java b/testing/trino-testing/src/main/java/io/trino/testing/TestingConnectorBehavior.java index d4426301f9e9..13ce0b7eafae 100644 --- a/testing/trino-testing/src/main/java/io/trino/testing/TestingConnectorBehavior.java +++ b/testing/trino-testing/src/main/java/io/trino/testing/TestingConnectorBehavior.java @@ -69,6 +69,7 @@ public enum TestingConnectorBehavior SUPPORTS_COMMENT_ON_TABLE, SUPPORTS_COMMENT_ON_VIEW(false), SUPPORTS_COMMENT_ON_COLUMN, + SUPPORTS_COMMENT_ON_VIEW_COLUMN(false), SUPPORTS_CREATE_VIEW(false), diff --git a/testing/trino-tests/src/test/java/io/trino/execution/TestEventListenerBasic.java b/testing/trino-tests/src/test/java/io/trino/execution/TestEventListenerBasic.java index 40938d31668d..7a2b13f3d2fa 100644 --- a/testing/trino-tests/src/test/java/io/trino/execution/TestEventListenerBasic.java +++ b/testing/trino-tests/src/test/java/io/trino/execution/TestEventListenerBasic.java @@ -150,7 +150,7 @@ public Iterable getConnectorFactories() "SELECT nationkey AS test_column FROM tpch.tiny.nation", Optional.empty(), Optional.empty(), - ImmutableList.of(new ConnectorViewDefinition.ViewColumn("test_column", BIGINT.getTypeId())), + ImmutableList.of(new ConnectorViewDefinition.ViewColumn("test_column", BIGINT.getTypeId(), Optional.empty())), Optional.empty(), Optional.empty(), true); diff --git a/testing/trino-tests/src/test/java/io/trino/security/TestAccessControl.java b/testing/trino-tests/src/test/java/io/trino/security/TestAccessControl.java index 04a49b808a52..b5c90af88c51 100644 --- a/testing/trino-tests/src/test/java/io/trino/security/TestAccessControl.java +++ b/testing/trino-tests/src/test/java/io/trino/security/TestAccessControl.java @@ -48,6 +48,7 @@ import static io.trino.spi.security.SelectedRole.Type.ROLE; import static io.trino.spi.type.BigintType.BIGINT; import static io.trino.testing.TestingAccessControlManager.TestingPrivilegeType.ADD_COLUMN; +import static io.trino.testing.TestingAccessControlManager.TestingPrivilegeType.COMMENT_COLUMN; import static io.trino.testing.TestingAccessControlManager.TestingPrivilegeType.COMMENT_VIEW; import static io.trino.testing.TestingAccessControlManager.TestingPrivilegeType.CREATE_MATERIALIZED_VIEW; import static io.trino.testing.TestingAccessControlManager.TestingPrivilegeType.CREATE_TABLE; @@ -103,7 +104,7 @@ protected QueryRunner createQueryRunner() "select 1", Optional.of("mock"), Optional.of("default"), - ImmutableList.of(new ConnectorViewDefinition.ViewColumn("test", BIGINT.getTypeId())), + ImmutableList.of(new ConnectorViewDefinition.ViewColumn("test", BIGINT.getTypeId(), Optional.empty())), Optional.of("comment"), Optional.of("admin"), false); @@ -111,7 +112,7 @@ protected QueryRunner createQueryRunner() "select 1", Optional.of("mock"), Optional.of("default"), - ImmutableList.of(new ConnectorViewDefinition.ViewColumn("test", BIGINT.getTypeId())), + ImmutableList.of(new ConnectorViewDefinition.ViewColumn("test", BIGINT.getTypeId(), Optional.empty())), Optional.of("comment"), Optional.empty(), true); @@ -396,6 +397,15 @@ public void testViewWithTableFunction(boolean securityDefiner) assertUpdate("DROP VIEW " + viewName); } + @Test + public void testCommentColumnView() + { + String viewName = "comment_view" + randomTableSuffix(); + assertUpdate("CREATE VIEW " + viewName + " AS SELECT * FROM orders"); + assertAccessDenied("COMMENT ON COLUMN " + viewName + ".orderkey IS 'new order key comment'", "Cannot comment column to .*", privilege(viewName, COMMENT_COLUMN)); + assertUpdate(getSession(), "COMMENT ON COLUMN " + viewName + ".orderkey IS 'new comment'"); + } + @Test public void testSetTableProperties() { diff --git a/testing/trino-tests/src/test/java/io/trino/tests/TestMetadataManager.java b/testing/trino-tests/src/test/java/io/trino/tests/TestMetadataManager.java index b95412fe71dc..78a876f54696 100644 --- a/testing/trino-tests/src/test/java/io/trino/tests/TestMetadataManager.java +++ b/testing/trino-tests/src/test/java/io/trino/tests/TestMetadataManager.java @@ -212,7 +212,7 @@ private static ConnectorViewDefinition getConnectorViewDefinition() "test view SQL", Optional.of("upper_case_schema_catalog"), Optional.of("upper_case_schema"), - ImmutableList.of(new ConnectorViewDefinition.ViewColumn("col", BIGINT.getTypeId())), + ImmutableList.of(new ConnectorViewDefinition.ViewColumn("col", BIGINT.getTypeId(), Optional.empty())), Optional.of("comment"), Optional.of("test_owner"), false); diff --git a/testing/trino-tests/src/test/java/io/trino/tests/TestMockConnector.java b/testing/trino-tests/src/test/java/io/trino/tests/TestMockConnector.java index af765d1c66cd..c96b7e07438b 100644 --- a/testing/trino-tests/src/test/java/io/trino/tests/TestMockConnector.java +++ b/testing/trino-tests/src/test/java/io/trino/tests/TestMockConnector.java @@ -84,7 +84,7 @@ protected QueryRunner createQueryRunner() "SELECT nationkey FROM mock.default.test_table", Optional.of("mock"), Optional.of("default"), - ImmutableList.of(new ViewColumn("nationkey", BIGINT.getTypeId())), + ImmutableList.of(new ViewColumn("nationkey", BIGINT.getTypeId(), Optional.empty())), Optional.empty(), Optional.of("alice"), false)))