Skip to content

[WIP] Add comments at column level for views #10854

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
shandeep00 wants to merge 1 commit into from
Closed

[WIP] Add comments at column level for views #10854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 29 additions & 10 deletions core/trino-main/src/main/java/io/trino/execution/CommentTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import io.trino.metadata.Metadata;
import io.trino.metadata.QualifiedObjectName;
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;
Expand Down Expand Up @@ -86,20 +88,37 @@ else if (statement.getType() == Comment.Type.COLUMN) {
}

QualifiedObjectName tableName = createQualifiedObjectName(session, statement, prefix.get());
Optional<TableHandle> tableHandle = metadata.getTableHandle(session, tableName);
if (tableHandle.isEmpty()) {
throw semanticException(TABLE_NOT_FOUND, statement, "Table does not exist: " + tableName);
}

String columnName = statement.getName().getSuffix();
Map<String, ColumnHandle> columnHandles = metadata.getColumnHandles(session, tableHandle.get());
if (!columnHandles.containsKey(columnName)) {
throw semanticException(COLUMN_NOT_FOUND, statement, "Column does not exist: " + columnName);
Optional<ViewDefinition> optionalView = metadata.getView(session, tableName);
if (optionalView.isPresent()) {
ViewDefinition viewDefinition = optionalView.get();
List<ViewColumn> viewColumns = viewDefinition.getColumns();
Optional<ViewColumn> optionalViewColumn = viewColumns
.stream()
.filter(viewColumn -> viewColumn.getName().equals(columnName))
.findFirst();

if (optionalViewColumn.isEmpty()) {
throw semanticException(COLUMN_NOT_FOUND, statement, "Column does not exist: " + columnName);
}

accessControl.checkCanSetColumnComment(session.toSecurityContext(), tableName);
Copy link
Copy Markdown
Member

@ebyhr ebyhr Jan 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was able to add comment to viewdefiniton

The comment isn't yet stored to HMS as far as I checked. How did you confirm the comment?

}
else {
Optional<TableHandle> tableHandle = metadata.getTableHandle(session, tableName);
if (tableHandle.isEmpty()) {
throw semanticException(TABLE_NOT_FOUND, statement, "Table does not exist: " + tableName);
}

Map<String, ColumnHandle> columnHandles = metadata.getColumnHandles(session, tableHandle.get());
if (!columnHandles.containsKey(columnName)) {
throw semanticException(COLUMN_NOT_FOUND, statement, "Column does not exist: " + columnName);
}

accessControl.checkCanSetColumnComment(session.toSecurityContext(), tableName);
accessControl.checkCanSetColumnComment(session.toSecurityContext(), tableName);

metadata.setColumnComment(session, tableHandle.get(), columnHandles.get(columnName), statement.getComment());
metadata.setColumnComment(session, tableHandle.get(), columnHandles.get(columnName), statement.getComment());
}
}
else {
throw semanticException(NOT_SUPPORTED, statement, "Unsupported comment type: %s", statement.getType());
Expand Down
5 changes: 5 additions & 0 deletions core/trino-main/src/main/java/io/trino/metadata/Metadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,4 +713,9 @@ default boolean isMaterializedView(Session session, QualifiedObjectName viewName
* Returns a table handle for the specified table name with a specified version
*/
Optional<TableHandle> getTableHandle(Session session, QualifiedObjectName tableName, Optional<TableVersion> startVersion, Optional<TableVersion> endVersion);

/**
* Comments to the specified view column
*/
void setViewColumnComment(Session session, QualifiedObjectName viewName, ViewDefinition viewDefinition, ViewColumn viewColumn, Optional<String> comment);
}
10 changes: 10 additions & 0 deletions core/trino-main/src/main/java/io/trino/metadata/MetadataManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2539,6 +2539,16 @@ private Optional<ConnectorTableVersion> toConnectorVersion(Optional<TableVersion
return connectorVersion;
}

@Override
public void setViewColumnComment(Session session, QualifiedObjectName viewName, ViewDefinition viewDefinition, ViewColumn viewColumn, Optional<String> comment)
{
CatalogMetadata catalogMetadata = getCatalogMetadataForWrite(session, viewName.getCatalogName());
CatalogName catalogName = catalogMetadata.getCatalogName();
ConnectorMetadata metadata = catalogMetadata.getMetadata();

metadata.setViewColumnComment(session.toConnectorSession(catalogName), viewName.asSchemaTableName(), viewDefinition.toConnectorViewDefinition(), viewColumn.getName(), comment);
}

private static class OperatorCacheKey
{
private final OperatorType operatorType;
Expand Down
24 changes: 21 additions & 3 deletions core/trino-main/src/main/java/io/trino/metadata/ViewColumn.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,27 @@
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
{
private final String name;
private final TypeId type;
private final Optional<String> comment;

public ViewColumn(String name, TypeId type)
{
this(name, type, Optional.empty());
}

public ViewColumn(String name, TypeId type, Optional<String> comment)
{
this.name = requireNonNull(name, "name is null");
this.type = requireNonNull(type, "type is null");
this.comment = comment;
}

public String getName()
Expand All @@ -40,10 +49,19 @@ public TypeId getType()
return type;
}

public Optional<String> 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
Expand All @@ -56,12 +74,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);
}
}
2 changes: 1 addition & 1 deletion core/trino-main/src/main/java/io/trino/metadata/ViewDefinition.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
6 changes: 6 additions & 0 deletions core/trino-main/src/test/java/io/trino/metadata/AbstractMockMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -886,4 +886,10 @@ public Optional<TableHandle> getTableHandle(Session session, QualifiedObjectName
{
throw new UnsupportedOperationException();
}

@Override
public void setViewColumnComment(Session session, QualifiedObjectName viewName, ViewDefinition viewDefinition, ViewColumn viewColumn, Optional<String> comment)
{
throw new UnsupportedOperationException();
}
}
8 changes: 8 additions & 0 deletions core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1340,4 +1340,12 @@ default boolean isSupportedVersionType(ConnectorSession session, SchemaTableName
{
throw new TrinoException(NOT_SUPPORTED, "This connector does not support versioned tables");
}

/**
* Comments to the specified view column
*/
default void setViewColumnComment(ConnectorSession session, SchemaTableName viewName, ConnectorViewDefinition viewDefinition, String columnName, Optional<String> comment)
{
throw new TrinoException(NOT_SUPPORTED, "This connector does not support setting view column comments");
}
}
18 changes: 16 additions & 2 deletions core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorViewDefinition.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,22 @@ public static final class ViewColumn
{
private final String name;
private final TypeId type;
private final Optional<String> comment;

public ViewColumn(String name, TypeId type)
{
this(name, type, Optional.empty());
}

@JsonCreator
public ViewColumn(
@JsonProperty("name") String name,
@JsonProperty("type") TypeId type)
@JsonProperty("type") TypeId type,
@JsonProperty("comment") Optional<String> comment)
{
this.name = requireNonNull(name, "name is null");
this.type = requireNonNull(type, "type is null");
this.comment = comment;
}

@JsonProperty
Expand All @@ -155,10 +163,16 @@ public TypeId getType()
return type;
}

@JsonProperty
public Optional<String> getComment()
{
return comment;
}

@Override
public String toString()
{
return name + " " + type;
return name + " " + type + " " + comment.orElse(null);
}
}
}
8 changes: 8 additions & 0 deletions ...lkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1038,4 +1038,12 @@ public boolean isSupportedVersionType(ConnectorSession session, SchemaTableName
return delegate.isSupportedVersionType(session, tableName, pointerType, versioning);
}
}

@Override
public void setViewColumnComment(ConnectorSession session, SchemaTableName viewName, ConnectorViewDefinition viewDefinition, String columnName, Optional<String> comment)
{
try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) {
delegate.setViewColumnComment(session, viewName, viewDefinition, columnName, comment);
}
}
}
44 changes: 44 additions & 0 deletions plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3482,6 +3482,50 @@ private static TableNameSplitResult splitTableName(String tableName)
new TableNameSplitResult(tableName.substring(0, metadataMarkerIndex), Optional.of(tableName.substring(metadataMarkerIndex)));
}

@Override
public void setViewColumnComment(ConnectorSession session, SchemaTableName viewName, ConnectorViewDefinition viewDefinition, String columnName, Optional<String> comment)
{
Table table = metastore.getTable(new HiveIdentity(session), viewName.getSchemaName(), viewName.getTableName()).get();

List<ConnectorViewDefinition.ViewColumn> viewColumns = new ArrayList<>();
for (ConnectorViewDefinition.ViewColumn viewColumn : viewDefinition.getColumns()) {
if (columnName.equals(viewColumn.getName())) {
ConnectorViewDefinition.ViewColumn updatedColumn = new ConnectorViewDefinition.ViewColumn(viewColumn.getName(), viewColumn.getType(), comment);
viewColumns.add(updatedColumn);
}
else {
viewColumns.add(viewColumn);
}
}

ConnectorViewDefinition definition = new ConnectorViewDefinition(viewDefinition.getOriginalSql(),
viewDefinition.getCatalog(),
viewDefinition.getSchema(),
viewColumns,
viewDefinition.getComment(),
viewDefinition.getOwner(),
viewDefinition.isRunAsInvoker());

Table.Builder tableBuilder = Table.builder()
.setDatabaseName(table.getDatabaseName())
.setTableName(table.getTableName())
.setOwner(table.getOwner())
.setTableType(table.getTableType())
.setDataColumns(table.getDataColumns())
.setPartitionColumns(table.getPartitionColumns())
.setParameters(table.getParameters())
.setViewOriginalText(Optional.of(encodeViewData(definition)))
.setViewExpandedText(table.getViewExpandedText());

tableBuilder.getStorageBuilder()
.setStorageFormat(table.getStorage().getStorageFormat())
.setLocation(table.getStorage().getLocation());
Table newTable = tableBuilder.build();
PrincipalPrivileges principalPrivileges = accessControlMetadata.isUsingSystemSecurity() ? NO_PRIVILEGES : buildInitialPrivilegeSet(session.getUser());

metastore.replaceTable(new HiveIdentity(session), viewName.getSchemaName(), viewName.getTableName(), newTable, principalPrivileges);
}

private static class TableNameSplitResult
{
private final String baseTableName;
Expand Down