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 8708411eb790..8146c5430499 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 @@ -1658,7 +1658,7 @@ public MaterializedViewFreshness getMaterializedViewFreshness(Session session, Q ConnectorSession connectorSession = session.toConnectorSession(catalogHandle); return metadata.getMaterializedViewFreshness(connectorSession, viewName.asSchemaTableName()); } - return new MaterializedViewFreshness(STALE); + return new MaterializedViewFreshness(STALE, Optional.empty()); } @Override diff --git a/core/trino-main/src/main/java/io/trino/security/AccessControl.java b/core/trino-main/src/main/java/io/trino/security/AccessControl.java index 2602e4dddee1..b3e6c24f394e 100644 --- a/core/trino-main/src/main/java/io/trino/security/AccessControl.java +++ b/core/trino-main/src/main/java/io/trino/security/AccessControl.java @@ -252,14 +252,6 @@ public interface AccessControl */ void checkCanShowColumns(SecurityContext context, CatalogSchemaTableName table); - /** - * Filter the list of columns to those visible to the identity. - * - * @deprecated Use {@link #filterColumns(SecurityContext, String, Map)} - */ - @Deprecated - Set filterColumns(SecurityContext context, CatalogSchemaTableName tableName, Set columns); - /** * Filter lists of columns of multiple tables to those visible to the identity. */ diff --git a/core/trino-main/src/main/java/io/trino/security/AccessControlManager.java b/core/trino-main/src/main/java/io/trino/security/AccessControlManager.java index 05c67dde8b6f..90ff4afab672 100644 --- a/core/trino-main/src/main/java/io/trino/security/AccessControlManager.java +++ b/core/trino-main/src/main/java/io/trino/security/AccessControlManager.java @@ -627,32 +627,6 @@ public void checkCanShowColumns(SecurityContext securityContext, CatalogSchemaTa catalogAuthorizationCheck(table.getCatalogName(), securityContext, (control, context) -> control.checkCanShowColumns(context, table.getSchemaTableName())); } - @Override - public Set filterColumns(SecurityContext securityContext, CatalogSchemaTableName table, Set columns) - { - requireNonNull(securityContext, "securityContext is null"); - requireNonNull(table, "tableName is null"); - - if (columns.isEmpty()) { - // Do not call plugin-provided implementation unnecessarily. - return ImmutableSet.of(); - } - - if (filterTables(securityContext, table.getCatalogName(), ImmutableSet.of(table.getSchemaTableName())).isEmpty()) { - return ImmutableSet.of(); - } - - for (SystemAccessControl systemAccessControl : getSystemAccessControls()) { - columns = systemAccessControl.filterColumns(securityContext.toSystemSecurityContext(), table, columns); - } - - ConnectorAccessControl connectorAccessControl = getConnectorAccessControl(securityContext.getTransactionId(), table.getCatalogName()); - if (connectorAccessControl != null) { - columns = connectorAccessControl.filterColumns(toConnectorSecurityContext(table.getCatalogName(), securityContext), table.getSchemaTableName(), columns); - } - return columns; - } - @Override public Map> filterColumns(SecurityContext securityContext, String catalogName, Map> tableColumns) { diff --git a/core/trino-main/src/main/java/io/trino/security/AllowAllAccessControl.java b/core/trino-main/src/main/java/io/trino/security/AllowAllAccessControl.java index 15d393af9232..64f23efeefbf 100644 --- a/core/trino-main/src/main/java/io/trino/security/AllowAllAccessControl.java +++ b/core/trino-main/src/main/java/io/trino/security/AllowAllAccessControl.java @@ -180,12 +180,6 @@ public void checkCanShowColumns(SecurityContext context, CatalogSchemaTableName { } - @Override - public Set filterColumns(SecurityContext context, CatalogSchemaTableName tableName, Set columns) - { - return columns; - } - @Override public Map> filterColumns(SecurityContext context, String catalogName, Map> tableColumns) { diff --git a/core/trino-main/src/main/java/io/trino/security/DenyAllAccessControl.java b/core/trino-main/src/main/java/io/trino/security/DenyAllAccessControl.java index 7d9fa4c2e78a..7d3013b1c535 100644 --- a/core/trino-main/src/main/java/io/trino/security/DenyAllAccessControl.java +++ b/core/trino-main/src/main/java/io/trino/security/DenyAllAccessControl.java @@ -263,12 +263,6 @@ public void checkCanShowColumns(SecurityContext context, CatalogSchemaTableName denyShowColumns(table.toString()); } - @Override - public Set filterColumns(SecurityContext context, CatalogSchemaTableName tableName, Set columns) - { - return ImmutableSet.of(); - } - @Override public Map> filterColumns(SecurityContext context, String catalogName, Map> tableColumns) { diff --git a/core/trino-main/src/main/java/io/trino/security/ForwardingAccessControl.java b/core/trino-main/src/main/java/io/trino/security/ForwardingAccessControl.java index e6ae8161cdb2..a9522729a2ec 100644 --- a/core/trino-main/src/main/java/io/trino/security/ForwardingAccessControl.java +++ b/core/trino-main/src/main/java/io/trino/security/ForwardingAccessControl.java @@ -233,12 +233,6 @@ public void checkCanShowColumns(SecurityContext context, CatalogSchemaTableName delegate().checkCanShowColumns(context, table); } - @Override - public Set filterColumns(SecurityContext context, CatalogSchemaTableName tableName, Set columns) - { - return delegate().filterColumns(context, tableName, columns); - } - @Override public Map> filterColumns(SecurityContext context, String catalogName, Map> tableColumns) { diff --git a/core/trino-main/src/main/java/io/trino/security/InjectedConnectorAccessControl.java b/core/trino-main/src/main/java/io/trino/security/InjectedConnectorAccessControl.java index 7da3441b45ed..33b71e97f033 100644 --- a/core/trino-main/src/main/java/io/trino/security/InjectedConnectorAccessControl.java +++ b/core/trino-main/src/main/java/io/trino/security/InjectedConnectorAccessControl.java @@ -184,13 +184,6 @@ public void checkCanShowColumns(ConnectorSecurityContext context, SchemaTableNam accessControl.checkCanShowColumns(securityContext, new CatalogSchemaTableName(catalogName, tableName)); } - @Override - public Set filterColumns(ConnectorSecurityContext context, SchemaTableName tableName, Set columns) - { - checkArgument(context == null, "context must be null"); - return accessControl.filterColumns(securityContext, new CatalogSchemaTableName(catalogName, tableName), columns); - } - @Override public Map> filterColumns(ConnectorSecurityContext context, Map> tableColumns) { diff --git a/core/trino-main/src/main/java/io/trino/security/ViewAccessControl.java b/core/trino-main/src/main/java/io/trino/security/ViewAccessControl.java index 195298d6024c..59ac8f210630 100644 --- a/core/trino-main/src/main/java/io/trino/security/ViewAccessControl.java +++ b/core/trino-main/src/main/java/io/trino/security/ViewAccessControl.java @@ -14,7 +14,6 @@ package io.trino.security; import io.trino.metadata.QualifiedObjectName; -import io.trino.spi.connector.CatalogSchemaTableName; import io.trino.spi.connector.SchemaTableName; import io.trino.spi.security.AccessDeniedException; import io.trino.spi.security.ViewExpression; @@ -54,12 +53,6 @@ public void checkCanSelectFromColumns(SecurityContext context, QualifiedObjectNa wrapAccessDeniedException(() -> delegate.checkCanCreateViewWithSelectFromColumns(context, tableName, columnNames)); } - @Override - public Set filterColumns(SecurityContext context, CatalogSchemaTableName tableName, Set columns) - { - return delegate.filterColumns(context, tableName, columns); - } - @Override public Map> filterColumns(SecurityContext context, String catalogName, Map> tableColumns) { diff --git a/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java b/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java index 2d74e0a8fbc1..484d28d0e851 100644 --- a/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java +++ b/core/trino-main/src/main/java/io/trino/sql/analyzer/StatementAnalyzer.java @@ -4415,11 +4415,14 @@ private List filterInaccessibleFields(List fields) tableFieldsMap.asMap().forEach((table, tableFields) -> { Set accessibleColumns = accessControl.filterColumns( - session.toSecurityContext(), - table.asCatalogSchemaTableName(), - tableFields.stream() - .map(field -> field.getOriginColumnName().get()) - .collect(toImmutableSet())); + session.toSecurityContext(), + table.getCatalogName(), + ImmutableMap.of( + table.asSchemaTableName(), + tableFields.stream() + .map(field -> field.getOriginColumnName().get()) + .collect(toImmutableSet()))) + .getOrDefault(table.asSchemaTableName(), ImmutableSet.of()); accessibleFields.addAll(tableFields.stream() .filter(field -> accessibleColumns.contains(field.getOriginColumnName().get())) .collect(toImmutableList())); diff --git a/core/trino-main/src/main/java/io/trino/testing/TestingAccessControlManager.java b/core/trino-main/src/main/java/io/trino/testing/TestingAccessControlManager.java index 8ed1e6459d1c..9a7d2edb6f8d 100644 --- a/core/trino-main/src/main/java/io/trino/testing/TestingAccessControlManager.java +++ b/core/trino-main/src/main/java/io/trino/testing/TestingAccessControlManager.java @@ -39,6 +39,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -640,19 +641,12 @@ public void checkCanShowColumns(SecurityContext context, CatalogSchemaTableName } } - @Override - public Set filterColumns(SecurityContext context, CatalogSchemaTableName table, Set columns) - { - Set visibleColumns = localFilterColumns(context, table.getSchemaTableName(), columns); - return super.filterColumns(context, table, visibleColumns); - } - @Override public Map> filterColumns(SecurityContext context, String catalogName, Map> tableColumns) { tableColumns = tableColumns.entrySet().stream() .collect(toImmutableMap( - Map.Entry::getKey, + Entry::getKey, e -> localFilterColumns(context, e.getKey(), e.getValue()))); return super.filterColumns(context, catalogName, tableColumns); } diff --git a/core/trino-main/src/main/java/io/trino/tracing/TracingAccessControl.java b/core/trino-main/src/main/java/io/trino/tracing/TracingAccessControl.java index 4c9837fe1e81..672a9db11824 100644 --- a/core/trino-main/src/main/java/io/trino/tracing/TracingAccessControl.java +++ b/core/trino-main/src/main/java/io/trino/tracing/TracingAccessControl.java @@ -320,19 +320,10 @@ public void checkCanShowColumns(SecurityContext context, CatalogSchemaTableName } } - @Override - public Set filterColumns(SecurityContext context, CatalogSchemaTableName tableName, Set columns) - { - Span span = startSpan("filterColumns"); - try (var ignored = scopedSpan(span)) { - return delegate.filterColumns(context, tableName, columns); - } - } - @Override public Map> filterColumns(SecurityContext context, String catalogName, Map> tableColumns) { - Span span = startSpan("filterColumns bulk"); + Span span = startSpan("filterColumns"); try (var ignored = scopedSpan(span)) { return delegate.filterColumns(context, catalogName, tableColumns); } diff --git a/core/trino-main/src/test/java/io/trino/connector/MockConnector.java b/core/trino-main/src/test/java/io/trino/connector/MockConnector.java index a4b9362cdb60..82bf03c0a12a 100644 --- a/core/trino-main/src/test/java/io/trino/connector/MockConnector.java +++ b/core/trino-main/src/test/java/io/trino/connector/MockConnector.java @@ -685,7 +685,7 @@ public MaterializedViewFreshness getMaterializedViewFreshness(ConnectorSession s { ConnectorMaterializedViewDefinition view = getMaterializedViews.apply(session, viewName.toSchemaTablePrefix()).get(viewName); checkArgument(view != null, "Materialized view %s does not exist", viewName); - return new MaterializedViewFreshness(view.getStorageTable().isPresent() ? FRESH : STALE); + return new MaterializedViewFreshness(view.getStorageTable().isPresent() ? FRESH : STALE, Optional.empty()); } @Override diff --git a/core/trino-main/src/test/java/io/trino/sql/query/TestColumnMask.java b/core/trino-main/src/test/java/io/trino/sql/query/TestColumnMask.java index 9a1c569409c0..e1d6cc342035 100644 --- a/core/trino-main/src/test/java/io/trino/sql/query/TestColumnMask.java +++ b/core/trino-main/src/test/java/io/trino/sql/query/TestColumnMask.java @@ -115,10 +115,10 @@ public TestColumnMask() Optional.empty(), Optional.empty(), ImmutableList.of( - new ConnectorMaterializedViewDefinition.Column("nationkey", BigintType.BIGINT.getTypeId()), - new ConnectorMaterializedViewDefinition.Column("name", VarcharType.createVarcharType(25).getTypeId()), - new ConnectorMaterializedViewDefinition.Column("regionkey", BigintType.BIGINT.getTypeId()), - new ConnectorMaterializedViewDefinition.Column("comment", VarcharType.createVarcharType(152).getTypeId())), + new ConnectorMaterializedViewDefinition.Column("nationkey", BigintType.BIGINT.getTypeId(), Optional.empty()), + new ConnectorMaterializedViewDefinition.Column("name", VarcharType.createVarcharType(25).getTypeId(), Optional.empty()), + new ConnectorMaterializedViewDefinition.Column("regionkey", BigintType.BIGINT.getTypeId(), Optional.empty()), + new ConnectorMaterializedViewDefinition.Column("comment", VarcharType.createVarcharType(152).getTypeId(), Optional.empty())), Optional.of(Duration.ZERO), Optional.empty(), Optional.of(VIEW_OWNER), @@ -131,10 +131,10 @@ public TestColumnMask() Optional.empty(), Optional.empty(), ImmutableList.of( - new ConnectorMaterializedViewDefinition.Column("nationkey", BigintType.BIGINT.getTypeId()), - new ConnectorMaterializedViewDefinition.Column("name", VarcharType.createVarcharType(25).getTypeId()), - new ConnectorMaterializedViewDefinition.Column("regionkey", BigintType.BIGINT.getTypeId()), - new ConnectorMaterializedViewDefinition.Column("comment", VarcharType.createVarcharType(152).getTypeId())), + new ConnectorMaterializedViewDefinition.Column("nationkey", BigintType.BIGINT.getTypeId(), Optional.empty()), + new ConnectorMaterializedViewDefinition.Column("name", VarcharType.createVarcharType(25).getTypeId(), Optional.empty()), + new ConnectorMaterializedViewDefinition.Column("regionkey", BigintType.BIGINT.getTypeId(), Optional.empty()), + new ConnectorMaterializedViewDefinition.Column("comment", VarcharType.createVarcharType(152).getTypeId(), Optional.empty())), Optional.of(Duration.ZERO), Optional.empty(), Optional.of(VIEW_OWNER), @@ -147,10 +147,10 @@ public TestColumnMask() Optional.empty(), Optional.empty(), ImmutableList.of( - new ConnectorMaterializedViewDefinition.Column("nationkey", BigintType.BIGINT.getTypeId()), - new ConnectorMaterializedViewDefinition.Column("name", VarcharType.createVarcharType(2).getTypeId()), - new ConnectorMaterializedViewDefinition.Column("regionkey", BigintType.BIGINT.getTypeId()), - new ConnectorMaterializedViewDefinition.Column("comment", VarcharType.createVarcharType(152).getTypeId())), + new ConnectorMaterializedViewDefinition.Column("nationkey", BigintType.BIGINT.getTypeId(), Optional.empty()), + new ConnectorMaterializedViewDefinition.Column("name", VarcharType.createVarcharType(2).getTypeId(), Optional.empty()), + new ConnectorMaterializedViewDefinition.Column("regionkey", BigintType.BIGINT.getTypeId(), Optional.empty()), + new ConnectorMaterializedViewDefinition.Column("comment", VarcharType.createVarcharType(152).getTypeId(), Optional.empty())), Optional.of(Duration.ZERO), Optional.empty(), Optional.of(VIEW_OWNER), diff --git a/core/trino-main/src/test/java/io/trino/testing/TestTestingMetadata.java b/core/trino-main/src/test/java/io/trino/testing/TestTestingMetadata.java index 8abce5709f1b..8b5bad533ce5 100644 --- a/core/trino-main/src/test/java/io/trino/testing/TestTestingMetadata.java +++ b/core/trino-main/src/test/java/io/trino/testing/TestTestingMetadata.java @@ -58,7 +58,7 @@ private static ConnectorMaterializedViewDefinition someMaterializedView() Optional.empty(), Optional.empty(), Optional.empty(), - ImmutableList.of(new Column("test", BIGINT.getTypeId())), + ImmutableList.of(new Column("test", BIGINT.getTypeId(), Optional.empty())), Optional.of(Duration.ZERO), Optional.empty(), Optional.of("owner"), diff --git a/core/trino-spi/src/main/java/io/trino/spi/connector/Connector.java b/core/trino-spi/src/main/java/io/trino/spi/connector/Connector.java index ae511a01d18b..30efe527db58 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/connector/Connector.java +++ b/core/trino-spi/src/main/java/io/trino/spi/connector/Connector.java @@ -30,15 +30,6 @@ public interface Connector { - /** - * @deprecated use {@link #beginTransaction(IsolationLevel, boolean, boolean)} - */ - @Deprecated - default ConnectorTransactionHandle beginTransaction(IsolationLevel isolationLevel, boolean readOnly) - { - throw new UnsupportedOperationException(); - } - /** * Start a new transaction and return a handle for it. The engine will call * {@link #getMetadata} to fetch the metadata instance for the transaction. @@ -56,7 +47,7 @@ default ConnectorTransactionHandle beginTransaction(IsolationLevel isolationLeve */ default ConnectorTransactionHandle beginTransaction(IsolationLevel isolationLevel, boolean readOnly, boolean autoCommit) { - return beginTransaction(isolationLevel, readOnly); + throw new UnsupportedOperationException(); } /** @@ -64,18 +55,6 @@ default ConnectorTransactionHandle beginTransaction(IsolationLevel isolationLeve * in a single threaded context. */ default ConnectorMetadata getMetadata(ConnectorSession session, ConnectorTransactionHandle transactionHandle) - { - return getMetadata(transactionHandle); - } - - /** - * Guaranteed to be called at most once per transaction. The returned metadata will only be accessed - * in a single threaded context. - * - * @deprecated use {@link #getMetadata(ConnectorSession, ConnectorTransactionHandle)} - */ - @Deprecated - default ConnectorMetadata getMetadata(ConnectorTransactionHandle transactionHandle) { throw new UnsupportedOperationException(); } diff --git a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorAccessControl.java b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorAccessControl.java index 59b49306592b..df2a1d82c3d2 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorAccessControl.java +++ b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorAccessControl.java @@ -24,7 +24,6 @@ import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.stream.Collectors; import static io.trino.spi.security.AccessDeniedException.denyAddColumn; import static io.trino.spi.security.AccessDeniedException.denyAlterColumn; @@ -83,6 +82,7 @@ import static io.trino.spi.security.AccessDeniedException.denyTruncateTable; import static io.trino.spi.security.AccessDeniedException.denyUpdateTableColumns; import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; public interface ConnectorAccessControl @@ -275,26 +275,12 @@ default void checkCanShowColumns(ConnectorSecurityContext context, SchemaTableNa denyShowColumns(tableName.getTableName()); } - /** - * Filter the list of columns to those visible to the identity. - * - * @deprecated Use {@link #filterColumns(ConnectorSecurityContext, Map)} - */ - @Deprecated - default Set filterColumns(ConnectorSecurityContext context, SchemaTableName tableName, Set columns) - { - return emptySet(); - } - /** * Filter lists of columns of multiple tables to those visible to the identity. */ default Map> filterColumns(ConnectorSecurityContext context, Map> tableColumns) { - return tableColumns.entrySet().stream() - .collect(Collectors.toMap( - Map.Entry::getKey, - entry -> filterColumns(context, entry.getKey(), entry.getValue()))); + return emptyMap(); } /** 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 70523c97f018..98a429d3dcee 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 @@ -172,12 +172,6 @@ public static final class Column 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"); diff --git a/core/trino-spi/src/main/java/io/trino/spi/connector/MaterializedViewFreshness.java b/core/trino-spi/src/main/java/io/trino/spi/connector/MaterializedViewFreshness.java index 9d7ee5df9449..0cd3c986352e 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/connector/MaterializedViewFreshness.java +++ b/core/trino-spi/src/main/java/io/trino/spi/connector/MaterializedViewFreshness.java @@ -25,12 +25,6 @@ public final class MaterializedViewFreshness private final Freshness freshness; private final Optional lastFreshTime; - @Deprecated - public MaterializedViewFreshness(Freshness freshness) - { - this(freshness, Optional.empty()); - } - public MaterializedViewFreshness(Freshness freshness, Optional lastFreshTime) { this.freshness = requireNonNull(freshness, "freshness is null"); diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorAccessControl.java index 159f9bd5105e..635ec60d85cb 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/classloader/ClassLoaderSafeConnectorAccessControl.java @@ -189,14 +189,6 @@ public void checkCanShowColumns(ConnectorSecurityContext context, SchemaTableNam } } - @Override - public Set filterColumns(ConnectorSecurityContext context, SchemaTableName tableName, Set columns) - { - try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) { - return delegate.filterColumns(context, tableName, columns); - } - } - @Override public Map> filterColumns(ConnectorSecurityContext context, Map> tableColumns) { diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllAccessControl.java index 2da20cc186e1..6dfd62124b9e 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/AllowAllAccessControl.java @@ -124,12 +124,6 @@ public void checkCanShowColumns(ConnectorSecurityContext context, SchemaTableNam { } - @Override - public Set filterColumns(ConnectorSecurityContext context, SchemaTableName tableName, Set columns) - { - return columns; - } - @Override public Map> filterColumns(ConnectorSecurityContext context, Map> tableColumns) { diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedAccessControl.java index 15fb53c8ee4c..6d6023d85c08 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/FileBasedAccessControl.java @@ -31,12 +31,14 @@ import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Optional; import java.util.Set; import java.util.function.Predicate; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static io.trino.plugin.base.security.TableAccessControlRule.TablePrivilege.DELETE; import static io.trino.plugin.base.security.TableAccessControlRule.TablePrivilege.GRANT_SELECT; @@ -254,7 +256,15 @@ public void checkCanShowColumns(ConnectorSecurityContext context, SchemaTableNam } @Override - public Set filterColumns(ConnectorSecurityContext context, SchemaTableName tableName, Set columns) + public Map> filterColumns(ConnectorSecurityContext context, Map> tableColumns) + { + return tableColumns.entrySet().stream() + .collect(toImmutableMap( + Entry::getKey, + entry -> filterColumns(context, entry.getKey(), entry.getValue()))); + } + + private Set filterColumns(ConnectorSecurityContext context, SchemaTableName tableName, Set columns) { if (INFORMATION_SCHEMA_NAME.equals(tableName.getSchemaName())) { return columns; @@ -280,13 +290,6 @@ public Set filterColumns(ConnectorSecurityContext context, SchemaTableNa .collect(toImmutableSet()); } - @Override - public Map> filterColumns(ConnectorSecurityContext context, Map> tableColumns) - { - // Default implementation is good enough. Explicit implementation is expected by the test though. - return ConnectorAccessControl.super.filterColumns(context, tableColumns); - } - @Override public void checkCanRenameTable(ConnectorSecurityContext context, SchemaTableName tableName, SchemaTableName newTableName) { diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingConnectorAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingConnectorAccessControl.java index bb4643aedd9d..f17efc069ead 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingConnectorAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ForwardingConnectorAccessControl.java @@ -157,12 +157,6 @@ public void checkCanShowColumns(ConnectorSecurityContext context, SchemaTableNam delegate().checkCanShowColumns(context, tableName); } - @Override - public Set filterColumns(ConnectorSecurityContext context, SchemaTableName tableName, Set columns) - { - return delegate().filterColumns(context, tableName, columns); - } - @Override public Map> filterColumns(ConnectorSecurityContext context, Map> tableColumns) { diff --git a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ReadOnlyAccessControl.java b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ReadOnlyAccessControl.java index 0d506c962678..0803a719ed50 100644 --- a/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ReadOnlyAccessControl.java +++ b/lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/security/ReadOnlyAccessControl.java @@ -143,12 +143,6 @@ public void checkCanShowColumns(ConnectorSecurityContext context, SchemaTableNam // allow } - @Override - public Set filterColumns(ConnectorSecurityContext context, SchemaTableName tableName, Set columns) - { - return columns; - } - @Override public Map> filterColumns(ConnectorSecurityContext context, Map> tableColumns) { diff --git a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/BaseFileBasedConnectorAccessControlTest.java b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/BaseFileBasedConnectorAccessControlTest.java index 1e2c0cabd4a9..dd5cb9fb8e16 100644 --- a/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/BaseFileBasedConnectorAccessControlTest.java +++ b/lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/security/BaseFileBasedConnectorAccessControlTest.java @@ -294,13 +294,13 @@ public void testTableRules() accessControl.checkCanShowColumns(ALICE, bobTable); assertEquals( - accessControl.filterColumns(ALICE, bobTable, ImmutableSet.of("a")), - ImmutableSet.of("a")); + accessControl.filterColumns(ALICE, Map.of(bobTable, ImmutableSet.of("a"))), + Map.of(bobTable, ImmutableSet.of("a"))); accessControl.checkCanSelectFromColumns(BOB, bobTable, ImmutableSet.of()); accessControl.checkCanShowColumns(BOB, bobTable); assertEquals( - accessControl.filterColumns(BOB, bobTable, ImmutableSet.of("a")), - ImmutableSet.of("a")); + accessControl.filterColumns(BOB, Map.of(bobTable, ImmutableSet.of("a"))), + Map.of(bobTable, ImmutableSet.of("a"))); accessControl.checkCanInsertIntoTable(BOB, bobTable); accessControl.checkCanDeleteFromTable(BOB, bobTable); @@ -491,11 +491,12 @@ public void testTableFilter() public void testNoTableRules() { ConnectorAccessControl accessControl = createAccessControl("no-access.json"); - assertDenied(() -> accessControl.checkCanShowColumns(BOB, new SchemaTableName("bobschema", "bobtable"))); + SchemaTableName bobTable = new SchemaTableName("bobschema", "bobtable"); + assertDenied(() -> accessControl.checkCanShowColumns(BOB, bobTable)); assertDenied(() -> accessControl.checkCanShowTables(BOB, "bobschema")); assertEquals( - accessControl.filterColumns(BOB, new SchemaTableName("bobschema", "bobtable"), ImmutableSet.of("a")), - ImmutableSet.of()); + accessControl.filterColumns(BOB, Map.of(bobTable, ImmutableSet.of("a"))), + Map.of(bobTable, ImmutableSet.of())); Set tables = ImmutableSet.builder() .add(new SchemaTableName("restricted", "any")) diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/LegacyAccessControl.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/LegacyAccessControl.java index 8f28b3e075e8..877f33d1e01d 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/LegacyAccessControl.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/LegacyAccessControl.java @@ -188,12 +188,6 @@ public void checkCanShowColumns(ConnectorSecurityContext context, SchemaTableNam { } - @Override - public Set filterColumns(ConnectorSecurityContext context, SchemaTableName tableName, Set columns) - { - return columns; - } - @Override public Map> filterColumns(ConnectorSecurityContext context, Map> tableColumns) { diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/SqlStandardAccessControl.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/SqlStandardAccessControl.java index 47416b11133b..932c247e787e 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/SqlStandardAccessControl.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/SqlStandardAccessControl.java @@ -36,10 +36,12 @@ import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Optional; import java.util.Set; import java.util.stream.Stream; +import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static io.trino.plugin.hive.metastore.Database.DEFAULT_DATABASE_NAME; import static io.trino.plugin.hive.metastore.HivePrivilegeInfo.HivePrivilege; @@ -260,7 +262,15 @@ public void checkCanShowColumns(ConnectorSecurityContext context, SchemaTableNam } @Override - public Set filterColumns(ConnectorSecurityContext context, SchemaTableName tableName, Set columns) + public Map> filterColumns(ConnectorSecurityContext context, Map> tableColumns) + { + return tableColumns.entrySet().stream() + .collect(toImmutableMap( + Entry::getKey, + entry -> filterColumns(context, entry.getKey(), entry.getValue()))); + } + + private Set filterColumns(ConnectorSecurityContext context, SchemaTableName tableName, Set columns) { if (!hasAnyTablePermission(context, tableName)) { return ImmutableSet.of(); @@ -268,13 +278,6 @@ public Set filterColumns(ConnectorSecurityContext context, SchemaTableNa return columns; } - @Override - public Map> filterColumns(ConnectorSecurityContext context, Map> tableColumns) - { - // Default implementation is good enough. Explicit implementation is expected by the test though. - return ConnectorAccessControl.super.filterColumns(context, tableColumns); - } - @Override public void checkCanAddColumn(ConnectorSecurityContext context, SchemaTableName tableName) { diff --git a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/SystemTableAwareAccessControl.java b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/SystemTableAwareAccessControl.java index 08329e51f19f..c9a1a32c2806 100644 --- a/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/SystemTableAwareAccessControl.java +++ b/plugin/trino-hive/src/main/java/io/trino/plugin/hive/security/SystemTableAwareAccessControl.java @@ -14,6 +14,8 @@ package io.trino.plugin.hive.security; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.inject.Inject; import io.trino.plugin.base.security.ForwardingConnectorAccessControl; import io.trino.plugin.hive.SystemTableProvider; @@ -22,9 +24,12 @@ import io.trino.spi.connector.SchemaTableName; import io.trino.spi.security.AccessDeniedException; +import java.util.Map; +import java.util.Map.Entry; import java.util.Optional; import java.util.Set; +import static com.google.common.collect.ImmutableMap.toImmutableMap; import static io.trino.plugin.hive.util.SystemTables.getSourceTableNameFromSystemTable; import static io.trino.spi.security.AccessDeniedException.denySelectTable; import static io.trino.spi.security.AccessDeniedException.denyShowColumns; @@ -67,13 +72,23 @@ public void checkCanShowColumns(ConnectorSecurityContext context, SchemaTableNam } @Override - public Set filterColumns(ConnectorSecurityContext context, SchemaTableName tableName, Set columns) + public Map> filterColumns(ConnectorSecurityContext context, Map> tableColumns) + { + return tableColumns.entrySet().stream() + .collect(toImmutableMap( + Entry::getKey, + // TODO call delegate.filterColumns in bulk + entry -> filterColumns(context, entry.getKey(), entry.getValue()))); + } + + private Set filterColumns(ConnectorSecurityContext context, SchemaTableName tableName, Set columns) { Optional sourceTableName = getSourceTableNameFromSystemTable(systemTableProviders, tableName); if (sourceTableName.isPresent()) { + // TODO system table may have quite different columns that its source table. It's unclear why it's a good idea to conflate the two. return filterColumns(context, sourceTableName.get(), columns); } - return delegate.filterColumns(context, tableName, columns); + return delegate.filterColumns(context, ImmutableMap.of(tableName, columns)).getOrDefault(tableName, ImmutableSet.of()); } @Override diff --git a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHive.java b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHive.java index 8cbba9f75c2b..14767cbde0d6 100644 --- a/plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHive.java +++ b/plugin/trino-hive/src/test/java/io/trino/plugin/hive/AbstractTestHive.java @@ -884,7 +884,7 @@ public Optional getMaterializedView(Connect Optional.empty(), Optional.empty(), Optional.empty(), - ImmutableList.of(new ConnectorMaterializedViewDefinition.Column("abc", TypeId.of("type"))), + ImmutableList.of(new ConnectorMaterializedViewDefinition.Column("abc", TypeId.of("type"), Optional.empty())), Optional.of(java.time.Duration.ZERO), Optional.empty(), Optional.of("alice"), diff --git a/testing/trino-tests/src/test/java/io/trino/execution/TestEventListenerBasic.java b/testing/trino-tests/src/test/java/io/trino/execution/TestEventListenerBasic.java index 200f0ddf307f..7f3c05ffcff7 100644 --- a/testing/trino-tests/src/test/java/io/trino/execution/TestEventListenerBasic.java +++ b/testing/trino-tests/src/test/java/io/trino/execution/TestEventListenerBasic.java @@ -167,7 +167,7 @@ public Iterable getConnectorFactories() Optional.empty(), Optional.empty(), Optional.empty(), - ImmutableList.of(new Column("test_column", BIGINT.getTypeId())), + ImmutableList.of(new Column("test_column", BIGINT.getTypeId(), Optional.empty())), Optional.of(Duration.ZERO), Optional.empty(), Optional.of("alice"), diff --git a/testing/trino-tests/src/test/java/io/trino/execution/TestRefreshMaterializedView.java b/testing/trino-tests/src/test/java/io/trino/execution/TestRefreshMaterializedView.java index 6c45998fbf4f..b805209c0b6b 100644 --- a/testing/trino-tests/src/test/java/io/trino/execution/TestRefreshMaterializedView.java +++ b/testing/trino-tests/src/test/java/io/trino/execution/TestRefreshMaterializedView.java @@ -102,7 +102,7 @@ protected QueryRunner createQueryRunner() Optional.of(new CatalogSchemaTableName("mock", "default", "test_storage")), Optional.of("mock"), Optional.of("default"), - ImmutableList.of(new ConnectorMaterializedViewDefinition.Column("nationkey", BIGINT.getTypeId())), + ImmutableList.of(new ConnectorMaterializedViewDefinition.Column("nationkey", BIGINT.getTypeId(), Optional.empty())), Optional.of(Duration.ZERO), Optional.empty(), Optional.of("alice"), 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 195d43bae951..6624bfbbf9db 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 @@ -210,7 +210,7 @@ public Map apply(Connector Optional.empty(), Optional.empty(), Optional.empty(), - ImmutableList.of(new ConnectorMaterializedViewDefinition.Column("test", BIGINT.getTypeId())), + ImmutableList.of(new ConnectorMaterializedViewDefinition.Column("test", BIGINT.getTypeId(), Optional.empty())), Optional.of(Duration.ZERO), Optional.of("comment"), Optional.of("owner"), diff --git a/testing/trino-tests/src/test/java/io/trino/tests/TestMockConnector.java b/testing/trino-tests/src/test/java/io/trino/tests/TestMockConnector.java index 8defcb268fdb..62e927c58b65 100644 --- a/testing/trino-tests/src/test/java/io/trino/tests/TestMockConnector.java +++ b/testing/trino-tests/src/test/java/io/trino/tests/TestMockConnector.java @@ -102,7 +102,7 @@ protected QueryRunner createQueryRunner() Optional.of(new CatalogSchemaTableName("mock", "default", "test_storage")), Optional.of("mock"), Optional.of("default"), - ImmutableList.of(new Column("nationkey", BIGINT.getTypeId())), + ImmutableList.of(new Column("nationkey", BIGINT.getTypeId(), Optional.empty())), Optional.of(Duration.ZERO), Optional.empty(), Optional.of("alice"),