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 a5df968b4c8b2..486afdc32aed5 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 @@ -52,6 +52,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; @@ -136,6 +137,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-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcClient.java b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcClient.java index 722841fe05c3e..512ba32dc4976 100644 --- a/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcClient.java +++ b/presto-base-jdbc/src/main/java/com/facebook/presto/plugin/jdbc/JdbcClient.java @@ -19,6 +19,7 @@ import com.facebook.presto.spi.ConnectorSession; import com.facebook.presto.spi.ConnectorSplitSource; import com.facebook.presto.spi.ConnectorTableMetadata; +import com.facebook.presto.spi.PrestoException; import com.facebook.presto.spi.SchemaTableName; import com.facebook.presto.spi.statistics.TableStatistics; @@ -28,9 +29,12 @@ import java.sql.PreparedStatement; import java.sql.SQLException; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; +import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED; + public interface JdbcClient { default boolean schemaExists(ConnectorSession session, JdbcIdentity identity, String schema) @@ -73,6 +77,11 @@ PreparedStatement buildSql(ConnectorSession session, Connection connection, Jdbc void renameTable(ConnectorSession session, JdbcIdentity identity, JdbcTableHandle handle, SchemaTableName newTableName); + default void setTableProperties(ConnectorSession session, JdbcTableHandle tableHandle, Map properties) + { + throw new PrestoException(NOT_SUPPORTED, "This connector does not support setting table properties"); + } + void createTable(ConnectorSession session, ConnectorTableMetadata tableMetadata); JdbcOutputTableHandle beginCreateTable(ConnectorSession session, ConnectorTableMetadata tableMetadata); diff --git a/presto-clickhouse/src/main/java/com/facebook/presto/plugin/clickhouse/ClickHouseClient.java b/presto-clickhouse/src/main/java/com/facebook/presto/plugin/clickhouse/ClickHouseClient.java index fce660719beff..41a760bc6d12c 100755 --- a/presto-clickhouse/src/main/java/com/facebook/presto/plugin/clickhouse/ClickHouseClient.java +++ b/presto-clickhouse/src/main/java/com/facebook/presto/plugin/clickhouse/ClickHouseClient.java @@ -71,6 +71,7 @@ import static com.facebook.presto.plugin.clickhouse.ClickHouseEngineType.MERGETREE; import static com.facebook.presto.plugin.clickhouse.ClickHouseErrorCode.JDBC_ERROR; import static com.facebook.presto.plugin.clickhouse.ClickhouseDXLKeyWords.ORDER_BY_PROPERTY; +import static com.facebook.presto.plugin.clickhouse.ClickhouseDXLKeyWords.SAMPLE_BY_PROPERTY; import static com.facebook.presto.plugin.clickhouse.StandardReadMappings.jdbcTypeToPrestoType; import static com.facebook.presto.spi.StandardErrorCode.INVALID_TABLE_PROPERTY; import static com.facebook.presto.spi.StandardErrorCode.NOT_FOUND; @@ -781,6 +782,24 @@ protected void renameTable(ClickHouseIdentity identity, String catalogName, Sche } } + public void setTableProperties(ClickHouseIdentity identity, ClickHouseTableHandle handle, Map properties) + { + checkArgument(properties.size() == 1 && properties.containsKey(SAMPLE_BY_PROPERTY), "Only support setting `sample_by` property"); + ImmutableList.Builder tableOptions = ImmutableList.builder(); + ClickHouseTableProperties.getSampleBy(properties).ifPresent(value -> tableOptions.add("SAMPLE BY " + value)); + + try (Connection connection = connectionFactory.openConnection(identity)) { + String sql = format( + "ALTER TABLE %s MODIFY %s", + quoted(handle.getCatalogName(), handle.getSchemaName(), handle.getTableName()), + join(" ", tableOptions.build())); + execute(connection, sql); + } + catch (SQLException e) { + throw new PrestoException(JDBC_ERROR, e); + } + } + private String getColumnDefinitionSql(ColumnMetadata column, String columnName) { StringBuilder builder = new StringBuilder() diff --git a/presto-clickhouse/src/main/java/com/facebook/presto/plugin/clickhouse/ClickHouseMetadata.java b/presto-clickhouse/src/main/java/com/facebook/presto/plugin/clickhouse/ClickHouseMetadata.java index 07b2181812186..48572d4155812 100755 --- a/presto-clickhouse/src/main/java/com/facebook/presto/plugin/clickhouse/ClickHouseMetadata.java +++ b/presto-clickhouse/src/main/java/com/facebook/presto/plugin/clickhouse/ClickHouseMetadata.java @@ -253,6 +253,12 @@ public void renameTable(ConnectorSession session, ConnectorTableHandle table, Sc clickHouseClient.renameTable(ClickHouseIdentity.from(session), tableHandle, newTableName); } + public void setTableProperties(ConnectorSession session, ConnectorTableHandle table, Map properties) + { + ClickHouseTableHandle tableHandle = (ClickHouseTableHandle) table; + clickHouseClient.setTableProperties(ClickHouseIdentity.from(session), tableHandle, properties); + } + @Override public TableStatistics getTableStatistics(ConnectorSession session, ConnectorTableHandle tableHandle, Optional tableLayoutHandle, List columnHandles, Constraint constraint) { diff --git a/presto-docs/src/main/sphinx/connector/iceberg.rst b/presto-docs/src/main/sphinx/connector/iceberg.rst index 90e2c28eb466f..1ffbd24c8bb74 100644 --- a/presto-docs/src/main/sphinx/connector/iceberg.rst +++ b/presto-docs/src/main/sphinx/connector/iceberg.rst @@ -251,6 +251,10 @@ Property Name Description ``format_version`` Optionally specifies the format version of the Iceberg specification to use for new tables, either ``1`` or ``2``. Defaults to ``1``. + +``commit_retries`` Determines the number of attempts for committing the metadata + in case of concurrent upsert requests, before failing. The + default value is 4. ========================================= =============================================================== The table definition below specifies format ``ORC``, partitioning by columns ``c1`` and ``c2``, @@ -497,7 +501,7 @@ that is stored using the ORC file format, partitioned by ``ds`` and partitioning = ARRAY['ds', 'country'] ) -Create an Iceberg table with Iceberg format version 2:: +Create an Iceberg table with Iceberg format version 2 and with commit_retries set to 5:: CREATE TABLE iceberg.web.page_views_v2 ( view_time timestamp, @@ -509,7 +513,8 @@ Create an Iceberg table with Iceberg format version 2:: WITH ( format = 'ORC', partitioning = ARRAY['ds', 'country'], - format_version = '2' + format_version = '2', + commit_retries = 5 ) Partition Column Transform @@ -644,6 +649,11 @@ The table is partitioned by the transformed value of the column:: ALTER TABLE iceberg.web.page_views ADD COLUMN ts timestamp WITH (partitioning = 'hour'); +Table properties can be modified for an iceberg table using ALTER TABLE SET PROPERTIES statement. Only `commit_retries` can be modified at present. +For example, to set commit_retries to 6 for table `iceberg.web.page_views_v2`, use the below statement:: + + ALTER TABLE iceberg.web.page_views_v2 SET PROPERTIES (commit_retries = 6); + TRUNCATE ^^^^^^^^ diff --git a/presto-docs/src/main/sphinx/sql/alter-table.rst b/presto-docs/src/main/sphinx/sql/alter-table.rst index 1686300bacf87..47b2cb57538fc 100644 --- a/presto-docs/src/main/sphinx/sql/alter-table.rst +++ b/presto-docs/src/main/sphinx/sql/alter-table.rst @@ -58,6 +58,10 @@ Rename column ``id`` to ``user_id`` in the ``users`` table if table ``users`` an ALTER TABLE IF EXISTS users RENAME column IF EXISTS id to user_id; +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 0c5a3afcde723..11d66488afabc 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; @@ -95,6 +96,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 94e9eb12652ea..006b1266656fb 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; @@ -72,6 +73,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; @@ -148,6 +150,15 @@ 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.getSource(), Optional.empty(), false, HiveColumnConverterProvider.DEFAULT_COLUMN_CONVERTER_PROVIDER); + 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-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java index f0f4d12a4d035..000abd29838e0 100644 --- a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java +++ b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java @@ -73,8 +73,10 @@ import org.apache.iceberg.Schema; import org.apache.iceberg.SchemaParser; import org.apache.iceberg.Table; +import org.apache.iceberg.TableProperties; import org.apache.iceberg.TableScan; import org.apache.iceberg.Transaction; +import org.apache.iceberg.UpdateProperties; import org.apache.iceberg.expressions.Expression; import org.apache.iceberg.io.CloseableIterable; import org.apache.iceberg.types.Type; @@ -105,6 +107,7 @@ import static com.facebook.presto.iceberg.IcebergColumnHandle.primitiveIcebergColumnHandle; import static com.facebook.presto.iceberg.IcebergErrorCode.ICEBERG_INVALID_SNAPSHOT_ID; import static com.facebook.presto.iceberg.IcebergSessionProperties.isPushdownFilterEnabled; +import static com.facebook.presto.iceberg.IcebergTableProperties.CONCURRENCY_RETRIES; import static com.facebook.presto.iceberg.IcebergTableProperties.FILE_FORMAT_PROPERTY; import static com.facebook.presto.iceberg.IcebergTableProperties.FORMAT_VERSION; import static com.facebook.presto.iceberg.IcebergTableProperties.LOCATION_PROPERTY; @@ -791,6 +794,28 @@ public OptionalLong metadataDelete(ConnectorSession session, ConnectorTableHandl } } + @Override + public void setTableProperties(ConnectorSession session, ConnectorTableHandle tableHandle, Map properties) + { + IcebergTableHandle handle = (IcebergTableHandle) tableHandle; + Table icebergTable = getIcebergTable(session, handle.getSchemaTableName()); + transaction = icebergTable.newTransaction(); + + UpdateProperties updateProperties = transaction.updateProperties(); + for (Map.Entry entry : properties.entrySet()) { + switch (entry.getKey()) { + case CONCURRENCY_RETRIES: + updateProperties.set(TableProperties.COMMIT_NUM_RETRIES, String.valueOf(entry.getValue())); + break; + default: + throw new PrestoException(NOT_SUPPORTED, "Updating property " + entry.getKey() + " is not supported currently"); + } + } + + updateProperties.commit(); + transaction.commitTransaction(); + } + /** * Deletes all the files within a particular scan * diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergTableProperties.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergTableProperties.java index 66dd97c1db834..81abf123fb080 100644 --- a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergTableProperties.java +++ b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergTableProperties.java @@ -17,6 +17,7 @@ import com.facebook.presto.spi.session.PropertyMetadata; import com.google.common.collect.ImmutableList; import org.apache.iceberg.FileFormat; +import org.apache.iceberg.TableProperties; import javax.inject.Inject; @@ -26,6 +27,7 @@ import static com.facebook.presto.common.type.VarcharType.VARCHAR; import static com.facebook.presto.common.type.VarcharType.createUnboundedVarcharType; +import static com.facebook.presto.spi.session.PropertyMetadata.integerProperty; import static com.facebook.presto.spi.session.PropertyMetadata.stringProperty; import static com.google.common.collect.ImmutableList.toImmutableList; import static java.util.Locale.ENGLISH; @@ -36,6 +38,7 @@ public class IcebergTableProperties public static final String PARTITIONING_PROPERTY = "partitioning"; public static final String LOCATION_PROPERTY = "location"; public static final String FORMAT_VERSION = "format_version"; + public static final String CONCURRENCY_RETRIES = "commit_retries"; private final List> tableProperties; private final List> columnProperties; @@ -74,6 +77,11 @@ public IcebergTableProperties(IcebergConfig icebergConfig) "Format version for the table", null, false)) + .add(integerProperty( + CONCURRENCY_RETRIES, + "Determines the number of attempts in case of concurrent upserts", + TableProperties.COMMIT_NUM_RETRIES_DEFAULT, + false)) .build(); columnProperties = ImmutableList.of(stringProperty( @@ -114,4 +122,9 @@ public static String getFormatVersion(Map tableProperties) { return (String) tableProperties.get(FORMAT_VERSION); } + + public static Integer getConcurrencyRetries(Map tableProperties) + { + return (Integer) tableProperties.get(CONCURRENCY_RETRIES); + } } diff --git a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergDistributedSmokeTestBase.java b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergDistributedSmokeTestBase.java index e6cb35b6a742b..79e926dd6d810 100644 --- a/presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergDistributedSmokeTestBase.java +++ b/presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergDistributedSmokeTestBase.java @@ -17,6 +17,7 @@ import com.facebook.presto.metadata.CatalogManager; import com.facebook.presto.spi.ConnectorId; import com.facebook.presto.spi.ConnectorSession; +import com.facebook.presto.spi.PrestoException; import com.facebook.presto.testing.MaterializedResult; import com.facebook.presto.testing.QueryRunner; import com.facebook.presto.testing.assertions.Assert; @@ -45,6 +46,7 @@ import static java.util.Locale.ENGLISH; import static java.util.Objects.requireNonNull; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; @@ -1121,4 +1123,38 @@ public void testTimestampPartitionedByHour() dropTable(session, tableName); } + + @Test + public void testUpdatingInvalidProperty() + { + Session session = getSession(); + String tableName = "test_invalid_property_update"; + assertUpdate(session, "CREATE TABLE " + tableName + " (c1 integer, c2 varchar) WITH(commit_retries = 4)"); + assertThatThrownBy(() -> assertUpdate("ALTER TABLE " + tableName + " SET PROPERTIES (format = 'PARQUET')")) + .hasMessage("Updating property format is not supported currently"); + assertUpdate("DROP TABLE " + tableName); + } + + @Test + public void testUpdatingRandomProperty() + { + Session session = getSession(); + String tableName = "test_random_property_update"; + assertUpdate(session, "CREATE TABLE " + tableName + " (c1 integer, c2 varchar) WITH(commit_retries = 4)"); + assertThatThrownBy(() -> assertUpdate("ALTER TABLE " + tableName + " SET PROPERTIES (some_config = 2)")) + .hasMessage("Catalog 'iceberg' does not support table property 'some_config'"); + assertUpdate("DROP TABLE " + tableName); + } + + @Test + public void testUpdatingCommitRetries() + { + Session session = getSession(); + String tableName = "test_commit_retries_update"; + assertUpdate(session, "CREATE TABLE " + tableName + " (c1 integer, c2 varchar) WITH(commit_retries = 4)"); + assertQuery("SELECT value FROM \"" + tableName + "$properties\" WHERE key = 'commit.retry.num-retries'", "VALUES 4"); + assertUpdate("ALTER TABLE " + tableName + " SET PROPERTIES (commit_retries = 5)"); + assertQuery("SELECT value FROM \"" + tableName + "$properties\" WHERE key = 'commit.retry.num-retries'", "VALUES 5"); + assertUpdate("DROP TABLE " + 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..c5590128e5e74 --- /dev/null +++ b/presto-main/src/main/java/com/facebook/presto/execution/SetPropertiesTask.java @@ -0,0 +1,89 @@ +/* + * 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.NodeUtils; +import com.facebook.presto.sql.analyzer.utils.ParameterUtils; +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.spi.StandardErrorCode.NOT_FOUND; +import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED; +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.getName()); + Map sqlProperties = NodeUtils.mapFromProperties(statement.getProperties()); + if (statement.getType() == SetProperties.Type.TABLE) { + Map properties = metadata.getTablePropertyManager().getProperties( + null, + tableName.getCatalogName(), + sqlProperties, + session, + metadata, + ParameterUtils.parameterExtractor(statement, parameters)); + setTableProperties(tableName, metadata, accessControl, session, properties); + } + else { + throw new PrestoException(NOT_SUPPORTED, format("Unsupported target type: %s", statement.getType())); + } + + return immediateFuture(null); + } + + private void setTableProperties(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()) { + throw new PrestoException(NOT_FOUND, format("Table does not exist: %s", tableName)); + } + + 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/DelegatingMetadataManager.java b/presto-main/src/main/java/com/facebook/presto/metadata/DelegatingMetadataManager.java index 74de592712fdc..85d7f0e64d0e8 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 @@ -237,6 +237,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 3d29324f55c05..ab77580982928 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 @@ -210,6 +210,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 e9ceb161e7ff1..9560766fe24ea 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 @@ -664,6 +664,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/security/AccessControlManager.java b/presto-main/src/main/java/com/facebook/presto/security/AccessControlManager.java index aed99a82ff809..b759e2acb2334 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 d230904934472..71d18485662ce 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 03a51ce68126b..db49d2ccd16e9 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 @@ -64,6 +64,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; @@ -278,6 +279,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/sql/analyzer/StatementAnalyzer.java b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java index bf56caa10df19..543897470fd10 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java @@ -141,6 +141,7 @@ import com.facebook.presto.sql.tree.Select; import com.facebook.presto.sql.tree.SelectItem; import com.facebook.presto.sql.tree.SetOperation; +import com.facebook.presto.sql.tree.SetProperties; import com.facebook.presto.sql.tree.SetSession; import com.facebook.presto.sql.tree.SimpleGroupBy; import com.facebook.presto.sql.tree.SingleColumn; @@ -962,6 +963,12 @@ protected Scope visitRenameTable(RenameTable node, Optional scope) return createAndAssignScope(node, scope); } + @Override + protected Scope visitSetProperties(SetProperties node, Optional scope) + { + return createAndAssignScope(node, scope); + } + @Override protected Scope visitRenameColumn(RenameColumn node, Optional scope) { 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 1a143f6535b9f..07c49010dfc9a 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 @@ -72,6 +72,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; @@ -204,6 +205,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; @@ -562,6 +564,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 2500a2be42351..38ca34c0f2142 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; @@ -50,6 +51,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; @@ -69,6 +71,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; @@ -185,6 +188,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) { @@ -345,7 +360,7 @@ public enum TestingPrivilegeType CREATE_SCHEMA, DROP_SCHEMA, RENAME_SCHEMA, CREATE_TABLE, DROP_TABLE, RENAME_TABLE, INSERT_TABLE, DELETE_TABLE, TRUNCATE_TABLE, UPDATE_TABLE, ADD_COLUMN, DROP_COLUMN, RENAME_COLUMN, SELECT_COLUMN, - 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/test/java/com/facebook/presto/connector/MockConnectorFactory.java b/presto-main/src/test/java/com/facebook/presto/connector/MockConnectorFactory.java index 31be45c41aa32..c9059448a2861 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 e63e15bd70141..2b45248146a0e 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 @@ -282,6 +282,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 c7640e7c91c94..500f167408a9b 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 @@ -339,6 +339,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); }); @@ -382,6 +383,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 42885cf9903df..d55264d752e3e 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 @@ -58,6 +58,8 @@ statement DROP COLUMN (IF EXISTS)? column=qualifiedName #dropColumn | ALTER TABLE (IF EXISTS)? tableName=qualifiedName ADD COLUMN (IF NOT EXISTS)? column=columnDefinition #addColumn + | 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 f7ecbbfe7a268..cbecbc1bbe45c 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 @@ -88,6 +88,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; @@ -810,6 +811,23 @@ protected Void visitShowTables(ShowTables node, Integer context) return null; } + protected Void visitSetProperties(SetProperties node, Integer context) + { + builder.append("ALTER TABLE "); + builder.append(node.getName()).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 a66e7d2024c64..31336ee57d37b 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 @@ -137,6 +137,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; @@ -447,6 +448,17 @@ 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), SetProperties.Type.TABLE, getQualifiedName(context.qualifiedName()), properties); + } + @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 17d8552d1464d..81ef605afb9dd 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..791151d862337 --- /dev/null +++ b/presto-parser/src/main/java/com/facebook/presto/sql/tree/SetProperties.java @@ -0,0 +1,111 @@ +/* + * 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 name; + private final List properties; + + public SetProperties(Type type, QualifiedName name, List properties) + { + this(Optional.empty(), type, name, properties); + } + + public SetProperties(NodeLocation location, Type type, QualifiedName name, List properties) + { + this(Optional.of(location), type, name, properties); + } + + private SetProperties(Optional location, Type type, QualifiedName name, List properties) + { + super(location); + this.type = requireNonNull(type, "type is null"); + this.name = requireNonNull(name, "name is null"); + this.properties = ImmutableList.copyOf(requireNonNull(properties, "properties is null")); + } + + public Type getType() + { + return type; + } + + public QualifiedName getName() + { + return name; + } + + 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, name, properties); + } + + @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(name, o.name) && + Objects.equals(properties, o.properties); + } + + @Override + public String toString() + { + return toStringHelper(this) + .add("type", type) + .add("name", name) + .add("properties", properties) + .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 dc26c8cb7c3ac..12151b9de91ec 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 @@ -116,6 +116,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; @@ -1612,6 +1613,18 @@ 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"))))); + 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"))))); + 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"))))); + 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"))))); + 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"))))); + + assertInvalidStatement("ALTER TABLE a 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 ade7f5a17bf09..de1c55ab2688f 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 @@ -185,6 +185,9 @@ 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 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 48e2f02937b9f..2310061391658 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 11a1a38f22861..bf2a016ec3a2c 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; @@ -56,6 +57,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; @@ -115,6 +117,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 6610cb92bc273..decaacea38568 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 e04b8efe93b37..34309d99c1971 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 0478e501e0430..ed8f75acbf588 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 @@ -21,6 +21,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; @@ -66,8 +67,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 b3777704a6a2a..8e28bdebba530 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; @@ -46,6 +47,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; @@ -119,6 +121,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 7cc33b02a5803..0d2d195036466 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 @@ -349,6 +349,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 26e695b74581d..7a0caf7b6dc09 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 @@ -414,6 +414,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 3907615005bb7..759b7f9ed1c15 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 e0818932c9f6a..f892b98a50a10 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 e06259d449e03..02ac32f5bbff1 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 0c44d88562b16..60c386c1063f5 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; @@ -50,6 +51,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; @@ -125,6 +127,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 418ac8bc47139..b3069c9635181 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 @@ -43,6 +43,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; @@ -155,6 +156,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. *