Skip to content
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.

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

Post merge clean up for materialized view column comment #18628

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions core/trino-main/src/main/java/io/trino/metadata/Metadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,6 @@ Optional<TableExecuteHandle> getTableHandleForExecute(
*/
void setViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional<String> comment);

/**
vlad-lyutenko marked this conversation as resolved.
Show resolved Hide resolved
* Comments to the specified materialized view column.
*/
void setMaterializedViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional<String> comment);

/**
* Comments to the specified column.
*/
Expand Down Expand Up @@ -724,6 +719,11 @@ default boolean isMaterializedView(Session session, QualifiedObjectName viewName
*/
void setMaterializedViewProperties(Session session, QualifiedObjectName viewName, Map<String, Optional<Object>> properties);

/**
* Comments to the specified materialized view column.
*/
void setMaterializedViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional<String> comment);

/**
* Returns the result of redirecting the table scan on a given table to a different table.
* This method is used by the engine during the plan optimization phase to allow a connector to offload table scans to any other connector.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -824,15 +824,6 @@ 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<String> 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<String> comment)
{
Expand Down Expand Up @@ -1609,6 +1600,15 @@ public void setMaterializedViewProperties(Session session, QualifiedObjectName v
metadata.setMaterializedViewProperties(session.toConnectorSession(catalogHandle), viewName.asSchemaTableName(), properties);
}

@Override
public void setMaterializedViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional<String> 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);
}

private static boolean isExternalInformationSchema(CatalogHandle catalogHandle, Optional<String> schemaName)
{
return schemaName.isPresent() && isExternalInformationSchema(catalogHandle, schemaName.get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,15 +434,6 @@ public void setViewColumnComment(Session session, QualifiedObjectName viewName,
}
}

@Override
public void setMaterializedViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional<String> 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<String> comment)
{
Expand Down Expand Up @@ -1313,6 +1304,15 @@ public void setMaterializedViewProperties(Session session, QualifiedObjectName v
}
}

@Override
public void setMaterializedViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional<String> comment)
{
Span span = startSpan("setMaterializedViewColumnComment", viewName);
try (var ignored = scopedSpan(span)) {
delegate.setMaterializedViewColumnComment(session, viewName, columnName, comment);
}
}

@Override
public Optional<TableScanRedirectApplicationResult> applyTableScanRedirect(Session session, TableHandle tableHandle)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,26 @@ public synchronized void setMaterializedViewProperties(
newProperties));
}

@Override
public void setMaterializedViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional<String> 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 dropMaterializedView(Session session, QualifiedObjectName viewName)
{
Expand Down Expand Up @@ -576,26 +596,6 @@ public void setViewColumnComment(Session session, QualifiedObjectName viewName,
view.getRunAsIdentity()));
}

@Override
public void setMaterializedViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional<String> 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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,12 +303,6 @@ public void setViewColumnComment(Session session, QualifiedObjectName viewName,
throw new UnsupportedOperationException();
}

@Override
public void setMaterializedViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional<String> comment)
{
throw new UnsupportedOperationException();
}

@Override
public void setColumnComment(Session session, TableHandle tableHandle, ColumnHandle column, Optional<String> comment)
{
Expand Down Expand Up @@ -905,6 +899,12 @@ public void setMaterializedViewProperties(Session session, QualifiedObjectName v
throw new UnsupportedOperationException();
}

@Override
public void setMaterializedViewColumnComment(Session session, QualifiedObjectName viewName, String columnName, Optional<String> comment)
{
throw new UnsupportedOperationException();
}

@Override
public Optional<TableScanRedirectApplicationResult> applyTableScanRedirect(Session session, TableHandle tableHandle)
{
Expand Down
3 changes: 0 additions & 3 deletions docs/src/main/sphinx/connector/blackhole.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,6 @@ additional features:
- {doc}`/sql/create-view`
- {doc}`/sql/show-create-view`
- {doc}`/sql/drop-view`
- {doc}`/sql/create-materialized-view`
- {doc}`/sql/show-create-materialized-view`
- {doc}`/sql/drop-materialized-view`
vlad-lyutenko marked this conversation as resolved.
Show resolved Hide resolved

:::{note}
The connector discards all written data. While read operations are supported,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import io.trino.spi.connector.ColumnMetadata;
import io.trino.spi.connector.ConnectorAnalyzeMetadata;
import io.trino.spi.connector.ConnectorInsertTableHandle;
import io.trino.spi.connector.ConnectorMaterializedViewDefinition;
vlad-lyutenko marked this conversation as resolved.
Show resolved Hide resolved
import io.trino.spi.connector.ConnectorMergeTableHandle;
import io.trino.spi.connector.ConnectorMetadata;
import io.trino.spi.connector.ConnectorOutputMetadata;
Expand All @@ -34,7 +33,6 @@
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;
Expand Down Expand Up @@ -73,7 +71,6 @@
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;
Expand All @@ -89,7 +86,6 @@ public class BlackHoleMetadata
private final List<String> schemas = new ArrayList<>();
private final Map<SchemaTableName, BlackHoleTableHandle> tables = new ConcurrentHashMap<>();
private final Map<SchemaTableName, ConnectorViewDefinition> views = new ConcurrentHashMap<>();
private final Map<SchemaTableName, ConnectorMaterializedViewDefinition> materializedViews = new ConcurrentHashMap<>();

public BlackHoleMetadata()
{
Expand Down Expand Up @@ -427,66 +423,4 @@ public void setViewColumnComment(ConnectorSession session, SchemaTableName viewN
view.getOwner(),
view.isRunAsInvoker()));
}

@Override
public boolean delegateMaterializedViewRefreshToConnector(ConnectorSession session, SchemaTableName viewName)
{
return false;
}

@Override
public void dropMaterializedView(ConnectorSession session, SchemaTableName viewName)
{
materializedViews.remove(viewName);
}

@Override
public void createMaterializedView(ConnectorSession session, SchemaTableName viewName, ConnectorMaterializedViewDefinition definition, boolean replace, boolean ignoreExisting)
{
materializedViews.put(viewName, definition);
}

@Override
public Optional<ConnectorMaterializedViewDefinition> getMaterializedView(ConnectorSession session, SchemaTableName viewName)
{
return Optional.ofNullable(materializedViews.get(viewName));
}

@Override
public Map<SchemaTableName, ConnectorMaterializedViewDefinition> getMaterializedViews(ConnectorSession session, Optional<String> schemaName)
{
return ImmutableMap.copyOf(materializedViews);
}

@Override
public MaterializedViewFreshness getMaterializedViewFreshness(ConnectorSession session, SchemaTableName name)
{
return new MaterializedViewFreshness(STALE, Optional.empty());
}

@Override
public List<SchemaTableName> listMaterializedViews(ConnectorSession session, Optional<String> schemaName)
{
return ImmutableList.copyOf(materializedViews.keySet());
}

@Override
public void setMaterializedViewColumnComment(ConnectorSession session, SchemaTableName viewName, String columnName, Optional<String> 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()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -220,59 +220,6 @@ private String getTableComment(String tableName)
.getOnlyValue();
}

@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);
assertThat(rows.getRowCount()).isEqualTo(25);

// listing
assertThat(queryRunner.execute("SHOW TABLES").getOnlyColumnAsSet())
.contains(viewName);
}
finally {
assertThatQueryDoesNotReturnValues("DROP MATERIALIZED VIEW " + viewName);
}
}

@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()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,6 @@ Transaction newCreateTableTransaction(

void updateViewColumnComment(ConnectorSession session, SchemaTableName schemaViewName, String columnName, Optional<String> comment);

void updateMaterializedViewColumnComment(ConnectorSession session, SchemaTableName schemaViewName, String columnName, Optional<String> comment);

String defaultTableLocation(ConnectorSession session, SchemaTableName schemaTableName);

void setTablePrincipal(ConnectorSession session, SchemaTableName schemaTableName, TrinoPrincipal principal);
Expand Down Expand Up @@ -155,6 +153,8 @@ void createMaterializedView(
boolean replace,
boolean ignoreExisting);

void updateMaterializedViewColumnComment(ConnectorSession session, SchemaTableName schemaViewName, String columnName, Optional<String> comment);

void dropMaterializedView(ConnectorSession session, SchemaTableName viewName);

Optional<ConnectorMaterializedViewDefinition> getMaterializedView(ConnectorSession session, SchemaTableName viewName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,12 +311,6 @@ 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<String> comment)
{
throw new TrinoException(NOT_SUPPORTED, "updateMaterializedViewColumnComment is not supported for Iceberg JDBC catalogs");
}

@Override
public String defaultTableLocation(ConnectorSession session, SchemaTableName schemaTableName)
{
Expand Down Expand Up @@ -400,6 +394,12 @@ public void createMaterializedView(ConnectorSession session, SchemaTableName sch
throw new TrinoException(NOT_SUPPORTED, "createMaterializedView is not supported for Iceberg JDBC catalogs");
}

@Override
public void updateMaterializedViewColumnComment(ConnectorSession session, SchemaTableName schemaViewName, String columnName, Optional<String> comment)
{
throw new TrinoException(NOT_SUPPORTED, "updateMaterializedViewColumnComment is not supported for Iceberg JDBC catalogs");
}

@Override
public void dropMaterializedView(ConnectorSession session, SchemaTableName schemaViewName)
{
Expand Down
Loading