From fe30d7a7e93e6678413be2ce1eadabf590513195 Mon Sep 17 00:00:00 2001 From: Alex Jo Date: Mon, 18 Sep 2023 14:45:42 -0400 Subject: [PATCH] Support altering column comments in Hive Glue catalogs --- .../metastore/glue/GlueHiveMetastore.java | 52 ++++++++++++++++++- .../metastore/glue/TestHiveGlueMetastore.java | 32 ++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueHiveMetastore.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueHiveMetastore.java index 78c596ceda2c..03fdb6bc1786 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueHiveMetastore.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueHiveMetastore.java @@ -144,6 +144,7 @@ import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.util.concurrent.MoreExecutors.directExecutor; import static io.trino.plugin.hive.HiveErrorCode.HIVE_FILESYSTEM_ERROR; +import static io.trino.plugin.hive.HiveErrorCode.HIVE_INVALID_METADATA; import static io.trino.plugin.hive.HiveErrorCode.HIVE_METASTORE_ERROR; import static io.trino.plugin.hive.TableType.MANAGED_TABLE; import static io.trino.plugin.hive.TableType.VIRTUAL_VIEW; @@ -727,7 +728,56 @@ public void setTableOwner(String databaseName, String tableName, HivePrincipal p @Override public void commentColumn(String databaseName, String tableName, String columnName, Optional comment) { - throw new TrinoException(NOT_SUPPORTED, "Column comment is not yet supported by Glue service"); + Table table = getExistingTable(databaseName, tableName); + List dataColumns = table.getDataColumns(); + List partitionColumns = table.getPartitionColumns(); + + Optional matchingDataColumn = indexOfColumnWithName(dataColumns, columnName); + Optional matchingPartitionColumn = indexOfColumnWithName(partitionColumns, columnName); + + if (matchingDataColumn.isPresent() && matchingPartitionColumn.isPresent()) { + throw new TrinoException(HIVE_INVALID_METADATA, "Found two columns with names matching " + columnName); + } + if (matchingDataColumn.isEmpty() && matchingPartitionColumn.isEmpty()) { + throw new ColumnNotFoundException(table.getSchemaTableName(), columnName); + } + + Table updatedTable = Table.builder(table) + .setDataColumns(matchingDataColumn.map(index -> setColumnCommentForIndex(dataColumns, index, comment)).orElse(dataColumns)) + .setPartitionColumns(matchingPartitionColumn.map(index -> setColumnCommentForIndex(partitionColumns, index, comment)).orElse(partitionColumns)) + .build(); + + replaceTable(databaseName, tableName, updatedTable, null); + } + + private static Optional indexOfColumnWithName(List columns, String columnName) + { + Optional index = Optional.empty(); + for (int i = 0; i < columns.size(); i++) { + // Glue columns are always lowercase + if (columns.get(i).getName().equals(columnName)) { + index.ifPresent(ignored -> { + throw new TrinoException(HIVE_INVALID_METADATA, "Found two columns with names matching " + columnName); + }); + index = Optional.of(i); + } + } + return index; + } + + private static List setColumnCommentForIndex(List columns, int indexToUpdate, Optional comment) + { + ImmutableList.Builder newColumns = ImmutableList.builder(); + for (int i = 0; i < columns.size(); i++) { + Column originalColumn = columns.get(i); + if (i == indexToUpdate) { + newColumns.add(new Column(originalColumn.getName(), originalColumn.getType(), comment, originalColumn.getProperties())); + } + else { + newColumns.add(originalColumn); + } + } + return newColumns.build(); } @Override diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestHiveGlueMetastore.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestHiveGlueMetastore.java index 5f5a9ae0d6c3..28159ebf32f9 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestHiveGlueMetastore.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/glue/TestHiveGlueMetastore.java @@ -1425,6 +1425,38 @@ public void testGlueObjectsWithoutStorageDescriptor() } } + @Test + public void testAlterColumnComment() + throws Exception + { + SchemaTableName tableName = temporaryTable("test_alter_column_comment"); + List columns = ImmutableList.of( + new ColumnMetadata("first_column", BIGINT), + new ColumnMetadata("second_column", VARCHAR), + new ColumnMetadata("partition_column", BIGINT)); + createDummyPartitionedTable(tableName, columns, ImmutableList.of("partition_column"), ImmutableList.of()); + try { + metastore.commentColumn(tableName.getSchemaName(), tableName.getTableName(), "second_column", Optional.of("second column comment")); + metastore.commentColumn(tableName.getSchemaName(), tableName.getTableName(), "partition_column", Optional.of("partition column comment")); + + Table withComment = metastore.getTable(tableName.getSchemaName(), tableName.getTableName()).orElseThrow(); + assertThat(withComment.getColumn("first_column").orElseThrow().getComment()).isEmpty(); + assertThat(withComment.getColumn("second_column").orElseThrow().getComment()).isEqualTo(Optional.of("second column comment")); + assertThat(withComment.getColumn("partition_column").orElseThrow().getComment()).isEqualTo(Optional.of("partition column comment")); + + metastore.commentColumn(tableName.getSchemaName(), tableName.getTableName(), "second_column", Optional.empty()); + withComment = metastore.getTable(tableName.getSchemaName(), tableName.getTableName()).orElseThrow(); + assertThat(withComment.getColumn("first_column").orElseThrow().getComment()).isEmpty(); + assertThat(withComment.getColumn("second_column").orElseThrow().getComment()).isEmpty(); + assertThat(withComment.getColumn("partition_column").orElseThrow().getComment()).isEqualTo(Optional.of("partition column comment")); + } + finally { + glueClient.deleteTable(new DeleteTableRequest() + .withDatabaseName(tableName.getSchemaName()) + .withName(tableName.getTableName())); + } + } + private Block singleValueBlock(long value) { BlockBuilder blockBuilder = BIGINT.createBlockBuilder(null, 1);