From 2175ff5f8be9dc877058f23a7e1c9f21328767d9 Mon Sep 17 00:00:00 2001 From: pratyakshsharma Date: Fri, 4 Oct 2024 16:45:05 +0530 Subject: [PATCH] Add ALTER TABLE SET PROPERTIES statement Cherry-pick of https://github.com/trinodb/trino/pull/9401/commits/72df2b94a15343498c29133dadec68e2881d258e Co-authored-by: ebyhr --- .../sql/analyzer/utils/StatementUtils.java | 2 + .../src/main/sphinx/sql/alter-table.rst | 5 + .../hive/security/LegacyAccessControl.java | 6 + .../security/SqlStandardAccessControl.java | 20 +++ .../SystemTableAwareAccessControl.java | 7 + .../presto/execution/SetPropertiesTask.java | 95 ++++++++++++++ .../metadata/AbstractPropertyManager.java | 36 +++++- .../metadata/DelegatingMetadataManager.java | 5 + .../facebook/presto/metadata/Metadata.java | 5 + .../presto/metadata/MetadataManager.java | 8 ++ .../presto/metadata/MetadataUtil.java | 7 + .../presto/security/AccessControlManager.java | 13 ++ .../security/AllowAllSystemAccessControl.java | 4 + .../FileBasedSystemAccessControl.java | 8 ++ .../presto/testing/LocalQueryRunner.java | 3 + .../testing/TestingAccessControlManager.java | 17 ++- .../util/PrestoDataDefBindingHelper.java | 3 + .../connector/MockConnectorFactory.java | 4 + .../presto/metadata/AbstractMockMetadata.java | 6 + .../TestFileBasedSystemAccessControl.java | 5 + .../com/facebook/presto/sql/parser/SqlBase.g4 | 2 + .../com/facebook/presto/sql/SqlFormatter.java | 22 ++++ .../presto/sql/parser/AstBuilder.java | 17 +++ .../facebook/presto/sql/tree/AstVisitor.java | 5 + .../presto/sql/tree/SetProperties.java | 120 ++++++++++++++++++ .../presto/sql/parser/TestSqlParser.java | 15 +++ .../sql/parser/TestStatementBuilder.java | 4 + .../base/security/AllowAllAccessControl.java | 6 + .../base/security/FileBasedAccessControl.java | 10 ++ .../ForwardingConnectorAccessControl.java | 7 + .../ForwardingSystemAccessControl.java | 6 + .../security/TestFileBasedAccessControl.java | 4 + .../src/test/resources/table.json | 6 + .../spi/connector/ConnectorAccessControl.java | 12 ++ .../spi/connector/ConnectorMetadata.java | 5 + .../ClassLoaderSafeConnectorMetadata.java | 7 + .../presto/spi/security/AccessControl.java | 8 ++ .../spi/security/AccessDeniedException.java | 10 ++ .../spi/security/AllowAllAccessControl.java | 6 + .../spi/security/DenyAllAccessControl.java | 8 ++ .../spi/security/SystemAccessControl.java | 11 ++ 41 files changed, 543 insertions(+), 7 deletions(-) create mode 100644 presto-main/src/main/java/com/facebook/presto/execution/SetPropertiesTask.java create mode 100644 presto-parser/src/main/java/com/facebook/presto/sql/tree/SetProperties.java diff --git a/presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/utils/StatementUtils.java b/presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/utils/StatementUtils.java index 797ab5dcf614a..0a2b34bebef10 100644 --- a/presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/utils/StatementUtils.java +++ b/presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/utils/StatementUtils.java @@ -55,6 +55,7 @@ import com.facebook.presto.sql.tree.Revoke; import com.facebook.presto.sql.tree.RevokeRoles; import com.facebook.presto.sql.tree.Rollback; +import com.facebook.presto.sql.tree.SetProperties; import com.facebook.presto.sql.tree.SetRole; import com.facebook.presto.sql.tree.SetSession; import com.facebook.presto.sql.tree.ShowCatalogs; @@ -142,6 +143,7 @@ private StatementUtils() {} builder.put(DropFunction.class, QueryType.CONTROL); builder.put(Use.class, QueryType.CONTROL); builder.put(SetSession.class, QueryType.CONTROL); + builder.put(SetProperties.class, QueryType.DATA_DEFINITION); builder.put(ResetSession.class, QueryType.CONTROL); builder.put(StartTransaction.class, QueryType.CONTROL); builder.put(Commit.class, QueryType.CONTROL); diff --git a/presto-docs/src/main/sphinx/sql/alter-table.rst b/presto-docs/src/main/sphinx/sql/alter-table.rst index abb3e94b2eeba..451b4dd2794b6 100644 --- a/presto-docs/src/main/sphinx/sql/alter-table.rst +++ b/presto-docs/src/main/sphinx/sql/alter-table.rst @@ -14,6 +14,7 @@ Synopsis ALTER TABLE [ IF EXISTS ] name ADD [ CONSTRAINT constraint_name ] { PRIMARY KEY | UNIQUE } ( { column_name [, ...] } ) [ { ENABLED | DISABLED } ] [ [ NOT ] RELY ] [ [ NOT ] ENFORCED } ] ALTER TABLE [ IF EXISTS ] name DROP CONSTRAINT [ IF EXISTS ] constraint_name ALTER TABLE [ IF EXISTS ] name ALTER [ COLUMN ] column_name { SET | DROP } NOT NULL + ALTER TABLE [ IF EXISTS ] name SET PROPERTIES (property_name=value, [, ...]) Description ----------- @@ -89,6 +90,10 @@ Drop not null constraint from column ``zip`` in the ``users`` table:: ALTER TABLE users ALTER COLUMN zip DROP NOT NULL; +Set table property (``x=y``) to table ``users``:: + + ALTER TABLE users SET PROPERTIES (x='y'); + See Also -------- diff --git a/presto-hive/src/main/java/com/facebook/presto/hive/security/LegacyAccessControl.java b/presto-hive/src/main/java/com/facebook/presto/hive/security/LegacyAccessControl.java index a4a971ab2d692..2277d1dab0f53 100644 --- a/presto-hive/src/main/java/com/facebook/presto/hive/security/LegacyAccessControl.java +++ b/presto-hive/src/main/java/com/facebook/presto/hive/security/LegacyAccessControl.java @@ -29,6 +29,7 @@ import javax.inject.Inject; +import java.util.Map; import java.util.Optional; import java.util.Set; @@ -101,6 +102,11 @@ public void checkCanCreateTable(ConnectorTransactionHandle transaction, Connecto { } + @Override + public void checkCanSetTableProperties(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName, Map properties) + { + } + @Override public void checkCanDropTable(ConnectorTransactionHandle transaction, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName) { diff --git a/presto-hive/src/main/java/com/facebook/presto/hive/security/SqlStandardAccessControl.java b/presto-hive/src/main/java/com/facebook/presto/hive/security/SqlStandardAccessControl.java index 44447422f154d..6fd805e34c5e2 100644 --- a/presto-hive/src/main/java/com/facebook/presto/hive/security/SqlStandardAccessControl.java +++ b/presto-hive/src/main/java/com/facebook/presto/hive/security/SqlStandardAccessControl.java @@ -33,6 +33,7 @@ import javax.inject.Inject; +import java.util.Map; import java.util.Optional; import java.util.Set; @@ -74,6 +75,7 @@ import static com.facebook.presto.spi.security.AccessDeniedException.denySelectTable; import static com.facebook.presto.spi.security.AccessDeniedException.denySetCatalogSessionProperty; import static com.facebook.presto.spi.security.AccessDeniedException.denySetRole; +import static com.facebook.presto.spi.security.AccessDeniedException.denySetTableProperties; import static com.facebook.presto.spi.security.AccessDeniedException.denyShowRoles; import static com.facebook.presto.spi.security.AccessDeniedException.denyTruncateTable; import static com.facebook.presto.spi.security.AccessDeniedException.denyUpdateTableColumns; @@ -186,6 +188,24 @@ public void checkCanCreateTable(ConnectorTransactionHandle transaction, Connecto } } + @Override + public void checkCanSetTableProperties(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName, Map properties) + { + MetastoreContext metastoreContext = new MetastoreContext(identity, + context.getQueryId().getId(), + context.getClientInfo(), + context.getClientTags(), + context.getSource(), + Optional.empty(), + false, + HiveColumnConverterProvider.DEFAULT_COLUMN_CONVERTER_PROVIDER, + context.getWarningCollector(), + context.getRuntimeStats()); + if (!isTableOwner(transactionHandle, identity, metastoreContext, tableName)) { + denySetTableProperties(tableName.toString()); + } + } + @Override public void checkCanDropTable(ConnectorTransactionHandle transaction, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName) { diff --git a/presto-hive/src/main/java/com/facebook/presto/hive/security/SystemTableAwareAccessControl.java b/presto-hive/src/main/java/com/facebook/presto/hive/security/SystemTableAwareAccessControl.java index b6271dec0a733..9f2b182d39d4b 100644 --- a/presto-hive/src/main/java/com/facebook/presto/hive/security/SystemTableAwareAccessControl.java +++ b/presto-hive/src/main/java/com/facebook/presto/hive/security/SystemTableAwareAccessControl.java @@ -25,6 +25,7 @@ import com.facebook.presto.spi.security.PrestoPrincipal; import com.facebook.presto.spi.security.Privilege; +import java.util.Map; import java.util.Optional; import java.util.Set; @@ -83,6 +84,12 @@ public void checkCanCreateTable(ConnectorTransactionHandle transactionHandle, Co delegate.checkCanCreateTable(transactionHandle, identity, context, tableName); } + @Override + public void checkCanSetTableProperties(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName, Map properties) + { + delegate.checkCanSetTableProperties(transactionHandle, identity, context, tableName, properties); + } + @Override public void checkCanDropTable(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName) { diff --git a/presto-main/src/main/java/com/facebook/presto/execution/SetPropertiesTask.java b/presto-main/src/main/java/com/facebook/presto/execution/SetPropertiesTask.java new file mode 100644 index 0000000000000..d9ccb217bfe51 --- /dev/null +++ b/presto-main/src/main/java/com/facebook/presto/execution/SetPropertiesTask.java @@ -0,0 +1,95 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.facebook.presto.execution; + +import com.facebook.presto.Session; +import com.facebook.presto.common.QualifiedObjectName; +import com.facebook.presto.metadata.Metadata; +import com.facebook.presto.metadata.MetadataUtil; +import com.facebook.presto.spi.PrestoException; +import com.facebook.presto.spi.TableHandle; +import com.facebook.presto.spi.WarningCollector; +import com.facebook.presto.spi.security.AccessControl; +import com.facebook.presto.sql.tree.Expression; +import com.facebook.presto.sql.tree.SetProperties; +import com.facebook.presto.transaction.TransactionManager; +import com.google.common.util.concurrent.ListenableFuture; + +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import static com.facebook.presto.metadata.MetadataUtil.getConnectorIdOrThrow; +import static com.facebook.presto.spi.StandardErrorCode.NOT_FOUND; +import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED; +import static com.facebook.presto.sql.NodeUtils.mapFromProperties; +import static com.facebook.presto.sql.analyzer.utils.ParameterUtils.parameterExtractor; +import static com.facebook.presto.sql.tree.SetProperties.Type.TABLE; +import static com.google.common.util.concurrent.Futures.immediateFuture; +import static java.lang.String.format; + +public class SetPropertiesTask + implements DDLDefinitionTask +{ + @Override + public String getName() + { + return "SET PROPERTIES"; + } + + @Override + public ListenableFuture execute(SetProperties statement, TransactionManager transactionManager, Metadata metadata, AccessControl accessControl, Session session, List parameters, WarningCollector warningCollector) + { + QualifiedObjectName tableName = MetadataUtil.createQualifiedObjectName(session, statement, statement.getTableName()); + Map sqlProperties = mapFromProperties(statement.getProperties()); + + if (statement.getType() == TABLE) { + Map properties = metadata.getTablePropertyManager().getUserSpecifiedProperties( + getConnectorIdOrThrow(session, metadata, tableName.getCatalogName()), + tableName.getCatalogName(), + sqlProperties, + session, + metadata, + parameterExtractor(statement, parameters)).build(); + setTableProperties(statement, tableName, metadata, accessControl, session, properties); + } + else { + throw new PrestoException(NOT_SUPPORTED, format("Unsupported target type: %s", statement.getType())); + } + + return immediateFuture(null); + } + + private void setTableProperties(SetProperties statement, QualifiedObjectName tableName, Metadata metadata, AccessControl accessControl, Session session, Map properties) + { + if (metadata.getMetadataResolver(session).getMaterializedView(tableName).isPresent()) { + throw new PrestoException(NOT_SUPPORTED, "Cannot set table properties to a materialized view"); + } + + if (metadata.getMetadataResolver(session).getView(tableName).isPresent()) { + throw new PrestoException(NOT_SUPPORTED, "Cannot set table properties to a view"); + } + + Optional tableHandle = metadata.getMetadataResolver(session).getTableHandle(tableName); + if (!tableHandle.isPresent()) { + if (!statement.isTableExists()) { + throw new PrestoException(NOT_FOUND, format("Table does not exist: %s", tableName)); + } + return; + } + + accessControl.checkCanSetTableProperties(session.getRequiredTransactionId(), session.getIdentity(), session.getAccessControlContext(), tableName, properties); + metadata.setTableProperties(session, tableHandle.get(), properties); + } +} diff --git a/presto-main/src/main/java/com/facebook/presto/metadata/AbstractPropertyManager.java b/presto-main/src/main/java/com/facebook/presto/metadata/AbstractPropertyManager.java index 630713273423e..11b4b805d6a59 100644 --- a/presto-main/src/main/java/com/facebook/presto/metadata/AbstractPropertyManager.java +++ b/presto-main/src/main/java/com/facebook/presto/metadata/AbstractPropertyManager.java @@ -70,7 +70,7 @@ public final void removeProperties(ConnectorId connectorId) connectorProperties.remove(connectorId); } - public final Map getProperties( + public final ImmutableMap.Builder getUserSpecifiedProperties( ConnectorId connectorId, String catalog, // only use this for error messages Map sqlPropertyValues, @@ -78,11 +78,7 @@ public final Map getProperties( Metadata metadata, Map, Expression> parameters) { - Map> supportedProperties = connectorProperties.get(connectorId); - if (supportedProperties == null) { - throw new PrestoException(NOT_FOUND, "Catalog not found: " + catalog); - } - + Map> supportedProperties = getSupportedProperties(connectorId, catalog); ImmutableMap.Builder properties = ImmutableMap.builder(); // Fill in user-specified properties @@ -125,6 +121,25 @@ public final Map getProperties( properties.put(property.getName(), value); } + return properties; + } + + public final Map getProperties( + ConnectorId connectorId, + String catalog, // only use this for error messages + Map sqlPropertyValues, + Session session, + Metadata metadata, + Map, Expression> parameters) + { + Map> supportedProperties = getSupportedProperties(connectorId, catalog); + ImmutableMap.Builder properties = getUserSpecifiedProperties( + connectorId, + catalog, + sqlPropertyValues, + session, + metadata, + parameters); Map userSpecifiedProperties = properties.build(); // Fill in the remaining properties with non-null defaults @@ -144,6 +159,15 @@ public Map>> getAllProperties() return ImmutableMap.copyOf(connectorProperties); } + private Map> getSupportedProperties(ConnectorId connectorId, String catalog) + { + Map> supportedProperties = connectorProperties.get(connectorId); + if (supportedProperties == null) { + throw new PrestoException(NOT_FOUND, "Catalog not found: " + catalog); + } + return supportedProperties; + } + private Object evaluatePropertyValue(Expression expression, Type expectedType, Session session, Metadata metadata, Map, Expression> parameters) { Expression rewritten = ExpressionTreeRewriter.rewriteWith(new ParameterRewriter(parameters), expression); diff --git a/presto-main/src/main/java/com/facebook/presto/metadata/DelegatingMetadataManager.java b/presto-main/src/main/java/com/facebook/presto/metadata/DelegatingMetadataManager.java index eb9e184625fba..6d69af471ee9e 100644 --- a/presto-main/src/main/java/com/facebook/presto/metadata/DelegatingMetadataManager.java +++ b/presto-main/src/main/java/com/facebook/presto/metadata/DelegatingMetadataManager.java @@ -239,6 +239,11 @@ public void renameTable(Session session, TableHandle tableHandle, QualifiedObjec delegate.renameTable(session, tableHandle, newTableName); } + public void setTableProperties(Session session, TableHandle tableHandle, Map properties) + { + delegate.setTableProperties(session, tableHandle, properties); + } + @Override public void renameColumn(Session session, TableHandle tableHandle, ColumnHandle source, String target) { diff --git a/presto-main/src/main/java/com/facebook/presto/metadata/Metadata.java b/presto-main/src/main/java/com/facebook/presto/metadata/Metadata.java index 55d5958ffd2a1..d47d5fe1bb630 100644 --- a/presto-main/src/main/java/com/facebook/presto/metadata/Metadata.java +++ b/presto-main/src/main/java/com/facebook/presto/metadata/Metadata.java @@ -212,6 +212,11 @@ public interface Metadata */ void renameTable(Session session, TableHandle tableHandle, QualifiedObjectName newTableName); + /** + * Set properties to the specified table. + */ + void setTableProperties(Session session, TableHandle tableHandle, Map properties); + /** * Rename the specified column. */ diff --git a/presto-main/src/main/java/com/facebook/presto/metadata/MetadataManager.java b/presto-main/src/main/java/com/facebook/presto/metadata/MetadataManager.java index 26c0ce32e4338..f001d50b43271 100644 --- a/presto-main/src/main/java/com/facebook/presto/metadata/MetadataManager.java +++ b/presto-main/src/main/java/com/facebook/presto/metadata/MetadataManager.java @@ -674,6 +674,14 @@ public void renameTable(Session session, TableHandle tableHandle, QualifiedObjec metadata.renameTable(session.toConnectorSession(connectorId), tableHandle.getConnectorHandle(), toSchemaTableName(newTableName)); } + @Override + public void setTableProperties(Session session, TableHandle tableHandle, Map properties) + { + ConnectorId connectorId = tableHandle.getConnectorId(); + ConnectorMetadata metadata = getMetadataForWrite(session, connectorId); + metadata.setTableProperties(session.toConnectorSession(connectorId), tableHandle.getConnectorHandle(), properties); + } + @Override public void renameColumn(Session session, TableHandle tableHandle, ColumnHandle source, String target) { diff --git a/presto-main/src/main/java/com/facebook/presto/metadata/MetadataUtil.java b/presto-main/src/main/java/com/facebook/presto/metadata/MetadataUtil.java index 243eb92717a97..b00783903ad95 100644 --- a/presto-main/src/main/java/com/facebook/presto/metadata/MetadataUtil.java +++ b/presto-main/src/main/java/com/facebook/presto/metadata/MetadataUtil.java @@ -40,6 +40,7 @@ import java.util.List; import java.util.Optional; +import static com.facebook.presto.spi.StandardErrorCode.NOT_FOUND; import static com.facebook.presto.spi.StandardErrorCode.SYNTAX_ERROR; import static com.facebook.presto.spi.security.PrincipalType.ROLE; import static com.facebook.presto.spi.security.PrincipalType.USER; @@ -84,6 +85,12 @@ public static SchemaTableName toSchemaTableName(QualifiedObjectName qualifiedObj return new SchemaTableName(qualifiedObjectName.getSchemaName(), qualifiedObjectName.getObjectName()); } + public static ConnectorId getConnectorIdOrThrow(Session session, Metadata metadata, String catalogName) + { + return metadata.getCatalogHandle(session, catalogName) + .orElseThrow(() -> new PrestoException(NOT_FOUND, "Catalog does not exist: " + catalogName)); + } + public static String checkLowerCase(String value, String name) { if (value == null) { diff --git a/presto-main/src/main/java/com/facebook/presto/security/AccessControlManager.java b/presto-main/src/main/java/com/facebook/presto/security/AccessControlManager.java index 011db98e315a7..d4d0125959c53 100644 --- a/presto-main/src/main/java/com/facebook/presto/security/AccessControlManager.java +++ b/presto-main/src/main/java/com/facebook/presto/security/AccessControlManager.java @@ -337,6 +337,19 @@ public void checkCanRenameTable(TransactionId transactionId, Identity identity, } } + @Override + public void checkCanSetTableProperties(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName, Map properties) + { + requireNonNull(identity, "identity is null"); + requireNonNull(tableName, "tableName is null"); + authenticationCheck(() -> checkCanAccessCatalog(identity, context, tableName.getCatalogName())); + authorizationCheck(() -> systemAccessControl.get().checkCanSetTableProperties(identity, context, toCatalogSchemaTableName(tableName))); + CatalogAccessControlEntry entry = getConnectorAccessControl(transactionId, tableName.getCatalogName()); + if (entry != null) { + authorizationCheck(() -> entry.getAccessControl().checkCanSetTableProperties(entry.getTransactionHandle(transactionId), identity.toConnectorIdentity(tableName.getCatalogName()), context, toSchemaTableName(tableName), properties)); + } + } + @Override public void checkCanShowTablesMetadata(TransactionId transactionId, Identity identity, AccessControlContext context, CatalogSchemaName schema) { diff --git a/presto-main/src/main/java/com/facebook/presto/security/AllowAllSystemAccessControl.java b/presto-main/src/main/java/com/facebook/presto/security/AllowAllSystemAccessControl.java index 01f260f64b6db..4197801047f77 100644 --- a/presto-main/src/main/java/com/facebook/presto/security/AllowAllSystemAccessControl.java +++ b/presto-main/src/main/java/com/facebook/presto/security/AllowAllSystemAccessControl.java @@ -122,6 +122,10 @@ public void checkCanCreateTable(Identity identity, AccessControlContext context, { } + public void checkCanSetTableProperties(Identity identity, AccessControlContext context, CatalogSchemaTableName table) + { + } + @Override public void checkCanDropTable(Identity identity, AccessControlContext context, CatalogSchemaTableName table) { diff --git a/presto-main/src/main/java/com/facebook/presto/security/FileBasedSystemAccessControl.java b/presto-main/src/main/java/com/facebook/presto/security/FileBasedSystemAccessControl.java index 8a4aab91a559a..f9bf1d5b976d8 100644 --- a/presto-main/src/main/java/com/facebook/presto/security/FileBasedSystemAccessControl.java +++ b/presto-main/src/main/java/com/facebook/presto/security/FileBasedSystemAccessControl.java @@ -66,6 +66,7 @@ import static com.facebook.presto.spi.security.AccessDeniedException.denyRenameSchema; import static com.facebook.presto.spi.security.AccessDeniedException.denyRenameTable; import static com.facebook.presto.spi.security.AccessDeniedException.denyRevokeTablePrivilege; +import static com.facebook.presto.spi.security.AccessDeniedException.denySetTableProperties; import static com.facebook.presto.spi.security.AccessDeniedException.denySetUser; import static com.facebook.presto.spi.security.AccessDeniedException.denyTruncateTable; import static com.facebook.presto.spi.security.AccessDeniedException.denyUpdateTableColumns; @@ -280,6 +281,13 @@ public void checkCanCreateTable(Identity identity, AccessControlContext context, } } + public void checkCanSetTableProperties(Identity identity, AccessControlContext context, CatalogSchemaTableName table) + { + if (!canAccessCatalog(identity, table.getCatalogName(), ALL)) { + denySetTableProperties(table.toString()); + } + } + @Override public void checkCanDropTable(Identity identity, AccessControlContext context, CatalogSchemaTableName table) { diff --git a/presto-main/src/main/java/com/facebook/presto/testing/LocalQueryRunner.java b/presto-main/src/main/java/com/facebook/presto/testing/LocalQueryRunner.java index d200afe8d916c..3656b5d734936 100644 --- a/presto-main/src/main/java/com/facebook/presto/testing/LocalQueryRunner.java +++ b/presto-main/src/main/java/com/facebook/presto/testing/LocalQueryRunner.java @@ -73,6 +73,7 @@ import com.facebook.presto.execution.ResetSessionTask; import com.facebook.presto.execution.RollbackTask; import com.facebook.presto.execution.ScheduledSplit; +import com.facebook.presto.execution.SetPropertiesTask; import com.facebook.presto.execution.SetSessionTask; import com.facebook.presto.execution.StartTransactionTask; import com.facebook.presto.execution.TaskManagerConfig; @@ -211,6 +212,7 @@ import com.facebook.presto.sql.tree.RenameTable; import com.facebook.presto.sql.tree.ResetSession; import com.facebook.presto.sql.tree.Rollback; +import com.facebook.presto.sql.tree.SetProperties; import com.facebook.presto.sql.tree.SetSession; import com.facebook.presto.sql.tree.StartTransaction; import com.facebook.presto.sql.tree.Statement; @@ -583,6 +585,7 @@ private LocalQueryRunner(Session defaultSession, FeaturesConfig featuresConfig, .put(StartTransaction.class, new StartTransactionTask()) .put(Commit.class, new CommitTask()) .put(Rollback.class, new RollbackTask()) + .put(SetProperties.class, new SetPropertiesTask()) .build(); SpillerStats spillerStats = new SpillerStats(); diff --git a/presto-main/src/main/java/com/facebook/presto/testing/TestingAccessControlManager.java b/presto-main/src/main/java/com/facebook/presto/testing/TestingAccessControlManager.java index 86658c0d2992e..1db74c076d0b2 100644 --- a/presto-main/src/main/java/com/facebook/presto/testing/TestingAccessControlManager.java +++ b/presto-main/src/main/java/com/facebook/presto/testing/TestingAccessControlManager.java @@ -29,6 +29,7 @@ import java.security.Principal; import java.util.Collections; import java.util.HashSet; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -52,6 +53,7 @@ import static com.facebook.presto.spi.security.AccessDeniedException.denySelectColumns; import static com.facebook.presto.spi.security.AccessDeniedException.denySetCatalogSessionProperty; import static com.facebook.presto.spi.security.AccessDeniedException.denySetSystemSessionProperty; +import static com.facebook.presto.spi.security.AccessDeniedException.denySetTableProperties; import static com.facebook.presto.spi.security.AccessDeniedException.denySetUser; import static com.facebook.presto.spi.security.AccessDeniedException.denyTruncateTable; import static com.facebook.presto.spi.security.AccessDeniedException.denyUpdateTableColumns; @@ -73,6 +75,7 @@ import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.RENAME_TABLE; import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.SELECT_COLUMN; import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.SET_SESSION; +import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.SET_TABLE_PROPERTIES; import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.SET_USER; import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.TRUNCATE_TABLE; import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.UPDATE_TABLE; @@ -189,6 +192,18 @@ public void checkCanRenameTable(TransactionId transactionId, Identity identity, } } + @Override + public void checkCanSetTableProperties(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName, Map properties) + { + if (shouldDenyPrivilege(identity.getUser(), tableName.getObjectName(), SET_TABLE_PROPERTIES)) { + denySetTableProperties(tableName.toString()); + } + + if (denyPrivileges.isEmpty()) { + super.checkCanSetTableProperties(transactionId, identity, context, tableName, properties); + } + } + @Override public void checkCanAddColumns(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName) { @@ -368,7 +383,7 @@ public enum TestingPrivilegeType CREATE_TABLE, DROP_TABLE, RENAME_TABLE, INSERT_TABLE, DELETE_TABLE, TRUNCATE_TABLE, UPDATE_TABLE, ADD_COLUMN, DROP_COLUMN, RENAME_COLUMN, SELECT_COLUMN, ADD_CONSTRAINT, DROP_CONSTRAINT, - CREATE_VIEW, DROP_VIEW, CREATE_VIEW_WITH_SELECT_COLUMNS, + CREATE_VIEW, DROP_VIEW, CREATE_VIEW_WITH_SELECT_COLUMNS, SET_TABLE_PROPERTIES, SET_SESSION } diff --git a/presto-main/src/main/java/com/facebook/presto/util/PrestoDataDefBindingHelper.java b/presto-main/src/main/java/com/facebook/presto/util/PrestoDataDefBindingHelper.java index ce4a41e5f0911..f83e282373e0d 100644 --- a/presto-main/src/main/java/com/facebook/presto/util/PrestoDataDefBindingHelper.java +++ b/presto-main/src/main/java/com/facebook/presto/util/PrestoDataDefBindingHelper.java @@ -46,6 +46,7 @@ import com.facebook.presto.execution.RevokeRolesTask; import com.facebook.presto.execution.RevokeTask; import com.facebook.presto.execution.RollbackTask; +import com.facebook.presto.execution.SetPropertiesTask; import com.facebook.presto.execution.SetRoleTask; import com.facebook.presto.execution.SetSessionTask; import com.facebook.presto.execution.StartTransactionTask; @@ -83,6 +84,7 @@ import com.facebook.presto.sql.tree.Revoke; import com.facebook.presto.sql.tree.RevokeRoles; import com.facebook.presto.sql.tree.Rollback; +import com.facebook.presto.sql.tree.SetProperties; import com.facebook.presto.sql.tree.SetRole; import com.facebook.presto.sql.tree.SetSession; import com.facebook.presto.sql.tree.StartTransaction; @@ -146,6 +148,7 @@ private PrestoDataDefBindingHelper() {} transactionDefBuilder.put(Commit.class, CommitTask.class); transactionDefBuilder.put(Rollback.class, RollbackTask.class); transactionDefBuilder.put(SetRole.class, SetRoleTask.class); + transactionDefBuilder.put(SetProperties.class, SetPropertiesTask.class); transactionDefBuilder.put(Prepare.class, PrepareTask.class); transactionDefBuilder.put(Deallocate.class, DeallocateTask.class); transactionDefBuilder.put(CreateFunction.class, CreateFunctionTask.class); diff --git a/presto-main/src/test/java/com/facebook/presto/connector/MockConnectorFactory.java b/presto-main/src/test/java/com/facebook/presto/connector/MockConnectorFactory.java index 58a1e343398f5..6b12c9aac74fd 100644 --- a/presto-main/src/test/java/com/facebook/presto/connector/MockConnectorFactory.java +++ b/presto-main/src/test/java/com/facebook/presto/connector/MockConnectorFactory.java @@ -179,6 +179,10 @@ public List listTables(ConnectorSession session, String schemaN return listTables.apply(session, schemaNameOrNull); } + public void setTableProperties(ConnectorSession session, ConnectorTableHandle tableHandle, Map properties) + { + } + @Override public Map getColumnHandles(ConnectorSession session, ConnectorTableHandle tableHandle) { diff --git a/presto-main/src/test/java/com/facebook/presto/metadata/AbstractMockMetadata.java b/presto-main/src/test/java/com/facebook/presto/metadata/AbstractMockMetadata.java index 995db6d5571ed..b822bfbf2a90f 100644 --- a/presto-main/src/test/java/com/facebook/presto/metadata/AbstractMockMetadata.java +++ b/presto-main/src/test/java/com/facebook/presto/metadata/AbstractMockMetadata.java @@ -284,6 +284,12 @@ public void renameTable(Session session, TableHandle tableHandle, QualifiedObjec throw new UnsupportedOperationException(); } + @Override + public void setTableProperties(Session session, TableHandle tableHandle, Map properties) + { + throw new UnsupportedOperationException(); + } + @Override public void renameColumn(Session session, TableHandle tableHandle, ColumnHandle source, String target) { diff --git a/presto-main/src/test/java/com/facebook/presto/security/TestFileBasedSystemAccessControl.java b/presto-main/src/test/java/com/facebook/presto/security/TestFileBasedSystemAccessControl.java index 9d0d574e7805d..f9f661381e659 100644 --- a/presto-main/src/test/java/com/facebook/presto/security/TestFileBasedSystemAccessControl.java +++ b/presto-main/src/test/java/com/facebook/presto/security/TestFileBasedSystemAccessControl.java @@ -342,6 +342,7 @@ public void testTableOperations() throws IOException accessControlManager.checkCanSelectFromColumns(transactionId, alice, context, aliceTable, ImmutableSet.of()); accessControlManager.checkCanInsertIntoTable(transactionId, alice, context, aliceTable); accessControlManager.checkCanDeleteFromTable(transactionId, alice, context, aliceTable); + accessControlManager.checkCanSetTableProperties(transactionId, alice, context, aliceTable, ImmutableMap.of()); accessControlManager.checkCanAddColumns(transactionId, alice, context, aliceTable); accessControlManager.checkCanRenameColumn(transactionId, alice, context, aliceTable); }); @@ -385,6 +386,10 @@ public void testTableOperationsReadOnly() throws IOException accessControlManager.checkCanDeleteFromTable(transactionId, alice, context, aliceTable); })); + assertThrows(AccessDeniedException.class, () -> transaction(transactionManager, accessControlManager).execute(transactionId -> { + accessControlManager.checkCanSetTableProperties(transactionId, alice, context, aliceTable, ImmutableMap.of()); + })); + assertThrows(AccessDeniedException.class, () -> transaction(transactionManager, accessControlManager).execute(transactionId -> { accessControlManager.checkCanAddColumns(transactionId, alice, context, aliceTable); })); diff --git a/presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4 b/presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4 index f495ed0aab474..c5b80d73eb2b3 100644 --- a/presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4 +++ b/presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4 @@ -66,6 +66,8 @@ statement ALTER (COLUMN)? column=identifier SET NOT NULL #alterColumnSetNotNull | ALTER TABLE (IF EXISTS)? tableName=qualifiedName ALTER (COLUMN)? column=identifier DROP NOT NULL #alterColumnDropNotNull + | ALTER TABLE (IF EXISTS)? tableName=qualifiedName + SET PROPERTIES properties #setTableProperties | ANALYZE qualifiedName (WITH properties)? #analyze | CREATE TYPE qualifiedName AS ( '(' sqlParameterDeclaration (',' sqlParameterDeclaration)* ')' diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java b/presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java index 4d73769e85ed9..ea7c192cc8592 100644 --- a/presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java +++ b/presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java @@ -92,6 +92,7 @@ import com.facebook.presto.sql.tree.SampledRelation; import com.facebook.presto.sql.tree.Select; import com.facebook.presto.sql.tree.SelectItem; +import com.facebook.presto.sql.tree.SetProperties; import com.facebook.presto.sql.tree.SetRole; import com.facebook.presto.sql.tree.SetSession; import com.facebook.presto.sql.tree.ShowCatalogs; @@ -815,6 +816,27 @@ protected Void visitShowTables(ShowTables node, Integer context) return null; } + protected Void visitSetProperties(SetProperties node, Integer context) + { + builder.append("ALTER TABLE "); + if (node.isTableExists()) { + builder.append("IF EXISTS "); + } + builder.append(formatName(node.getTableName())); + builder.append(" SET PROPERTIES ( "); + builder.append(joinProperties(node.getProperties())); + builder.append(" )"); + return null; + } + + private String joinProperties(List properties) + { + return properties.stream() + .map(element -> formatExpression(element.getName(), Optional.empty()) + " = " + + formatExpression(element.getValue(), Optional.empty())) + .collect(joining(", ")); + } + @Override protected Void visitShowCreate(ShowCreate node, Integer context) { diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/parser/AstBuilder.java b/presto-parser/src/main/java/com/facebook/presto/sql/parser/AstBuilder.java index a004fffad308b..f916feed3ad76 100644 --- a/presto-parser/src/main/java/com/facebook/presto/sql/parser/AstBuilder.java +++ b/presto-parser/src/main/java/com/facebook/presto/sql/parser/AstBuilder.java @@ -141,6 +141,7 @@ import com.facebook.presto.sql.tree.SearchedCaseExpression; import com.facebook.presto.sql.tree.Select; import com.facebook.presto.sql.tree.SelectItem; +import com.facebook.presto.sql.tree.SetProperties; import com.facebook.presto.sql.tree.SetRole; import com.facebook.presto.sql.tree.SetSession; import com.facebook.presto.sql.tree.ShowCatalogs; @@ -209,6 +210,7 @@ import static com.facebook.presto.sql.tree.RoutineCharacteristics.NullCallClause; import static com.facebook.presto.sql.tree.RoutineCharacteristics.NullCallClause.CALLED_ON_NULL_INPUT; import static com.facebook.presto.sql.tree.RoutineCharacteristics.NullCallClause.RETURNS_NULL_ON_NULL_INPUT; +import static com.facebook.presto.sql.tree.SetProperties.Type.TABLE; import static com.facebook.presto.sql.tree.TableVersionExpression.TableVersionOperator; import static com.facebook.presto.sql.tree.TableVersionExpression.TableVersionOperator.EQUAL; import static com.facebook.presto.sql.tree.TableVersionExpression.TableVersionOperator.LESS_THAN; @@ -457,6 +459,21 @@ public Node visitRenameTable(SqlBaseParser.RenameTableContext context) return new RenameTable(getLocation(context), getQualifiedName(context.from), getQualifiedName(context.to), context.EXISTS() != null); } + @Override + public Node visitSetTableProperties(SqlBaseParser.SetTablePropertiesContext context) + { + List properties = ImmutableList.of(); + if (context.properties() != null) { + properties = visit(context.properties().property(), Property.class); + } + + return new SetProperties(getLocation(context), + TABLE, + getQualifiedName(context.tableName), + properties, + context.EXISTS() != null); + } + @Override public Node visitRenameColumn(SqlBaseParser.RenameColumnContext context) { diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/tree/AstVisitor.java b/presto-parser/src/main/java/com/facebook/presto/sql/tree/AstVisitor.java index 49d3109c984f5..8eec8cf4936fb 100644 --- a/presto-parser/src/main/java/com/facebook/presto/sql/tree/AstVisitor.java +++ b/presto-parser/src/main/java/com/facebook/presto/sql/tree/AstVisitor.java @@ -127,6 +127,11 @@ protected R visitShowTables(ShowTables node, C context) return visitStatement(node, context); } + protected R visitSetProperties(SetProperties node, C context) + { + return visitStatement(node, context); + } + protected R visitShowSchemas(ShowSchemas node, C context) { return visitStatement(node, context); diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/tree/SetProperties.java b/presto-parser/src/main/java/com/facebook/presto/sql/tree/SetProperties.java new file mode 100644 index 0000000000000..ed7d2ef0797e8 --- /dev/null +++ b/presto-parser/src/main/java/com/facebook/presto/sql/tree/SetProperties.java @@ -0,0 +1,120 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.facebook.presto.sql.tree; + +import com.google.common.collect.ImmutableList; + +import java.util.List; +import java.util.Objects; +import java.util.Optional; + +import static com.google.common.base.MoreObjects.toStringHelper; +import static java.util.Objects.requireNonNull; + +public class SetProperties + extends Statement +{ + public enum Type + { + TABLE + } + + private final Type type; + private final QualifiedName tableName; + private final List properties; + private final boolean tableExists; + + public SetProperties(Type type, QualifiedName name, List properties, boolean tableExists) + { + this(Optional.empty(), type, name, properties, tableExists); + } + + public SetProperties(NodeLocation location, Type type, QualifiedName name, List properties, boolean tableExists) + { + this(Optional.of(location), type, name, properties, tableExists); + } + + private SetProperties(Optional location, Type type, QualifiedName name, List properties, boolean tableExists) + { + super(location); + this.type = requireNonNull(type, "type is null"); + this.tableName = requireNonNull(name, "name is null"); + this.properties = ImmutableList.copyOf(requireNonNull(properties, "properties is null")); + this.tableExists = tableExists; + } + + public Type getType() + { + return type; + } + + public boolean isTableExists() + { + return tableExists; + } + + public QualifiedName getTableName() + { + return tableName; + } + + public List getProperties() + { + return properties; + } + + public R accept(AstVisitor visitor, C context) + { + return visitor.visitSetProperties(this, context); + } + + @Override + public List getChildren() + { + return ImmutableList.of(); + } + + @Override + public int hashCode() + { + return Objects.hash(type, tableName, properties, tableExists); + } + + @Override + public boolean equals(Object obj) + { + if (this == obj) { + return true; + } + if ((obj == null) || (getClass() != obj.getClass())) { + return false; + } + SetProperties o = (SetProperties) obj; + return type == o.type && + Objects.equals(tableName, o.tableName) && + Objects.equals(properties, o.properties) && + Objects.equals(tableExists, o.tableExists); + } + + @Override + public String toString() + { + return toStringHelper(this) + .add("type", type) + .add("name", tableName) + .add("properties", properties) + .add("tableExists", tableExists) + .toString(); + } +} diff --git a/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParser.java b/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParser.java index 7b5bba37d60a4..ce3715b8c296f 100644 --- a/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParser.java +++ b/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParser.java @@ -120,6 +120,7 @@ import com.facebook.presto.sql.tree.Row; import com.facebook.presto.sql.tree.Select; import com.facebook.presto.sql.tree.SelectItem; +import com.facebook.presto.sql.tree.SetProperties; import com.facebook.presto.sql.tree.SetRole; import com.facebook.presto.sql.tree.SetSession; import com.facebook.presto.sql.tree.ShowCatalogs; @@ -1622,6 +1623,20 @@ public void testRenameTable() assertStatement("ALTER TABLE IF EXISTS a RENAME TO b", new RenameTable(QualifiedName.of("a"), QualifiedName.of("b"), true)); } + @Test + public void testSetProperties() + { + assertStatement("ALTER TABLE a SET PROPERTIES (foo='bar')", new SetProperties(SetProperties.Type.TABLE, QualifiedName.of("a"), ImmutableList.of(new Property(new Identifier("foo"), new StringLiteral("bar"))), false)); + assertStatement("ALTER TABLE a SET PROPERTIES (foo=true)", new SetProperties(SetProperties.Type.TABLE, QualifiedName.of("a"), ImmutableList.of(new Property(new Identifier("foo"), new BooleanLiteral("true"))), false)); + assertStatement("ALTER TABLE a SET PROPERTIES (foo=123)", new SetProperties(SetProperties.Type.TABLE, QualifiedName.of("a"), ImmutableList.of(new Property(new Identifier("foo"), new LongLiteral("123"))), false)); + assertStatement("ALTER TABLE a SET PROPERTIES (foo=123, bar=456)", new SetProperties(SetProperties.Type.TABLE, QualifiedName.of("a"), ImmutableList.of(new Property(new Identifier("foo"), new LongLiteral("123")), new Property(new Identifier("bar"), new LongLiteral("456"))), false)); + assertStatement("ALTER TABLE a SET PROPERTIES (\" s p a c e \"='bar')", new SetProperties(SetProperties.Type.TABLE, QualifiedName.of("a"), ImmutableList.of(new Property(new Identifier(" s p a c e "), new StringLiteral("bar"))), false)); + + assertInvalidStatement("ALTER TABLE a SET PROPERTIES ()", "mismatched input '\\)'. Expecting: "); + assertStatement("ALTER TABLE IF EXISTS b SET PROPERTIES (foo=12345)", new SetProperties(SetProperties.Type.TABLE, QualifiedName.of("b"), ImmutableList.of(new Property(new Identifier("foo"), new LongLiteral("12345"))), true)); + assertInvalidStatement("ALTER TABLE IF EXISTS b SET PROPERTIES ()", "mismatched input '\\)'. Expecting: "); + } + @Test public void testRenameColumn() { diff --git a/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestStatementBuilder.java b/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestStatementBuilder.java index edda43375c1ad..b2aaac7baf2e5 100644 --- a/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestStatementBuilder.java +++ b/presto-parser/src/test/java/com/facebook/presto/sql/parser/TestStatementBuilder.java @@ -194,6 +194,10 @@ public void testStatementBuilder() printStatement("alter table foo rename to bar"); printStatement("alter table a.b.c rename to d.e.f"); + printStatement("alter table foo set properties (a='1')"); + printStatement("alter table a.b.c set properties (a=true, b=123, c='x')"); + printStatement("alter table if exists bar set properties (b='1')"); + printStatement("alter table a.b.c rename column x to y"); printStatement("alter table a.b.c add column x bigint"); diff --git a/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/AllowAllAccessControl.java b/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/AllowAllAccessControl.java index 5384a53a3f42e..38f94530bedd5 100644 --- a/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/AllowAllAccessControl.java +++ b/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/AllowAllAccessControl.java @@ -22,6 +22,7 @@ import com.facebook.presto.spi.security.PrestoPrincipal; import com.facebook.presto.spi.security.Privilege; +import java.util.Map; import java.util.Optional; import java.util.Set; @@ -59,6 +60,11 @@ public void checkCanCreateTable(ConnectorTransactionHandle transaction, Connecto { } + @Override + public void checkCanSetTableProperties(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName, Map properties) + { + } + @Override public void checkCanDropTable(ConnectorTransactionHandle transaction, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName) { diff --git a/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/FileBasedAccessControl.java b/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/FileBasedAccessControl.java index f715fd34e6062..d9d13ccdb314b 100644 --- a/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/FileBasedAccessControl.java +++ b/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/FileBasedAccessControl.java @@ -29,6 +29,7 @@ import java.nio.file.Paths; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; @@ -58,6 +59,7 @@ import static com.facebook.presto.spi.security.AccessDeniedException.denyRenameTable; import static com.facebook.presto.spi.security.AccessDeniedException.denyRevokeTablePrivilege; import static com.facebook.presto.spi.security.AccessDeniedException.denySelectTable; +import static com.facebook.presto.spi.security.AccessDeniedException.denySetTableProperties; import static com.facebook.presto.spi.security.AccessDeniedException.denyTruncateTable; import static com.facebook.presto.spi.security.AccessDeniedException.denyUpdateTableColumns; @@ -117,6 +119,14 @@ public void checkCanCreateTable(ConnectorTransactionHandle transaction, Connecto } } + @Override + public void checkCanSetTableProperties(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName, Map properties) + { + if (!checkTablePermission(identity, tableName, OWNERSHIP)) { + denySetTableProperties(tableName.toString()); + } + } + @Override public void checkCanDropTable(ConnectorTransactionHandle transaction, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName) { diff --git a/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ForwardingConnectorAccessControl.java b/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ForwardingConnectorAccessControl.java index c6f769f7c9c2f..bb4f74134f024 100644 --- a/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ForwardingConnectorAccessControl.java +++ b/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ForwardingConnectorAccessControl.java @@ -22,6 +22,7 @@ import com.facebook.presto.spi.security.PrestoPrincipal; import com.facebook.presto.spi.security.Privilege; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.function.Supplier; @@ -94,6 +95,12 @@ public void checkCanRenameTable(ConnectorTransactionHandle transactionHandle, Co delegate().checkCanRenameTable(transactionHandle, identity, context, tableName, newTableName); } + @Override + public void checkCanSetTableProperties(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName, Map properties) + { + delegate().checkCanSetTableProperties(transactionHandle, identity, context, tableName, properties); + } + @Override public void checkCanShowTablesMetadata(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, String schemaName) { diff --git a/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ForwardingSystemAccessControl.java b/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ForwardingSystemAccessControl.java index a280e1a54cd90..0adec4a48aa32 100644 --- a/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ForwardingSystemAccessControl.java +++ b/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ForwardingSystemAccessControl.java @@ -122,6 +122,12 @@ public void checkCanCreateTable(Identity identity, AccessControlContext context, delegate().checkCanCreateTable(identity, context, table); } + @Override + public void checkCanSetTableProperties(Identity identity, AccessControlContext context, CatalogSchemaTableName table) + { + delegate().checkCanSetTableProperties(identity, context, table); + } + @Override public void checkCanDropTable(Identity identity, AccessControlContext context, CatalogSchemaTableName table) { diff --git a/presto-plugin-toolkit/src/test/java/com/facebook/presto/plugin/base/security/TestFileBasedAccessControl.java b/presto-plugin-toolkit/src/test/java/com/facebook/presto/plugin/base/security/TestFileBasedAccessControl.java index 177a0b8ba79d9..1c081887df656 100644 --- a/presto-plugin-toolkit/src/test/java/com/facebook/presto/plugin/base/security/TestFileBasedAccessControl.java +++ b/presto-plugin-toolkit/src/test/java/com/facebook/presto/plugin/base/security/TestFileBasedAccessControl.java @@ -23,6 +23,7 @@ import com.facebook.presto.spi.security.AccessControlContext; import com.facebook.presto.spi.security.AccessDeniedException; import com.facebook.presto.spi.security.ConnectorIdentity; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import org.testng.Assert.ThrowingRunnable; import org.testng.annotations.Test; @@ -69,8 +70,11 @@ public void testTableRules() accessControl.checkCanSelectFromColumns(TRANSACTION_HANDLE, user("joe"), CONTEXT, new SchemaTableName("bobschema", "bobtable"), ImmutableSet.of()); accessControl.checkCanCreateViewWithSelectFromColumns(TRANSACTION_HANDLE, user("bob"), CONTEXT, new SchemaTableName("bobschema", "bobtable"), ImmutableSet.of()); accessControl.checkCanDropTable(TRANSACTION_HANDLE, user("admin"), CONTEXT, new SchemaTableName("bobschema", "bobtable")); + accessControl.checkCanSetTableProperties(TRANSACTION_HANDLE, user("admin"), CONTEXT, new SchemaTableName("bobschema", "bobtable"), ImmutableMap.of()); + accessControl.checkCanSetTableProperties(TRANSACTION_HANDLE, user("alice"), CONTEXT, new SchemaTableName("aliceSchema", "aliceTable"), ImmutableMap.of()); assertDenied(() -> accessControl.checkCanInsertIntoTable(TRANSACTION_HANDLE, user("alice"), CONTEXT, new SchemaTableName("bobschema", "bobtable"))); assertDenied(() -> accessControl.checkCanDropTable(TRANSACTION_HANDLE, user("bob"), CONTEXT, new SchemaTableName("bobschema", "bobtable"))); + assertDenied(() -> accessControl.checkCanSetTableProperties(TRANSACTION_HANDLE, user("bob"), CONTEXT, new SchemaTableName("bobschema", "bobtable"), ImmutableMap.of())); assertDenied(() -> accessControl.checkCanInsertIntoTable(TRANSACTION_HANDLE, user("bob"), CONTEXT, new SchemaTableName("test", "test"))); assertDenied(() -> accessControl.checkCanSelectFromColumns(TRANSACTION_HANDLE, user("admin"), CONTEXT, new SchemaTableName("secret", "secret"), ImmutableSet.of())); assertDenied(() -> accessControl.checkCanSelectFromColumns(TRANSACTION_HANDLE, user("joe"), CONTEXT, new SchemaTableName("secret", "secret"), ImmutableSet.of())); diff --git a/presto-plugin-toolkit/src/test/resources/table.json b/presto-plugin-toolkit/src/test/resources/table.json index 1d52a13fd46ab..18b76320fc162 100644 --- a/presto-plugin-toolkit/src/test/resources/table.json +++ b/presto-plugin-toolkit/src/test/resources/table.json @@ -15,6 +15,12 @@ "table": "bobtable", "privileges": ["SELECT", "INSERT", "DELETE", "GRANT_SELECT"] }, + { + "user": "alice", + "schema": "aliceschema", + "table": "alicetable", + "privileges": ["OWNERSHIP"] + }, { "privileges": ["SELECT"] } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorAccessControl.java b/presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorAccessControl.java index 422d34d3fcbf8..62f7208301ee1 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorAccessControl.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorAccessControl.java @@ -20,6 +20,7 @@ import com.facebook.presto.spi.security.PrestoPrincipal; import com.facebook.presto.spi.security.Privilege; +import java.util.Map; import java.util.Optional; import java.util.Set; @@ -48,6 +49,7 @@ import static com.facebook.presto.spi.security.AccessDeniedException.denySelectColumns; import static com.facebook.presto.spi.security.AccessDeniedException.denySetCatalogSessionProperty; import static com.facebook.presto.spi.security.AccessDeniedException.denySetRole; +import static com.facebook.presto.spi.security.AccessDeniedException.denySetTableProperties; import static com.facebook.presto.spi.security.AccessDeniedException.denyShowCurrentRoles; import static com.facebook.presto.spi.security.AccessDeniedException.denyShowRoleGrants; import static com.facebook.presto.spi.security.AccessDeniedException.denyShowRoles; @@ -121,6 +123,16 @@ default void checkCanCreateTable(ConnectorTransactionHandle transactionHandle, C denyCreateTable(tableName.toString()); } + /** + * Check if identity is allowed to set properties to the specified table. + * + * @throws com.facebook.presto.spi.security.AccessDeniedException if not allowed + */ + default void checkCanSetTableProperties(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName, Map properties) + { + denySetTableProperties(tableName.toString()); + } + /** * Check if identity is allowed to drop the specified table in this catalog. * diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorMetadata.java b/presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorMetadata.java index 313500319bd35..8225d1d129a0f 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorMetadata.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorMetadata.java @@ -376,6 +376,11 @@ default void renameTable(ConnectorSession session, ConnectorTableHandle tableHan throw new PrestoException(NOT_SUPPORTED, "This connector does not support renaming tables"); } + default void setTableProperties(ConnectorSession session, ConnectorTableHandle tableHandle, Map properties) + { + throw new PrestoException(NOT_SUPPORTED, "This connector does not support setting table properties"); + } + /** * Add the specified column */ diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/connector/classloader/ClassLoaderSafeConnectorMetadata.java b/presto-spi/src/main/java/com/facebook/presto/spi/connector/classloader/ClassLoaderSafeConnectorMetadata.java index 73d696ca6a42b..2201e0b98b0bb 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/connector/classloader/ClassLoaderSafeConnectorMetadata.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/connector/classloader/ClassLoaderSafeConnectorMetadata.java @@ -427,6 +427,13 @@ public void renameTable(ConnectorSession session, ConnectorTableHandle tableHand } } + public void setTableProperties(ConnectorSession session, ConnectorTableHandle tableHandle, Map properties) + { + try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) { + delegate.setTableProperties(session, tableHandle, properties); + } + } + @Override public ConnectorOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata, Optional layout) { diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/security/AccessControl.java b/presto-spi/src/main/java/com/facebook/presto/spi/security/AccessControl.java index 0866803040a8a..c75f3ee58e248 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/security/AccessControl.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/security/AccessControl.java @@ -22,6 +22,7 @@ import java.security.Principal; import java.security.cert.X509Certificate; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; @@ -113,6 +114,13 @@ default AuthorizedIdentity selectAuthorizedIdentity(Identity identity, AccessCon */ void checkCanRenameTable(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName, QualifiedObjectName newTableName); + /** + * Check if identity is allowed to set properties to the specified table. + * + * @throws AccessDeniedException if not allowed + */ + void checkCanSetTableProperties(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName, Map properties); + /** * Check if identity is allowed to show metadata of tables by executing SHOW TABLES, SHOW GRANTS etc. in a catalog. *

diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/security/AccessDeniedException.java b/presto-spi/src/main/java/com/facebook/presto/spi/security/AccessDeniedException.java index 4a8ca11bdf593..1766e1b961998 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/security/AccessDeniedException.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/security/AccessDeniedException.java @@ -106,6 +106,16 @@ public static void denyCreateTable(String tableName, String extraInfo) throw new AccessDeniedException(format("Cannot create table %s%s", tableName, formatExtraInfo(extraInfo))); } + public static void denySetTableProperties(String tableName) + { + denySetTableProperties(tableName, null); + } + + public static void denySetTableProperties(String tableName, String extraInfo) + { + throw new AccessDeniedException(format("Cannot set table properties to %s%s", tableName, formatExtraInfo(extraInfo))); + } + public static void denyDropTable(String tableName) { denyDropTable(tableName, null); diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/security/AllowAllAccessControl.java b/presto-spi/src/main/java/com/facebook/presto/spi/security/AllowAllAccessControl.java index f1410a6f5717c..70458ba83ef03 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/security/AllowAllAccessControl.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/security/AllowAllAccessControl.java @@ -20,6 +20,7 @@ import com.facebook.presto.spi.SchemaTableName; import java.security.Principal; +import java.util.Map; import java.util.Optional; import java.util.Set; @@ -88,6 +89,11 @@ public void checkCanRenameTable(TransactionId transactionId, Identity identity, { } + @Override + public void checkCanSetTableProperties(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName, Map properties) + { + } + @Override public void checkCanShowTablesMetadata(TransactionId transactionId, Identity identity, AccessControlContext context, CatalogSchemaName schema) { diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/security/DenyAllAccessControl.java b/presto-spi/src/main/java/com/facebook/presto/spi/security/DenyAllAccessControl.java index a8a7fca4c969f..0fdbbe5576b61 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/security/DenyAllAccessControl.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/security/DenyAllAccessControl.java @@ -21,6 +21,7 @@ import java.security.Principal; import java.util.Collections; +import java.util.Map; import java.util.Optional; import java.util.Set; @@ -52,6 +53,7 @@ import static com.facebook.presto.spi.security.AccessDeniedException.denySetCatalogSessionProperty; import static com.facebook.presto.spi.security.AccessDeniedException.denySetRole; import static com.facebook.presto.spi.security.AccessDeniedException.denySetSystemSessionProperty; +import static com.facebook.presto.spi.security.AccessDeniedException.denySetTableProperties; import static com.facebook.presto.spi.security.AccessDeniedException.denySetUser; import static com.facebook.presto.spi.security.AccessDeniedException.denyShowCurrentRoles; import static com.facebook.presto.spi.security.AccessDeniedException.denyShowRoleGrants; @@ -127,6 +129,12 @@ public void checkCanRenameTable(TransactionId transactionId, Identity identity, denyRenameTable(tableName.toString(), newTableName.toString()); } + @Override + public void checkCanSetTableProperties(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName, Map properties) + { + denySetTableProperties(tableName.toString()); + } + @Override public void checkCanShowTablesMetadata(TransactionId transactionId, Identity identity, AccessControlContext context, CatalogSchemaName schema) { diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/security/SystemAccessControl.java b/presto-spi/src/main/java/com/facebook/presto/spi/security/SystemAccessControl.java index d51b517631382..7d209ce6b392d 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/security/SystemAccessControl.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/security/SystemAccessControl.java @@ -45,6 +45,7 @@ import static com.facebook.presto.spi.security.AccessDeniedException.denyRevokeTablePrivilege; import static com.facebook.presto.spi.security.AccessDeniedException.denySelectColumns; import static com.facebook.presto.spi.security.AccessDeniedException.denySetCatalogSessionProperty; +import static com.facebook.presto.spi.security.AccessDeniedException.denySetTableProperties; import static com.facebook.presto.spi.security.AccessDeniedException.denyShowSchemas; import static com.facebook.presto.spi.security.AccessDeniedException.denyShowTablesMetadata; import static com.facebook.presto.spi.security.AccessDeniedException.denyUpdateTableColumns; @@ -157,6 +158,16 @@ default void checkCanCreateTable(Identity identity, AccessControlContext context denyCreateTable(table.toString()); } + /** + * Check if identity is allowed to alter properties to the specified table in a catalog. + * + * @throws AccessDeniedException if not allowed + */ + default void checkCanSetTableProperties(Identity identity, AccessControlContext context, CatalogSchemaTableName table) + { + denySetTableProperties(table.toString()); + } + /** * Check if identity is allowed to drop the specified table in a catalog. *