diff --git a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java index c1adb40e9794..3cb0f505847e 100644 --- a/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java +++ b/plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java @@ -1242,6 +1242,12 @@ public void setViewComment(ConnectorSession session, SchemaTableName viewName, O trinoViewHiveMetastore.updateViewComment(session, viewName, comment); } + @Override + public void setViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional comment) + { + trinoViewHiveMetastore.updateViewColumnComment(session, viewName, columnName, comment); + } + @Override public void addColumn(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnMetadata newColumnMetadata) { diff --git a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeConnectorTest.java b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeConnectorTest.java index 0f0284813fc2..15307223a00b 100644 --- a/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeConnectorTest.java +++ b/plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeConnectorTest.java @@ -151,9 +151,6 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior) case SUPPORTS_SET_COLUMN_TYPE: return false; - case SUPPORTS_COMMENT_ON_VIEW_COLUMN: - return false; - case SUPPORTS_CREATE_MATERIALIZED_VIEW: return false; diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/TrinoViewHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/TrinoViewHiveMetastore.java index 04cee3ed4d10..f009267fdab4 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/TrinoViewHiveMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/TrinoViewHiveMetastore.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.stream.Stream; @@ -193,6 +194,27 @@ public void updateViewComment(ConnectorSession session, SchemaTableName viewName replaceView(session, viewName, view, newDefinition); } + 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 = TrinoViewUtil.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) 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 14adbf655083..a4baf17843f1 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 @@ -21,7 +21,6 @@ import io.trino.plugin.base.CatalogName; import io.trino.plugin.hive.HiveSchemaProperties; import io.trino.plugin.hive.TrinoViewHiveMetastore; -import io.trino.plugin.hive.TrinoViewUtil; import io.trino.plugin.hive.metastore.Column; import io.trino.plugin.hive.metastore.Database; import io.trino.plugin.hive.metastore.HivePrincipal; @@ -41,7 +40,6 @@ 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,7 +53,6 @@ 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; @@ -385,23 +382,7 @@ public void updateViewComment(ConnectorSession session, SchemaTableName viewName @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 = TrinoViewUtil.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); + trinoViewHiveMetastore.updateViewColumnComment(session, viewName, columnName, comment); } private void replaceView(ConnectorSession session, SchemaTableName viewName, io.trino.plugin.hive.metastore.Table view, ConnectorViewDefinition newDefinition) diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorSmokeTest.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorSmokeTest.java index 3b2d8d558f2d..5e59dfa7a329 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorSmokeTest.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorSmokeTest.java @@ -636,11 +636,6 @@ private String getTableLocation(String tableName) return (String) computeScalar("SELECT DISTINCT regexp_replace(\"$path\", '/[^/]*/[^/]*$', '') FROM " + 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 + "'"); - } - protected abstract void dropTableFromMetastore(String tableName); protected abstract String getMetadataLocation(String tableName); 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 496bda8eeac7..76daa56560ff 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 @@ -41,7 +41,6 @@ import io.trino.plugin.iceberg.IcebergQueryRunner; import io.trino.plugin.iceberg.SchemaInitializer; import io.trino.testing.QueryRunner; -import io.trino.testing.sql.TestView; import org.apache.iceberg.FileFormat; import org.testng.annotations.AfterClass; import org.testng.annotations.Parameters; @@ -144,30 +143,6 @@ public void testRenameSchema() .hasStackTraceContaining("renameNamespace is not supported for Iceberg Glue catalogs"); } - @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); - } - } - @Override protected void dropTableFromMetastore(String tableName) { 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 0a300bac9558..18d576e42178 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 @@ -27,6 +27,7 @@ import static io.trino.spi.connector.ConnectorMetadata.MODIFYING_ROWS_MESSAGE; 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; @@ -626,8 +627,50 @@ public void testCommentView() } } + @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 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)); } + + protected String getColumnComment(String tableName, String columnName) + { + return (String) computeScalar(format( + "SELECT comment FROM information_schema.columns WHERE table_schema = '%s' AND table_name = '%s' AND column_name = '%s'", + getSession().getSchema().orElseThrow(), + tableName, + columnName)); + } }