From ecbd733cc023afb26d12f300f04b4930f7f1a852 Mon Sep 17 00:00:00 2001 From: Vladyslav Lyutenko Date: Sat, 29 Jul 2023 14:43:41 +0200 Subject: [PATCH 1/3] Add support for materialized views in Blackhole connector --- plugin/trino-blackhole/pom.xml | 6 +++ .../plugin/blackhole/BlackHoleMetadata.java | 47 +++++++++++++++++++ .../plugin/blackhole/TestBlackHoleSmoke.java | 22 +++++++++ 3 files changed, 75 insertions(+) diff --git a/plugin/trino-blackhole/pom.xml b/plugin/trino-blackhole/pom.xml index 4dc313444991..c9db0afa303c 100644 --- a/plugin/trino-blackhole/pom.xml +++ b/plugin/trino-blackhole/pom.xml @@ -104,6 +104,12 @@ test + + io.trino + trino-testing-services + test + + io.trino trino-tpch 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 b21835626586..4dd60e2d0b52 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 @@ -24,6 +24,7 @@ import io.trino.spi.connector.ColumnMetadata; import io.trino.spi.connector.ConnectorAnalyzeMetadata; import io.trino.spi.connector.ConnectorInsertTableHandle; +import io.trino.spi.connector.ConnectorMaterializedViewDefinition; import io.trino.spi.connector.ConnectorMergeTableHandle; import io.trino.spi.connector.ConnectorMetadata; import io.trino.spi.connector.ConnectorOutputMetadata; @@ -33,6 +34,7 @@ import io.trino.spi.connector.ConnectorTableLayout; import io.trino.spi.connector.ConnectorTableMetadata; import io.trino.spi.connector.ConnectorViewDefinition; +import io.trino.spi.connector.MaterializedViewFreshness; import io.trino.spi.connector.RetryMode; import io.trino.spi.connector.RowChangeParadigm; import io.trino.spi.connector.SchemaNotFoundException; @@ -71,6 +73,7 @@ import static io.trino.plugin.blackhole.BlackHolePageSourceProvider.isNumericType; import static io.trino.spi.StandardErrorCode.ALREADY_EXISTS; import static io.trino.spi.StandardErrorCode.INVALID_TABLE_PROPERTY; +import static io.trino.spi.connector.MaterializedViewFreshness.Freshness.STALE; import static io.trino.spi.connector.RetryMode.NO_RETRIES; import static io.trino.spi.connector.RowChangeParadigm.DELETE_ROW_AND_INSERT_ROW; import static io.trino.spi.type.BigintType.BIGINT; @@ -86,6 +89,7 @@ public class BlackHoleMetadata private final List schemas = new ArrayList<>(); private final Map tables = new ConcurrentHashMap<>(); private final Map views = new ConcurrentHashMap<>(); + private final Map materializedViews = new ConcurrentHashMap<>(); public BlackHoleMetadata() { @@ -147,6 +151,7 @@ public List listTables(ConnectorSession session, Optional getMaterializedView(ConnectorSession session, SchemaTableName viewName) + { + return Optional.ofNullable(materializedViews.get(viewName)); + } + + @Override + public Map getMaterializedViews(ConnectorSession session, Optional schemaName) + { + return ImmutableMap.copyOf(materializedViews); + } + + @Override + public MaterializedViewFreshness getMaterializedViewFreshness(ConnectorSession session, SchemaTableName name) + { + return new MaterializedViewFreshness(STALE, Optional.empty()); + } + + @Override + public List listMaterializedViews(ConnectorSession session, Optional schemaName) + { + return ImmutableList.copyOf(materializedViews.keySet()); + } } diff --git a/plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/TestBlackHoleSmoke.java b/plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/TestBlackHoleSmoke.java index b53ce06badb1..7b151a4de972 100644 --- a/plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/TestBlackHoleSmoke.java +++ b/plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/TestBlackHoleSmoke.java @@ -38,11 +38,13 @@ import static io.trino.plugin.blackhole.BlackHoleConnector.ROWS_PER_PAGE_PROPERTY; import static io.trino.plugin.blackhole.BlackHoleConnector.SPLIT_COUNT_PROPERTY; import static io.trino.plugin.blackhole.BlackHoleQueryRunner.createQueryRunner; +import static io.trino.testing.TestingNames.randomNameSuffix; import static io.trino.testing.TestingSession.testSessionBuilder; import static java.lang.String.format; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.SECONDS; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.testng.Assert.assertEquals; @@ -174,6 +176,26 @@ public void dataGenerationUsage() assertThatQueryDoesNotReturnValues("DROP TABLE nation"); } + @Test + public void testMaterializedView() + { + String viewName = "test_materialized_view_" + randomNameSuffix(); + queryRunner.execute("CREATE MATERIALIZED VIEW " + viewName + " AS SELECT * FROM tpch.tiny.nation"); + + try { + // reading + MaterializedResult rows = queryRunner.execute("SELECT * FROM " + viewName); + assertEquals(rows.getRowCount(), 25); + + // listing + assertThat(queryRunner.execute("SHOW TABLES").getOnlyColumnAsSet()) + .contains(viewName); + } + finally { + assertThatQueryDoesNotReturnValues("DROP MATERIALIZED VIEW " + viewName); + } + } + @Test public void fieldLength() { From 39fac32f7e299f84117377581d5057dcb038474a Mon Sep 17 00:00:00 2001 From: Vladyslav Lyutenko Date: Sat, 29 Jul 2023 14:46:34 +0200 Subject: [PATCH 2/3] Add support for materialized view column comments in engine and blackhole --- .../java/io/trino/execution/CommentTask.java | 24 +++++++++----- .../metadata/MaterializedViewDefinition.java | 2 +- .../main/java/io/trino/metadata/Metadata.java | 5 +++ .../io/trino/metadata/MetadataManager.java | 15 ++++++++- .../main/java/io/trino/metadata/ViewInfo.java | 2 +- .../tracing/TracingConnectorMetadata.java | 9 +++++ .../io/trino/tracing/TracingMetadata.java | 9 +++++ .../execution/BaseDataDefinitionTaskTest.java | 20 +++++++++++ .../io/trino/execution/TestCommentTask.java | 19 +++++++++++ .../trino/metadata/AbstractMockMetadata.java | 6 ++++ .../ConnectorMaterializedViewDefinition.java | 18 ++++++++-- .../spi/connector/ConnectorMetadata.java | 8 +++++ .../ClassLoaderSafeConnectorMetadata.java | 8 +++++ .../plugin/blackhole/BlackHoleMetadata.java | 20 +++++++++++ .../plugin/blackhole/TestBlackHoleSmoke.java | 33 +++++++++++++++++++ .../hive/HiveMaterializedViewMetadata.java | 2 ++ .../io/trino/plugin/hive/HiveMetadata.java | 6 ++++ .../NoneHiveMaterializedViewMetadata.java | 6 ++++ .../io/trino/security/TestAccessControl.java | 9 +++++ 19 files changed, 208 insertions(+), 13 deletions(-) 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 108b7ed5922c..6423740d4f4e 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 @@ -17,6 +17,7 @@ import com.google.inject.Inject; import io.trino.Session; import io.trino.execution.warnings.WarningCollector; +import io.trino.metadata.MaterializedViewDefinition; import io.trino.metadata.Metadata; import io.trino.metadata.QualifiedObjectName; import io.trino.metadata.RedirectionAwareTableHandle; @@ -140,18 +141,14 @@ private void commentOnColumn(Comment statement, Session session) 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: %s", columnName)); - - accessControl.checkCanSetColumnComment(session.toSecurityContext(), originalObjectName); + ViewColumn viewColumn = findAndCheckViewColumn(statement, session, viewDefinition, originalObjectName); metadata.setViewColumnComment(session, originalObjectName, viewColumn.getName(), statement.getComment()); } else if (metadata.isMaterializedView(session, originalObjectName)) { - throw semanticException(TABLE_NOT_FOUND, statement, "Setting comments on the columns of materialized views is unsupported"); + MaterializedViewDefinition materializedViewDefinition = metadata.getMaterializedView(session, originalObjectName).get(); + ViewColumn viewColumn = findAndCheckViewColumn(statement, session, materializedViewDefinition, originalObjectName); + metadata.setMaterializedViewColumnComment(session, originalObjectName, viewColumn.getName(), statement.getComment()); } else { RedirectionAwareTableHandle redirectionAwareTableHandle = metadata.getRedirectionAwareTableHandle(session, originalObjectName); @@ -171,4 +168,15 @@ else if (metadata.isMaterializedView(session, originalObjectName)) { metadata.setColumnComment(session, tableHandle, columnHandles.get(columnName), statement.getComment()); } } + + private ViewColumn findAndCheckViewColumn(Comment statement, Session session, ViewDefinition viewDefinition, QualifiedObjectName originalObjectName) + { + String columnName = statement.getName().getSuffix(); + ViewColumn viewColumn = viewDefinition.getColumns().stream() + .filter(column -> column.getName().equals(columnName)) + .findAny() + .orElseThrow(() -> semanticException(COLUMN_NOT_FOUND, statement, "Column does not exist: %s", columnName)); + accessControl.checkCanSetColumnComment(session.toSecurityContext(), originalObjectName); + return viewColumn; + } } 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 76a961eb8922..26c5d47b57dd 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 @@ -92,7 +92,7 @@ public ConnectorMaterializedViewDefinition toConnectorMaterializedViewDefinition getCatalog(), getSchema(), getColumns().stream() - .map(column -> new ConnectorMaterializedViewDefinition.Column(column.getName(), column.getType())) + .map(column -> new ConnectorMaterializedViewDefinition.Column(column.getName(), column.getType(), column.getComment())) .collect(toImmutableList()), getGracePeriod(), getComment(), 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 004139f67f8f..8089238fe39c 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 @@ -227,6 +227,11 @@ Optional getTableHandleForExecute( */ void setViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional comment); + /** + * Comments to the specified materialized view column. + */ + void setMaterializedViewColumnComment(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 584399c2d98a..29398d41665b 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 @@ -609,7 +609,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 materialized view: %s", column.getType(), column.getName(), entry.getKey())); @@ -736,6 +740,15 @@ public void setViewColumnComment(Session session, QualifiedObjectName viewName, metadata.setViewColumnComment(session.toConnectorSession(catalogHandle), viewName.asSchemaTableName(), columnName, comment); } + @Override + public void setMaterializedViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional comment) + { + CatalogMetadata catalogMetadata = getCatalogMetadataForWrite(session, viewName.getCatalogName()); + CatalogHandle catalogHandle = catalogMetadata.getCatalogHandle(); + ConnectorMetadata metadata = catalogMetadata.getMetadata(session); + metadata.setMaterializedViewColumnComment(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/ViewInfo.java b/core/trino-main/src/main/java/io/trino/metadata/ViewInfo.java index 91e2755585f2..9706820b5a4d 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 @@ -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(), Optional.empty())) + .map(column -> new ViewColumn(column.getName(), column.getType(), column.getComment())) .collect(toImmutableList()); this.comment = viewDefinition.getComment(); this.storageTable = viewDefinition.getStorageTable(); diff --git a/core/trino-main/src/main/java/io/trino/tracing/TracingConnectorMetadata.java b/core/trino-main/src/main/java/io/trino/tracing/TracingConnectorMetadata.java index b56338301f90..fedb6179e625 100644 --- a/core/trino-main/src/main/java/io/trino/tracing/TracingConnectorMetadata.java +++ b/core/trino-main/src/main/java/io/trino/tracing/TracingConnectorMetadata.java @@ -423,6 +423,15 @@ public void setViewColumnComment(ConnectorSession session, SchemaTableName viewN } } + @Override + public void setMaterializedViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + Span span = startSpan("setMaterializedViewColumnComment", viewName); + try (var ignored = scopedSpan(span)) { + delegate.setMaterializedViewColumnComment(session, viewName, columnName, comment); + } + } + @Override public void setColumnComment(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnHandle column, Optional comment) { diff --git a/core/trino-main/src/main/java/io/trino/tracing/TracingMetadata.java b/core/trino-main/src/main/java/io/trino/tracing/TracingMetadata.java index 8a4c64cc4afd..dec860f71285 100644 --- a/core/trino-main/src/main/java/io/trino/tracing/TracingMetadata.java +++ b/core/trino-main/src/main/java/io/trino/tracing/TracingMetadata.java @@ -422,6 +422,15 @@ public void setViewColumnComment(Session session, QualifiedObjectName viewName, } } + @Override + public void setMaterializedViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional comment) + { + Span span = startSpan("setMaterializedViewColumnComment", viewName); + try (var ignored = scopedSpan(span)) { + delegate.setMaterializedViewColumnComment(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/execution/BaseDataDefinitionTaskTest.java b/core/trino-main/src/test/java/io/trino/execution/BaseDataDefinitionTaskTest.java index 8b1b84e9c7cc..6cab4b8243ad 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 @@ -576,6 +576,26 @@ public void setViewColumnComment(Session session, QualifiedObjectName viewName, view.getRunAsIdentity())); } + @Override + public void setMaterializedViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional comment) + { + MaterializedViewDefinition view = materializedViews.get(viewName.asSchemaTableName()); + materializedViews.put( + viewName.asSchemaTableName(), + new MaterializedViewDefinition( + 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.getGracePeriod(), + view.getComment(), + view.getRunAsIdentity().get(), + view.getStorageTable(), + view.getProperties())); + } + @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 6fa791b935e1..ef4c2baf3a81 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 @@ -141,6 +141,25 @@ public void testCommentViewColumn() .hasMessage("Column does not exist: %s", missingColumnName.getSuffix()); } + @Test + public void testCommentMaterializedViewColumn() + { + QualifiedObjectName materializedViewName = qualifiedObjectName("existing_materialized_view"); + metadata.createMaterializedView(testSession, QualifiedObjectName.valueOf(materializedViewName.toString()), someMaterializedView(), false, false); + assertThat(metadata.isMaterializedView(testSession, materializedViewName)).isTrue(); + + QualifiedName columnName = qualifiedColumnName("existing_materialized_view", "test"); + QualifiedName missingColumnName = qualifiedColumnName("existing_materialized_view", "missing"); + + getFutureValue(setComment(COLUMN, columnName, Optional.of("new test column comment"))); + assertThat(metadata.getMaterializedView(testSession, materializedViewName).get().getColumns().stream().filter(column -> "test".equals(column.getName())).collect(onlyElement()).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 a38dda8b79ad..5bdb9aff5299 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 @@ -294,6 +294,12 @@ public void setViewColumnComment(Session session, QualifiedObjectName viewName, throw new UnsupportedOperationException(); } + @Override + public void setMaterializedViewColumnComment(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-spi/src/main/java/io/trino/spi/connector/ConnectorMaterializedViewDefinition.java b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMaterializedViewDefinition.java index 27a4a59ad85d..5cded9795cda 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMaterializedViewDefinition.java +++ b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMaterializedViewDefinition.java @@ -182,11 +182,19 @@ public static final class Column { private final String name; private final TypeId type; + private final Optional comment; + @Deprecated public Column(String name, TypeId type) + { + this(name, type, Optional.empty()); + } + + public Column(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() @@ -199,6 +207,11 @@ public TypeId getType() return type; } + public Optional getComment() + { + return comment; + } + @Override public String toString() { @@ -216,13 +229,14 @@ public boolean equals(Object o) } Column column = (Column) o; return Objects.equals(name, column.name) && - Objects.equals(type, column.type); + Objects.equals(type, column.type) && + Objects.equals(comment, column.comment); } @Override public int hashCode() { - return Objects.hash(name, type); + return Objects.hash(name, type, comment); } } } 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 730af0bbc234..1737dce433af 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 @@ -434,6 +434,14 @@ default void setViewColumnComment(ConnectorSession session, SchemaTableName view throw new TrinoException(NOT_SUPPORTED, "This connector does not support setting view column comments"); } + /** + * Comments to the specified materialized view column. + */ + default void setMaterializedViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + throw new TrinoException(NOT_SUPPORTED, "This connector does not support setting materialized view column comments"); + } + /** * Comments to the specified column */ 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 19f452c80834..33709fef805b 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 @@ -487,6 +487,14 @@ public void setViewColumnComment(ConnectorSession session, SchemaTableName viewN } } + @Override + public void setMaterializedViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) { + delegate.setMaterializedViewColumnComment(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 4dd60e2d0b52..679c8450c88e 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 @@ -455,4 +455,24 @@ public List listMaterializedViews(ConnectorSession session, Opt { return ImmutableList.copyOf(materializedViews.keySet()); } + + @Override + public void setMaterializedViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + ConnectorMaterializedViewDefinition view = getMaterializedView(session, viewName).orElseThrow(() -> new ViewNotFoundException(viewName)); + materializedViews.put(viewName, new ConnectorMaterializedViewDefinition( + view.getOriginalSql(), + view.getStorageTable(), + view.getCatalog(), + view.getSchema(), + view.getColumns().stream() + .map(currentViewColumn -> Objects.equals(columnName, currentViewColumn.getName()) + ? new ConnectorMaterializedViewDefinition.Column(currentViewColumn.getName(), currentViewColumn.getType(), comment) + : currentViewColumn) + .collect(toImmutableList()), + view.getGracePeriod(), + view.getComment(), + view.getOwner(), + view.getProperties())); + } } diff --git a/plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/TestBlackHoleSmoke.java b/plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/TestBlackHoleSmoke.java index 7b151a4de972..56a68e5bd255 100644 --- a/plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/TestBlackHoleSmoke.java +++ b/plugin/trino-blackhole/src/test/java/io/trino/plugin/blackhole/TestBlackHoleSmoke.java @@ -196,6 +196,39 @@ public void testMaterializedView() } } + @Test + public void testCommentMaterializedViewColumn() + { + String viewName = "test_materialized_view_" + randomNameSuffix(); + try { + queryRunner.execute("CREATE MATERIALIZED VIEW " + viewName + " AS SELECT * FROM tpch.tiny.nation"); + + // comment set + queryRunner.execute("COMMENT ON COLUMN " + viewName + ".regionkey IS 'new region key comment'"); + assertThat(getColumnComment(viewName, "regionkey")).isEqualTo("new region key comment"); + + // comment updated + queryRunner.execute("COMMENT ON COLUMN " + viewName + ".regionkey IS 'updated region key comment'"); + assertThat(getColumnComment(viewName, "regionkey")).isEqualTo("updated region key comment"); + + // comment set to empty + queryRunner.execute("COMMENT ON COLUMN " + viewName + ".regionkey IS ''"); + assertThat(getColumnComment(viewName, "regionkey")).isEqualTo(""); + + // comment deleted + queryRunner.execute("COMMENT ON COLUMN " + viewName + ".regionkey IS NULL"); + assertThat(getColumnComment(viewName, "regionkey")).isEqualTo(null); + } + finally { + assertThatQueryDoesNotReturnValues("DROP MATERIALIZED VIEW " + viewName); + } + } + + private String getColumnComment(String tableName, String columnName) + { + return (String) queryRunner.execute(format("SELECT comment FROM information_schema.columns WHERE table_schema = '%s' AND table_name = '%s' AND column_name = '%s'", "default", tableName, columnName)).getOnlyValue(); + } + @Test public void fieldLength() { diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMaterializedViewMetadata.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMaterializedViewMetadata.java index d932c92d3b26..f47ad0dba86c 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMaterializedViewMetadata.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMaterializedViewMetadata.java @@ -44,4 +44,6 @@ public interface HiveMaterializedViewMetadata void renameMaterializedView(ConnectorSession session, SchemaTableName existingViewName, SchemaTableName newViewName); void setMaterializedViewProperties(ConnectorSession session, SchemaTableName viewName, Map> properties); + + void setMaterializedViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment); } 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 fb4a399f239d..c16b7d0cdfab 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 @@ -1492,6 +1492,12 @@ public void setViewColumnComment(ConnectorSession session, SchemaTableName viewN replaceView(session, viewName, view, newDefinition); } + @Override + public void setMaterializedViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + hiveMaterializedViewMetadata.setMaterializedViewColumnComment(session, viewName, columnName, comment); + } + private Table getView(SchemaTableName viewName) { Table view = metastore.getTable(viewName.getSchemaName(), viewName.getTableName()) diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/NoneHiveMaterializedViewMetadata.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/NoneHiveMaterializedViewMetadata.java index 62bfc9e57033..a23d562b616d 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/NoneHiveMaterializedViewMetadata.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/NoneHiveMaterializedViewMetadata.java @@ -91,4 +91,10 @@ public void setMaterializedViewProperties(ConnectorSession session, SchemaTableN { throw new TrinoException(NOT_SUPPORTED, "This connector does not support setting materialized view properties"); } + + @Override + public void setMaterializedViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + throw new TrinoException(NOT_SUPPORTED, "This connector does not support setting materialized view column comment"); + } } 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 ceac32d03a99..79e895b09cda 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 @@ -610,6 +610,15 @@ public void testCommentColumnView() assertUpdate(getSession(), "COMMENT ON COLUMN " + viewName + ".orderkey IS 'new comment'"); } + @Test + public void testCommentColumnMaterializedView() + { + String viewName = "comment_materialized_view" + randomNameSuffix(); + assertUpdate("CREATE MATERIALIZED 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 testSetColumnType() { From 46968023ced668d1b4e904f3ac0599c49a670e37 Mon Sep 17 00:00:00 2001 From: Vladyslav Lyutenko Date: Sat, 29 Jul 2023 14:46:56 +0200 Subject: [PATCH 3/3] Add materialized view column support for Iceberg --- docs/src/main/sphinx/connector/iceberg.rst | 1 + .../IcebergMaterializedViewDefinition.java | 13 +++++- .../trino/plugin/iceberg/IcebergMetadata.java | 6 +++ .../iceberg/catalog/AbstractTrinoCatalog.java | 2 +- .../plugin/iceberg/catalog/TrinoCatalog.java | 2 + .../catalog/glue/TrinoGlueCatalog.java | 40 +++++++++++++++++ .../iceberg/catalog/hms/TrinoHiveCatalog.java | 43 ++++++++++++++++++ .../catalog/jdbc/TrinoJdbcCatalog.java | 6 +++ .../catalog/nessie/TrinoNessieCatalog.java | 6 +++ .../catalog/rest/TrinoRestCatalog.java | 6 +++ .../BaseIcebergMaterializedViewTest.java | 17 +++++++ .../trino/testing/BaseConnectorSmokeTest.java | 44 +++++++++++++++++++ .../io/trino/testing/BaseConnectorTest.java | 43 ++++++++++++++++++ .../testing/TestingConnectorBehavior.java | 1 + 14 files changed, 227 insertions(+), 3 deletions(-) diff --git a/docs/src/main/sphinx/connector/iceberg.rst b/docs/src/main/sphinx/connector/iceberg.rst index 64ebc4e446b3..60be60ee0e69 100644 --- a/docs/src/main/sphinx/connector/iceberg.rst +++ b/docs/src/main/sphinx/connector/iceberg.rst @@ -1277,6 +1277,7 @@ The Iceberg connector supports setting comments on the following objects: - tables - views - table columns +- materialized view columns The ``COMMENT`` option is supported on both the table and the table columns for the :doc:`/sql/create-table` operation. diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMaterializedViewDefinition.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMaterializedViewDefinition.java index a337c5b4b54f..dc6cd42e317a 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMaterializedViewDefinition.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMaterializedViewDefinition.java @@ -75,7 +75,7 @@ public static IcebergMaterializedViewDefinition fromConnectorMaterializedViewDef definition.getCatalog(), definition.getSchema(), definition.getColumns().stream() - .map(column -> new Column(column.getName(), column.getType())) + .map(column -> new Column(column.getName(), column.getType(), column.getComment())) .collect(toImmutableList()), definition.getGracePeriod(), definition.getComment()); @@ -159,14 +159,17 @@ public static final class Column { private final String name; private final TypeId type; + private final Optional comment; @JsonCreator public Column( @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 @@ -181,6 +184,12 @@ public TypeId getType() return type; } + @JsonProperty + public Optional getComment() + { + return comment; + } + @Override public String toString() { 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 0668248fac64..1c95436fca96 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 @@ -746,6 +746,12 @@ public void setViewColumnComment(ConnectorSession session, SchemaTableName viewN catalog.updateViewColumnComment(session, viewName, columnName, comment); } + @Override + public void setMaterializedViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + catalog.updateMaterializedViewColumnComment(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/AbstractTrinoCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractTrinoCatalog.java index 4139dbad6d9d..67514d55ba9c 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractTrinoCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/AbstractTrinoCatalog.java @@ -346,7 +346,7 @@ protected ConnectorMaterializedViewDefinition getMaterializedViewDefinition( definition.getCatalog(), definition.getSchema(), definition.getColumns().stream() - .map(column -> new ConnectorMaterializedViewDefinition.Column(column.getName(), column.getType())) + .map(column -> new ConnectorMaterializedViewDefinition.Column(column.getName(), column.getType(), column.getComment())) .collect(toImmutableList()), definition.getGracePeriod(), definition.getComment(), 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 fde0d71aaccc..e5abf59fa3bd 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 @@ -108,6 +108,8 @@ Transaction newCreateTableTransaction( void updateViewColumnComment(ConnectorSession session, SchemaTableName schemaViewName, String columnName, Optional comment); + void updateMaterializedViewColumnComment(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 de7a94d8dfb6..262f9320684d 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 @@ -1002,6 +1002,46 @@ public void createMaterializedView( } } + @Override + public void updateMaterializedViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + ConnectorMaterializedViewDefinition definition = doGetMaterializedView(session, viewName) + .orElseThrow(() -> new ViewNotFoundException(viewName)); + ConnectorMaterializedViewDefinition newDefinition = new ConnectorMaterializedViewDefinition( + definition.getOriginalSql(), + definition.getStorageTable(), + definition.getCatalog(), + definition.getSchema(), + definition.getColumns().stream() + .map(currentViewColumn -> Objects.equals(columnName, currentViewColumn.getName()) ? new ConnectorMaterializedViewDefinition.Column(currentViewColumn.getName(), currentViewColumn.getType(), comment) : currentViewColumn) + .collect(toImmutableList()), + definition.getGracePeriod(), + definition.getComment(), + definition.getOwner(), + definition.getProperties()); + + updateMaterializedView(session, viewName, newDefinition); + } + + private void updateMaterializedView(ConnectorSession session, SchemaTableName viewName, ConnectorMaterializedViewDefinition newDefinition) + { + TableInput materializedViewTableInput = getMaterializedViewTableInput( + viewName.getTableName(), + encodeMaterializedViewData(fromConnectorMaterializedViewDefinition(newDefinition)), + session.getUser(), + createMaterializedViewProperties(session, newDefinition.getStorageTable().orElseThrow().getSchemaTableName())); + + try { + stats.getUpdateTable().call(() -> + glueClient.updateTable(new UpdateTableRequest() + .withDatabaseName(viewName.getSchemaName()) + .withTableInput(materializedViewTableInput))); + } + catch (AmazonServiceException e) { + throw new TrinoException(ICEBERG_CATALOG_ERROR, e); + } + } + @Override public void dropMaterializedView(ConnectorSession session, SchemaTableName viewName) { 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 fcad353a56ee..0bb3426adab7 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 @@ -42,6 +42,7 @@ import io.trino.spi.connector.SchemaNotFoundException; import io.trino.spi.connector.SchemaTableName; import io.trino.spi.connector.TableNotFoundException; +import io.trino.spi.connector.ViewNotFoundException; import io.trino.spi.security.TrinoPrincipal; import io.trino.spi.type.TypeManager; import org.apache.iceberg.BaseTable; @@ -55,6 +56,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; @@ -528,6 +530,47 @@ public void createMaterializedView( metastore.createTable(table, principalPrivileges); } + @Override + public void updateMaterializedViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + io.trino.plugin.hive.metastore.Table existing = metastore.getTable(viewName.getSchemaName(), viewName.getTableName()) + .orElseThrow(() -> new ViewNotFoundException(viewName)); + + if (!isTrinoMaterializedView(existing.getTableType(), existing.getParameters())) { + throw new TrinoException(UNSUPPORTED_TABLE_TYPE, "Existing table is not a Materialized View: " + viewName); + } + ConnectorMaterializedViewDefinition definition = doGetMaterializedView(session, viewName) + .orElseThrow(() -> new ViewNotFoundException(viewName)); + + ConnectorMaterializedViewDefinition newDefinition = new ConnectorMaterializedViewDefinition( + definition.getOriginalSql(), + definition.getStorageTable(), + definition.getCatalog(), + definition.getSchema(), + definition.getColumns().stream() + .map(currentViewColumn -> Objects.equals(columnName, currentViewColumn.getName()) + ? new ConnectorMaterializedViewDefinition.Column(currentViewColumn.getName(), currentViewColumn.getType(), comment) + : currentViewColumn) + .collect(toImmutableList()), + definition.getGracePeriod(), + definition.getComment(), + definition.getOwner(), + definition.getProperties()); + + replaceMaterializedView(session, viewName, existing, newDefinition); + } + + private void replaceMaterializedView(ConnectorSession session, SchemaTableName viewName, io.trino.plugin.hive.metastore.Table view, ConnectorMaterializedViewDefinition newDefinition) + { + io.trino.plugin.hive.metastore.Table.Builder viewBuilder = io.trino.plugin.hive.metastore.Table.builder(view) + .setViewOriginalText(Optional.of( + encodeMaterializedViewData(fromConnectorMaterializedViewDefinition(newDefinition)))); + + PrincipalPrivileges principalPrivileges = isUsingSystemSecurity ? NO_PRIVILEGES : buildInitialPrivilegeSet(session.getUser()); + + metastore.replaceTable(viewName.getSchemaName(), viewName.getTableName(), viewBuilder.build(), principalPrivileges); + } + @Override public void dropMaterializedView(ConnectorSession session, SchemaTableName viewName) { diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/jdbc/TrinoJdbcCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/jdbc/TrinoJdbcCatalog.java index 190dc57d0042..3f559766d178 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/jdbc/TrinoJdbcCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/jdbc/TrinoJdbcCatalog.java @@ -285,6 +285,12 @@ public void updateViewColumnComment(ConnectorSession session, SchemaTableName sc throw new TrinoException(NOT_SUPPORTED, "updateViewColumnComment is not supported for Iceberg JDBC catalogs"); } + @Override + public void updateMaterializedViewColumnComment(ConnectorSession session, SchemaTableName schemaViewName, String columnName, Optional comment) + { + throw new TrinoException(NOT_SUPPORTED, "updateMaterializedViewColumnComment is not supported for Iceberg JDBC catalogs"); + } + @Override public String defaultTableLocation(ConnectorSession session, SchemaTableName schemaTableName) { diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java index 61c2995b530a..78d30282cc9a 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/nessie/TrinoNessieCatalog.java @@ -284,6 +284,12 @@ public void updateViewColumnComment(ConnectorSession session, SchemaTableName sc throw new TrinoException(NOT_SUPPORTED, "updateViewColumnComment is not supported for Iceberg Nessie catalogs"); } + @Override + public void updateMaterializedViewColumnComment(ConnectorSession session, SchemaTableName schemaViewName, String columnName, Optional comment) + { + throw new TrinoException(NOT_SUPPORTED, "updateMaterializedViewColumnComment is not supported for Iceberg Nessie catalogs"); + } + @Override public void setViewPrincipal(ConnectorSession session, SchemaTableName schemaViewName, TrinoPrincipal principal) { diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/TrinoRestCatalog.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/TrinoRestCatalog.java index dbcd55bce99f..33abb3370d88 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/TrinoRestCatalog.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/TrinoRestCatalog.java @@ -415,6 +415,12 @@ public void updateViewColumnComment(ConnectorSession session, SchemaTableName sc throw new TrinoException(NOT_SUPPORTED, "updateViewColumnComment is not supported for Iceberg REST catalog"); } + @Override + public void updateMaterializedViewColumnComment(ConnectorSession session, SchemaTableName schemaViewName, String columnName, Optional comment) + { + throw new TrinoException(NOT_SUPPORTED, "updateMaterializedViewColumnComment is not supported for Iceberg REST catalog"); + } + private SessionCatalog.SessionContext convert(ConnectorSession session) { return switch (sessionType) { diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergMaterializedViewTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergMaterializedViewTest.java index 476388c465b4..73eaa253a4ac 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergMaterializedViewTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergMaterializedViewTest.java @@ -85,6 +85,18 @@ public void testShowTables() assertUpdate("DROP MATERIALIZED VIEW materialized_view_show_tables_test"); } + @Test + public void testCommentColumnMaterializedView() + { + String viewColumnName = "_bigint"; + String materializedViewName = "test_materialized_view_" + randomNameSuffix(); + assertUpdate(format("CREATE MATERIALIZED VIEW %s AS SELECT * FROM base_table1", materializedViewName)); + assertUpdate(format("COMMENT ON COLUMN %s.%s IS 'new comment'", materializedViewName, viewColumnName)); + assertThat(getColumnComment(materializedViewName, viewColumnName)).isEqualTo("new comment"); + assertQuery(format("SELECT count(*) FROM %s", materializedViewName), "VALUES 6"); + assertUpdate(format("DROP MATERIALIZED VIEW %s", materializedViewName)); + } + @Test public void testMaterializedViewsMetadata() { @@ -777,6 +789,11 @@ public Object[][] testTemporalPartitioningDataProvider() }; } + 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 SchemaTableName getStorageTable(String materializedViewName) { return getStorageTable(getSession().getCatalog().orElseThrow(), getSession().getSchema().orElseThrow(), materializedViewName); diff --git a/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorSmokeTest.java b/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorSmokeTest.java index 18d576e42178..c518138e5426 100644 --- a/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorSmokeTest.java +++ b/testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorSmokeTest.java @@ -26,6 +26,7 @@ import java.util.regex.Pattern; import static io.trino.spi.connector.ConnectorMetadata.MODIFYING_ROWS_MESSAGE; +import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_COMMENT_ON_MATERIALIZED_VIEW_COLUMN; 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; @@ -660,6 +661,49 @@ public void testCommentViewColumn() } } + @Test + public void testCommentMaterializedViewColumn() + { + if (!hasBehavior(SUPPORTS_COMMENT_ON_MATERIALIZED_VIEW_COLUMN)) { + if (hasBehavior(SUPPORTS_CREATE_MATERIALIZED_VIEW)) { + String viewName = "test_materialized_view_" + randomNameSuffix(); + assertUpdate("CREATE MATERIALIZED VIEW " + viewName + " AS SELECT * FROM nation"); + assertQueryFails("COMMENT ON COLUMN " + viewName + ".regionkey IS 'new region key comment'", "This connector does not support setting materialized view column comments"); + assertUpdate("DROP MATERIALIZED VIEW " + viewName); + return; + } + throw new SkipException("Skipping as connector does not support MATERIALIZED VIEW COLUMN COMMENT"); + } + + String viewName = "test_materialized_view_" + randomNameSuffix(); + try { + assertUpdate("CREATE MATERIALIZED VIEW " + viewName + " AS SELECT * FROM nation"); + + // comment set + assertUpdate("COMMENT ON COLUMN " + viewName + ".regionkey IS 'new region key comment'"); + assertThat(getColumnComment(viewName, "regionkey")).isEqualTo("new region key comment"); + + // comment updated + assertUpdate("COMMENT ON COLUMN " + viewName + ".regionkey IS 'updated region key comment'"); + assertThat(getColumnComment(viewName, "regionkey")).isEqualTo("updated region key comment"); + + // refresh materialized view + assertUpdate("REFRESH MATERIALIZED VIEW " + viewName, 25); + assertThat(getColumnComment(viewName, "regionkey")).isEqualTo("updated region key comment"); + + // comment set to empty + assertUpdate("COMMENT ON COLUMN " + viewName + ".regionkey IS ''"); + assertThat(getColumnComment(viewName, "regionkey")).isEqualTo(""); + + // comment deleted + assertUpdate("COMMENT ON COLUMN " + viewName + ".regionkey IS NULL"); + assertThat(getColumnComment(viewName, "regionkey")).isEqualTo(null); + } + finally { + assertUpdate("DROP MATERIALIZED VIEW " + viewName); + } + } + protected String getTableComment(String tableName) { return (String) computeScalar(format("SELECT comment FROM system.metadata.table_comments WHERE catalog_name = '%s' AND schema_name = '%s' AND table_name = '%s'", getSession().getCatalog().orElseThrow(), getSession().getSchema().orElseThrow(), tableName)); 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 a99389e84943..38622ac7d3e3 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 @@ -114,6 +114,7 @@ import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_ADD_FIELD; import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_ARRAY; import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_COMMENT_ON_COLUMN; +import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_COMMENT_ON_MATERIALIZED_VIEW_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; @@ -1584,6 +1585,48 @@ private Optional getMaterializedViewLastFreshTime(String material return Optional.ofNullable(lastFreshTime); } + @Test + public void testColumnCommentMaterializedView() + { + if (!hasBehavior(SUPPORTS_COMMENT_ON_MATERIALIZED_VIEW_COLUMN)) { + if (hasBehavior(SUPPORTS_CREATE_MATERIALIZED_VIEW)) { + String viewName = "test_materialized_view_" + randomNameSuffix(); + assertUpdate("CREATE MATERIALIZED VIEW " + viewName + " AS SELECT * FROM nation"); + assertQueryFails("COMMENT ON COLUMN " + viewName + ".regionkey IS 'new region key comment'", "This connector does not support setting materialized view column comments"); + assertUpdate("DROP MATERIALIZED VIEW " + viewName); + return; + } + throw new SkipException("Skipping as connector does not support MATERIALIZED VIEW COLUMN COMMENT"); + } + + String viewName = "test_materialized_view_" + randomNameSuffix(); + try { + assertUpdate("CREATE MATERIALIZED VIEW " + viewName + " AS SELECT * FROM nation"); + + assertUpdate("COMMENT ON COLUMN " + viewName + ".name IS 'new comment'"); + assertThat(getColumnComment(viewName, "name")).isEqualTo("new comment"); + + // comment deleted + assertUpdate("COMMENT ON COLUMN " + viewName + ".name IS NULL"); + assertThat(getColumnComment(viewName, "name")).isEqualTo(null); + + // comment set to non-empty value before verifying setting empty comment + assertUpdate("COMMENT ON COLUMN " + viewName + ".name IS 'updated comment'"); + assertThat(getColumnComment(viewName, "name")).isEqualTo("updated comment"); + + // refresh materialized view + assertUpdate("REFRESH MATERIALIZED VIEW " + viewName, 25); + assertThat(getColumnComment(viewName, "name")).isEqualTo("updated comment"); + + // comment set to empty + assertUpdate("COMMENT ON COLUMN " + viewName + ".name IS ''"); + assertThat(getColumnComment(viewName, "name")).isEmpty(); + } + finally { + assertUpdate("DROP MATERIALIZED VIEW " + viewName); + } + } + @Test public void testCompatibleTypeChangeForView() { 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 13e61516672a..a7273c3b9975 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 @@ -102,6 +102,7 @@ public enum TestingConnectorBehavior SUPPORTS_MATERIALIZED_VIEW_FRESHNESS_FROM_BASE_TABLES(SUPPORTS_CREATE_MATERIALIZED_VIEW), SUPPORTS_RENAME_MATERIALIZED_VIEW(SUPPORTS_CREATE_MATERIALIZED_VIEW), SUPPORTS_RENAME_MATERIALIZED_VIEW_ACROSS_SCHEMAS(SUPPORTS_RENAME_MATERIALIZED_VIEW), + SUPPORTS_COMMENT_ON_MATERIALIZED_VIEW_COLUMN(SUPPORTS_CREATE_MATERIALIZED_VIEW), SUPPORTS_NOT_NULL_CONSTRAINT(SUPPORTS_CREATE_TABLE), SUPPORTS_ADD_COLUMN_NOT_NULL_CONSTRAINT(and(SUPPORTS_NOT_NULL_CONSTRAINT, SUPPORTS_ADD_COLUMN)),