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 dfeb0b7899e37..f91b751043d06 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 @@ -62,6 +62,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.SetColumnType; import com.facebook.presto.sql.tree.SetProperties; import com.facebook.presto.sql.tree.SetRole; import com.facebook.presto.sql.tree.SetSession; @@ -172,6 +173,7 @@ private StatementUtils() {} builder.put(Revoke.class, QueryType.DATA_DEFINITION); builder.put(Prepare.class, QueryType.CONTROL); builder.put(Deallocate.class, QueryType.CONTROL); + builder.put(SetColumnType.class, QueryType.DATA_DEFINITION); STATEMENT_QUERY_TYPES = builder.build(); } diff --git a/presto-blackhole/src/main/java/com/facebook/presto/plugin/blackhole/BlackHoleMetadata.java b/presto-blackhole/src/main/java/com/facebook/presto/plugin/blackhole/BlackHoleMetadata.java index afe0f1efd8dc4..0a95f7598fdf6 100644 --- a/presto-blackhole/src/main/java/com/facebook/presto/plugin/blackhole/BlackHoleMetadata.java +++ b/presto-blackhole/src/main/java/com/facebook/presto/plugin/blackhole/BlackHoleMetadata.java @@ -14,6 +14,7 @@ package com.facebook.presto.plugin.blackhole; import com.facebook.airlift.units.Duration; +import com.facebook.presto.common.type.Type; import com.facebook.presto.spi.ColumnHandle; import com.facebook.presto.spi.ColumnMetadata; import com.facebook.presto.spi.ConnectorInsertTableHandle; @@ -286,4 +287,23 @@ private void checkSchemaExists(String schemaName) throw new SchemaNotFoundException(schemaName); } } + + @Override + public void setColumnType(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnHandle columnHandle, Type type) + { + BlackHoleTableHandle table = (BlackHoleTableHandle) tableHandle; + BlackHoleColumnHandle column = (BlackHoleColumnHandle) columnHandle; + List columns = new ArrayList<>(table.getColumnHandles()); + columns.set(columns.indexOf(column), new BlackHoleColumnHandle(column.getName(), type)); + + tables.put(table.toSchemaTableName(), new BlackHoleTableHandle( + table.getSchemaName(), + table.getTableName(), + ImmutableList.copyOf(columns), + table.getSplitCount(), + table.getPagesPerSplit(), + table.getRowsPerPage(), + table.getFieldsLength(), + table.getPageProcessingDelay())); + } } diff --git a/presto-common/src/main/java/com/facebook/presto/common/type/DoubleType.java b/presto-common/src/main/java/com/facebook/presto/common/type/DoubleType.java index 877d14b916bba..f2456e7263d9d 100644 --- a/presto-common/src/main/java/com/facebook/presto/common/type/DoubleType.java +++ b/presto-common/src/main/java/com/facebook/presto/common/type/DoubleType.java @@ -16,6 +16,7 @@ import com.facebook.presto.common.block.Block; import com.facebook.presto.common.block.BlockBuilder; import com.facebook.presto.common.block.BlockBuilderStatus; +import com.facebook.presto.common.block.IntArrayBlock; import com.facebook.presto.common.block.LongArrayBlockBuilder; import com.facebook.presto.common.block.PageBuilderStatus; import com.facebook.presto.common.block.UncheckedBlock; @@ -27,6 +28,7 @@ import static com.facebook.presto.common.type.TypeUtils.doubleHashCode; import static java.lang.Double.doubleToLongBits; import static java.lang.Double.longBitsToDouble; +import static java.lang.Float.intBitsToFloat; public final class DoubleType extends AbstractPrimitiveType @@ -67,6 +69,9 @@ public Object getObjectValue(SqlFunctionProperties properties, Block block, int if (block.isNull(position)) { return null; } + if (IntArrayBlock.class.isInstance(block)) { + return intBitsToFloat(block.getInt(position)); + } return longBitsToDouble(block.getLong(position)); } @@ -109,13 +114,22 @@ public void appendTo(Block block, int position, BlockBuilder blockBuilder) blockBuilder.appendNull(); } else { - blockBuilder.writeLong(block.getLong(position)).closeEntry(); + if (block instanceof IntArrayBlock) { + float flt = intBitsToFloat(block.getInt(position)); + blockBuilder.writeLong(doubleToLongBits(flt)).closeEntry(); + } + else { + blockBuilder.writeLong(block.getLong(position)).closeEntry(); + } } } @Override public double getDouble(Block block, int position) { + if (block instanceof IntArrayBlock) { + return intBitsToFloat(block.getInt(position)); + } return longBitsToDouble(block.getLong(position)); } diff --git a/presto-common/src/main/java/com/facebook/presto/common/type/LongDecimalType.java b/presto-common/src/main/java/com/facebook/presto/common/type/LongDecimalType.java index 2852cfc3d20fd..15899d6f81f87 100644 --- a/presto-common/src/main/java/com/facebook/presto/common/type/LongDecimalType.java +++ b/presto-common/src/main/java/com/facebook/presto/common/type/LongDecimalType.java @@ -17,19 +17,23 @@ import com.facebook.presto.common.block.BlockBuilder; import com.facebook.presto.common.block.BlockBuilderStatus; import com.facebook.presto.common.block.Int128ArrayBlockBuilder; +import com.facebook.presto.common.block.LongArrayBlock; import com.facebook.presto.common.block.PageBuilderStatus; import com.facebook.presto.common.function.SqlFunctionProperties; import io.airlift.slice.Slice; import io.airlift.slice.Slices; +import java.math.BigInteger; + import static com.facebook.presto.common.block.Int128ArrayBlock.INT128_BYTES; import static com.facebook.presto.common.type.Decimals.MAX_PRECISION; import static com.facebook.presto.common.type.Decimals.decodeUnscaledValue; +import static com.facebook.presto.common.type.Decimals.encodeUnscaledValue; import static com.facebook.presto.common.type.UnscaledDecimal128Arithmetic.UNSCALED_DECIMAL_128_SLICE_LENGTH; import static com.facebook.presto.common.type.UnscaledDecimal128Arithmetic.compare; import static io.airlift.slice.SizeOf.SIZE_OF_LONG; -final class LongDecimalType +public final class LongDecimalType extends DecimalType { LongDecimalType(int precision, int scale) @@ -77,6 +81,10 @@ public Object getObjectValue(SqlFunctionProperties properties, Block block, int if (block.isNull(position)) { return null; } + if (block instanceof LongArrayBlock) { + long unscaledValue = block.getLong(position); + return new SqlDecimal(BigInteger.valueOf(unscaledValue), getPrecision(), getScale()); + } Slice slice = getSlice(block, position); return new SqlDecimal(decodeUnscaledValue(slice), getPrecision(), getScale()); } @@ -112,9 +120,15 @@ public void appendTo(Block block, int position, BlockBuilder blockBuilder) blockBuilder.appendNull(); } else { - blockBuilder.writeLong(block.getLong(position, 0)); - blockBuilder.writeLong(block.getLong(position, SIZE_OF_LONG)); - blockBuilder.closeEntry(); + if (block instanceof LongArrayBlock) { + Slice slice = encodeUnscaledValue(block.getLong(position)); + writeSlice(blockBuilder, slice); + } + else { + blockBuilder.writeLong(block.getLong(position, 0)); + blockBuilder.writeLong(block.getLong(position, SIZE_OF_LONG)); + blockBuilder.closeEntry(); + } } } @@ -138,6 +152,9 @@ public void writeSlice(BlockBuilder blockBuilder, Slice value, int offset, int l @Override public Slice getSlice(Block block, int position) { + if (block instanceof LongArrayBlock) { + return encodeUnscaledValue(block.getLong(position)); + } return Slices.wrappedLongArray( block.getLong(position, 0), block.getLong(position, SIZE_OF_LONG)); diff --git a/presto-docs/src/main/sphinx/sql/alter-table.rst b/presto-docs/src/main/sphinx/sql/alter-table.rst index d3c270e73fbdf..fbb6bd2cb3497 100644 --- a/presto-docs/src/main/sphinx/sql/alter-table.rst +++ b/presto-docs/src/main/sphinx/sql/alter-table.rst @@ -17,6 +17,7 @@ Synopsis ALTER TABLE [ IF EXISTS ] name SET PROPERTIES (property_name=value, [, ...]) ALTER TABLE [ IF EXISTS ] name DROP BRANCH [ IF EXISTS ] branch_name ALTER TABLE [ IF EXISTS ] name DROP TAG [ IF EXISTS ] tag_name + ALTER TABLE [ IF EXISTS ] name ALTER COLUMN column_name SET DATA TYPE new_type ALTER TABLE [ IF EXISTS ] name CREATE [ OR REPLACE ] BRANCH [ IF NOT EXISTS ] branch_name ALTER TABLE [ IF EXISTS ] name CREATE [ OR REPLACE ] BRANCH [ IF NOT EXISTS ] branch_name FOR SYSTEM_VERSION AS OF version ALTER TABLE [ IF EXISTS ] name CREATE [ OR REPLACE ] BRANCH [ IF NOT EXISTS ] branch_name FOR SYSTEM_TIME AS OF timestamp 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 d52fd54fc4e4a..2d6f541df3cc2 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 @@ -241,6 +241,9 @@ public void checkCanCreateViewWithSelectFromColumns(ConnectorTransactionHandle t { } + @Override + public void checkCanAlterColumn(ConnectorTransactionHandle transaction, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName) {} + @Override public void checkCanSetCatalogSessionProperty(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, String propertyName) { 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 3171f56dbe691..8531903cbc93b 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 @@ -56,6 +56,7 @@ import static com.facebook.presto.hive.metastore.thrift.ThriftMetastoreUtil.listEnabledTablePrivileges; import static com.facebook.presto.spi.security.AccessDeniedException.denyAddColumn; import static com.facebook.presto.spi.security.AccessDeniedException.denyAddConstraint; +import static com.facebook.presto.spi.security.AccessDeniedException.denyAlterColumn; import static com.facebook.presto.spi.security.AccessDeniedException.denyCallProcedure; import static com.facebook.presto.spi.security.AccessDeniedException.denyCreateBranch; import static com.facebook.presto.spi.security.AccessDeniedException.denyCreateRole; @@ -405,6 +406,24 @@ public void checkCanUpdateTableColumns(ConnectorTransactionHandle transaction, C } } + @Override + public void checkCanAlterColumn(ConnectorTransactionHandle transaction, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName) + { + 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(transaction, identity, metastoreContext, tableName)) { + denyAlterColumn(tableName.toString()); + } + } + @Override public void checkCanCreateView(ConnectorTransactionHandle transaction, ConnectorIdentity identity, AccessControlContext context, SchemaTableName viewName) { 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 a495f8e615bc2..d81b01fdea0fb 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 @@ -180,6 +180,7 @@ import static com.facebook.presto.iceberg.IcebergColumnHandle.ROW_ID_COLUMN_HANDLE; import static com.facebook.presto.iceberg.IcebergColumnHandle.ROW_ID_COLUMN_METADATA; import static com.facebook.presto.iceberg.IcebergErrorCode.ICEBERG_COMMIT_ERROR; +import static com.facebook.presto.iceberg.IcebergErrorCode.ICEBERG_INCOMPATIBLE_COLUMN_TYPE; import static com.facebook.presto.iceberg.IcebergErrorCode.ICEBERG_INVALID_FORMAT_VERSION; import static com.facebook.presto.iceberg.IcebergErrorCode.ICEBERG_INVALID_MATERIALIZED_VIEW; import static com.facebook.presto.iceberg.IcebergErrorCode.ICEBERG_INVALID_SNAPSHOT_ID; @@ -262,6 +263,7 @@ import static com.facebook.presto.spi.connector.RowChangeParadigm.DELETE_ROW_AND_INSERT_ROW; import static com.facebook.presto.spi.statistics.TableStatisticType.ROW_COUNT; import static com.facebook.presto.spi.transaction.IsolationLevel.SERIALIZABLE; +import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.common.base.Verify.verify; import static com.google.common.collect.ImmutableList.toImmutableList; @@ -2446,6 +2448,23 @@ private boolean viewExists(ConnectorSession session, ConnectorTableMetadata view return getViewMetadata(session, viewMetadata.getTable()).isPresent(); } + @Override + public void setColumnType(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnHandle columnHandle, com.facebook.presto.common.type.Type type) + { + IcebergTableHandle table = (IcebergTableHandle) tableHandle; + IcebergColumnHandle column = (IcebergColumnHandle) columnHandle; + + Table icebergTable = getIcebergTable(session, table.getSchemaTableName()); + try { + icebergTable.updateSchema() + .updateColumn(column.getName(), toIcebergType(type).asPrimitiveType()) + .commit(); + } + catch (RuntimeException e) { + throw new PrestoException(ICEBERG_INCOMPATIBLE_COLUMN_TYPE, "Failed to set column type: " + firstNonNull(e.getMessage(), e), e); + } + } + protected void openCreateTableTransaction(SchemaTableName tableName, Transaction transaction) { transactionContext.registerTransaction(tableName, transaction); diff --git a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergErrorCode.java b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergErrorCode.java index 7575b318fc98f..7d980af175f67 100644 --- a/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergErrorCode.java +++ b/presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergErrorCode.java @@ -43,6 +43,7 @@ public enum IcebergErrorCode ICEBERG_INVALID_MATERIALIZED_VIEW(18, EXTERNAL), ICEBERG_INVALID_SPEC_ID(19, EXTERNAL), ICEBERG_TRANSACTION_CONFLICT_ERROR(20, EXTERNAL), + ICEBERG_INCOMPATIBLE_COLUMN_TYPE(21, USER_ERROR), /**/; private final ErrorCode errorCode; 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 c497520e70d2e..e8d4eff4347c1 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 @@ -45,6 +45,7 @@ import org.apache.hadoop.fs.Path; import org.apache.iceberg.Table; import org.apache.iceberg.UpdateProperties; +import org.apache.parquet.column.ParquetProperties; import org.intellij.lang.annotations.Language; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -87,6 +88,8 @@ import static java.util.stream.Collectors.joining; import static java.util.stream.IntStream.range; import static org.apache.iceberg.util.LocationUtil.stripTrailingSlash; +import static org.apache.parquet.column.ParquetProperties.WriterVersion.PARQUET_1_0; +import static org.apache.parquet.column.ParquetProperties.WriterVersion.PARQUET_2_0; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; @@ -1094,6 +1097,371 @@ private void testBucketTransformsForFormat(Session session, FileFormat format) dropTable(session, "test_bucket_transform"); } + @DataProvider(name = "batchReadAndParquetVersions") + public Object[][] batchReadAndParquetVersions() + { + return new Object[][] { + {true, PARQUET_1_0}, + {false, PARQUET_1_0}, + {true, PARQUET_2_0}, + {false, PARQUET_2_0} + }; + } + + @Test(dataProvider = "batchReadAndParquetVersions") + public void testAlterColumnType(boolean batchRead, ParquetProperties.WriterVersion writerVersion) + { + testWithAllFileFormats((session, fileFormat) -> { + session = Session.builder(session) + .setCatalogSessionProperty("iceberg", "parquet_batch_read_optimization_enabled", String.valueOf(batchRead)) + .setCatalogSessionProperty("iceberg", "parquet_writer_version", writerVersion.toString()) + .build(); + String tableName = "test_alter_column_type_" + fileFormat.name().toLowerCase(ENGLISH); + String schemaName = session.getSchema().get(); + try { + assertUpdate(session, format( + "CREATE TABLE %s (" + + "int_col INTEGER, " + + "float_col REAL, " + + "decimal_col DECIMAL(10, 2), " + + "bigint_col BIGINT, " + + "decimal_short DECIMAL(10, 3), " + + "decimal_boundary DECIMAL(18, 5)" + + ") WITH (format = '%s')", + tableName, fileFormat)); + + assertUpdate(session, format( + "INSERT INTO %s VALUES " + + "(100, REAL '1.5', DECIMAL '123.45', 1000, DECIMAL '999.123', DECIMAL '123456789012.12345'), " + + "(200, REAL '2.5', DECIMAL '234.56', 2000, DECIMAL '888.456', DECIMAL '987654321098.54321'), " + + "(NULL, NULL, NULL, NULL, NULL, NULL)", + tableName), 3); + + assertQuery(session, format("SELECT * FROM %s ORDER BY int_col NULLS LAST", tableName), + "VALUES " + + "(100, CAST(1.5 AS REAL), CAST(123.45 AS DECIMAL(10,2)), CAST(1000 AS BIGINT), CAST(999.123 AS DECIMAL(10,3)), CAST(123456789012.12345 AS DECIMAL(18,5))), " + + "(200, CAST(2.5 AS REAL), CAST(234.56 AS DECIMAL(10,2)), CAST(2000 AS BIGINT), CAST(888.456 AS DECIMAL(10,3)), CAST(987654321098.54321 AS DECIMAL(18,5))), " + + "(NULL, NULL, NULL, NULL, NULL, NULL)"); + + // Test 1: INTEGER → BIGINT conversion + assertUpdate(session, format("ALTER TABLE %s ALTER COLUMN int_col SET DATA TYPE BIGINT", tableName)); + assertQuery(session, format("SELECT int_col, typeof(int_col) FROM %s ORDER BY int_col NULLS LAST", tableName), + "VALUES (CAST(100 AS BIGINT), 'bigint'), (CAST(200 AS BIGINT), 'bigint'), (NULL, 'bigint')"); + validateShowCreateTable(session.getCatalog().get(), schemaName, tableName, + ImmutableList.of( + columnDefinition("int_col", "bigint"), + columnDefinition("float_col", "real"), + columnDefinition("decimal_col", "decimal(10,2)"), + columnDefinition("bigint_col", "bigint"), + columnDefinition("decimal_short", "decimal(10,3)"), + columnDefinition("decimal_boundary", "decimal(18,5)")), + null, + null); + + // Test 2: REAL → DOUBLE conversion (with NULL handling) + assertUpdate(session, format("ALTER TABLE %s ALTER COLUMN float_col SET DATA TYPE DOUBLE", tableName)); + validateShowCreateTable(session.getCatalog().get(), schemaName, tableName, + ImmutableList.of( + columnDefinition("int_col", "bigint"), + columnDefinition("float_col", "double"), + columnDefinition("decimal_col", "decimal(10,2)"), + columnDefinition("bigint_col", "bigint"), + columnDefinition("decimal_short", "decimal(10,3)"), + columnDefinition("decimal_boundary", "decimal(18,5)")), + null, + null); + assertQuery(session, format("SELECT float_col, typeof(float_col) FROM %s ORDER BY int_col NULLS LAST", tableName), + "VALUES (CAST(1.5 AS DOUBLE), 'double'), (CAST(2.5 AS DOUBLE), 'double'), (NULL, 'double')"); + + // Test 3: DECIMAL(10,2) → DECIMAL(15,2) - ShortDecimal to ShortDecimal + assertUpdate(session, format("ALTER TABLE %s ALTER COLUMN decimal_col SET DATA TYPE DECIMAL(15, 2)", tableName)); + validateShowCreateTable(session.getCatalog().get(), schemaName, tableName, + ImmutableList.of( + columnDefinition("int_col", "bigint"), + columnDefinition("float_col", "double"), + columnDefinition("decimal_col", "decimal(15,2)"), + columnDefinition("bigint_col", "bigint"), + columnDefinition("decimal_short", "decimal(10,3)"), + columnDefinition("decimal_boundary", "decimal(18,5)")), + null, + null); + assertQuery(session, format("SELECT decimal_col, typeof(decimal_col) FROM %s ORDER BY int_col NULLS LAST", tableName), + "VALUES (CAST(123.45 AS DECIMAL(15,2)), 'decimal(15,2)'), (CAST(234.56 AS DECIMAL(15,2)), 'decimal(15,2)'), (NULL, 'decimal(15,2)')"); + + // Test 4: DECIMAL(15,2) → DECIMAL(24,2) - ShortDecimal to LongDecimal (crossing precision 18 boundary) + assertUpdate(session, format("ALTER TABLE %s ALTER COLUMN decimal_col SET DATA TYPE DECIMAL(24, 2)", tableName)); + validateShowCreateTable(session.getCatalog().get(), schemaName, tableName, + ImmutableList.of( + columnDefinition("int_col", "bigint"), + columnDefinition("float_col", "double"), + columnDefinition("decimal_col", "decimal(24,2)"), + columnDefinition("bigint_col", "bigint"), + columnDefinition("decimal_short", "decimal(10,3)"), + columnDefinition("decimal_boundary", "decimal(18,5)")), + null, + null); + assertQuery(session, format("SELECT decimal_col, typeof(decimal_col) FROM %s ORDER BY int_col NULLS LAST", tableName), + "VALUES (CAST(123.45 AS DECIMAL(24,2)), 'decimal(24,2)'), (CAST(234.56 AS DECIMAL(24,2)), 'decimal(24,2)'), (NULL, 'decimal(24,2)')"); + + // Test 5: DECIMAL(10,3) → DECIMAL(18,3) - ShortDecimal to ShortDecimal at boundary + assertUpdate(session, format("ALTER TABLE %s ALTER COLUMN decimal_short SET DATA TYPE DECIMAL(18, 3)", tableName)); + validateShowCreateTable(session.getCatalog().get(), schemaName, tableName, + ImmutableList.of( + columnDefinition("int_col", "bigint"), + columnDefinition("float_col", "double"), + columnDefinition("decimal_col", "decimal(24,2)"), + columnDefinition("bigint_col", "bigint"), + columnDefinition("decimal_short", "decimal(18,3)"), + columnDefinition("decimal_boundary", "decimal(18,5)")), + null, + null); + assertQuery(session, format("SELECT decimal_short, typeof(decimal_short) FROM %s ORDER BY int_col NULLS LAST", tableName), + "VALUES (CAST(999.123 AS DECIMAL(18,3)), 'decimal(18,3)'), (CAST(888.456 AS DECIMAL(18,3)), 'decimal(18,3)'), (NULL, 'decimal(18,3)')"); + + // Test 6: DECIMAL(18,5) → DECIMAL(19,5) - ShortDecimal to LongDecimal at precision 18→19 boundary + assertUpdate(session, format("ALTER TABLE %s ALTER COLUMN decimal_boundary SET DATA TYPE DECIMAL(19, 5)", tableName)); + validateShowCreateTable(session.getCatalog().get(), schemaName, tableName, + ImmutableList.of( + columnDefinition("int_col", "bigint"), + columnDefinition("float_col", "double"), + columnDefinition("decimal_col", "decimal(24,2)"), + columnDefinition("bigint_col", "bigint"), + columnDefinition("decimal_short", "decimal(18,3)"), + columnDefinition("decimal_boundary", "decimal(19,5)")), + null, + null); + assertQuery(session, format("SELECT decimal_boundary, typeof(decimal_boundary) FROM %s ORDER BY int_col NULLS LAST", tableName), + "VALUES (CAST(123456789012.12345 AS DECIMAL(19,5)), 'decimal(19,5)'), (CAST(987654321098.54321 AS DECIMAL(19,5)), 'decimal(19,5)'), (NULL, 'decimal(19,5)')"); + + // Test 7: Verify all columns together after all conversions + assertQuery(session, format("SELECT * FROM %s ORDER BY int_col NULLS LAST", tableName), + "VALUES " + + "(CAST(100 AS BIGINT), CAST(1.5 AS DOUBLE), CAST(123.45 AS DECIMAL(24,2)), CAST(1000 AS BIGINT), CAST(999.123 AS DECIMAL(18,3)), CAST(123456789012.12345 AS DECIMAL(19,5))), " + + "(CAST(200 AS BIGINT), CAST(2.5 AS DOUBLE), CAST(234.56 AS DECIMAL(24,2)), CAST(2000 AS BIGINT), CAST(888.456 AS DECIMAL(18,3)), CAST(987654321098.54321 AS DECIMAL(19,5))), " + + "(NULL, NULL, NULL, NULL, NULL, NULL)"); + + // Test 8: Verify unsupported conversion (BIGINT → INTEGER should fail) + assertQueryFails( + session, + format("ALTER TABLE %s ALTER COLUMN bigint_col SET DATA TYPE INTEGER", tableName), + "Failed to set column type: Cannot change column type: bigint_col: long -> int"); + } + finally { + dropTable(session, tableName); + } + }); + } + + @Test(dataProvider = "batchReadAndParquetVersions") + public void testAlterColumnTypeWithSpecialFloatValues(boolean batchRead, ParquetProperties.WriterVersion writerVersion) + { + testWithAllFileFormats((session, fileFormat) -> { + String tableName = "test_alter_special_float_" + fileFormat.name().toLowerCase(ENGLISH); + try { + session = Session.builder(session) + .setCatalogSessionProperty("iceberg", "parquet_batch_read_optimization_enabled", String.valueOf(batchRead)) + .setCatalogSessionProperty("iceberg", "parquet_writer_version", writerVersion.toString()) + .build(); + assertUpdate(session, format( + "CREATE TABLE %s (id INTEGER, float_col REAL) WITH (format = '%s')", + tableName, fileFormat)); + + // Test with special float values: positive, negative, zero, and very small/large values + assertUpdate(session, format( + "INSERT INTO %s VALUES " + + "(1, REAL '0.0'), " + + "(2, REAL '-0.0'), " + + "(3, REAL '3.4028235E38'), " + // Max float + "(4, REAL '-3.4028235E38'), " + // Min float + "(5, REAL '1.401298464324817E-45'), " + // Smallest positive float (actual representation) + "(6, REAL '0.1')", // Binary representation precision issue + tableName), 6); + + // Verify initial data + assertQuery(session, format("SELECT id, float_col FROM %s ORDER BY id", tableName), + "VALUES " + + "(1, CAST(0.0 AS REAL)), " + + "(2, CAST(-0.0 AS REAL)), " + + "(3, CAST(3.4028235E38 AS REAL)), " + + "(4, CAST(-3.4028235E38 AS REAL)), " + + "(5, CAST(1.401298464324817E-45 AS REAL)), " + + "(6, CAST(0.1 AS REAL))"); + + // Convert REAL to DOUBLE + assertUpdate(session, format("ALTER TABLE %s ALTER COLUMN float_col SET DATA TYPE DOUBLE", tableName)); + + // Verify data after conversion - values should be preserved + // Note: When REAL is converted to DOUBLE, the float representation is preserved + assertQuery(session, format("SELECT id, float_col FROM %s ORDER BY id", tableName), + "VALUES " + + "(1, CAST(0.0 AS DOUBLE)), " + + "(2, CAST(-0.0 AS DOUBLE)), " + + "(3, CAST(3.4028235E38 AS DOUBLE)), " + + "(4, CAST(-3.4028235E38 AS DOUBLE)), " + + "(5, CAST(1.401298464324817E-45 AS DOUBLE)), " + + "(6, CAST(0.1 AS DOUBLE))"); + } + finally { + dropTable(session, tableName); + } + }); + } + + @Test + public void testAlterColumnTypeWithMaxDecimalValues() + { + testWithAllFileFormats((session, fileFormat) -> { + String tableName = "test_alter_max_decimal_" + fileFormat.name().toLowerCase(ENGLISH); + try { + assertUpdate(session, format( + "CREATE TABLE %s (" + + "id INTEGER, " + + "dec_max_short DECIMAL(18, 0), " + + "dec_near_max DECIMAL(37, 10)" + + ") WITH (format = '%s')", + tableName, fileFormat)); + + // Test with maximum values for ShortDecimal and near-maximum for LongDecimal + assertUpdate(session, format( + "INSERT INTO %s VALUES " + + "(1, DECIMAL '999999999999999999', DECIMAL '9999999999999999999999999.9999999999'), " + + "(2, DECIMAL '-999999999999999999', DECIMAL '-9999999999999999999999999.9999999999'), " + + "(3, DECIMAL '0', DECIMAL '0.0000000001')", + tableName), 3); + + // Verify initial data + assertQuery(session, format("SELECT * FROM %s ORDER BY id", tableName), + "VALUES " + + "(1, CAST(999999999999999999 AS DECIMAL(18,0)), CAST(9999999999999999999999999.9999999999 AS DECIMAL(37,10))), " + + "(2, CAST(-999999999999999999 AS DECIMAL(18,0)), CAST(-9999999999999999999999999.9999999999 AS DECIMAL(37,10))), " + + "(3, CAST(0 AS DECIMAL(18,0)), CAST(0.0000000001 AS DECIMAL(37,10)))"); + + // Convert ShortDecimal(18,0) to LongDecimal(19,0) + assertUpdate(session, format("ALTER TABLE %s ALTER COLUMN dec_max_short SET DATA TYPE DECIMAL(19, 0)", tableName)); + + // Verify data preserved after crossing ShortDecimal/LongDecimal boundary + assertQuery(session, format("SELECT id, dec_max_short FROM %s ORDER BY id", tableName), + "VALUES " + + "(1, CAST(999999999999999999 AS DECIMAL(19,0))), " + + "(2, CAST(-999999999999999999 AS DECIMAL(19,0))), " + + "(3, CAST(0 AS DECIMAL(19,0)))"); + + // Increase LongDecimal precision + assertUpdate(session, format("ALTER TABLE %s ALTER COLUMN dec_near_max SET DATA TYPE DECIMAL(38, 10)", tableName)); + + // Verify data preserved + assertQuery(session, format("SELECT id, dec_near_max FROM %s ORDER BY id", tableName), + "VALUES " + + "(1, CAST(9999999999999999999999999.9999999999 AS DECIMAL(38,10))), " + + "(2, CAST(-9999999999999999999999999.9999999999 AS DECIMAL(38,10))), " + + "(3, CAST(0.0000000001 AS DECIMAL(38,10)))"); + } + finally { + dropTable(session, tableName); + } + }); + } + + @Test + public void testAlterColumnTypeUnsupportedConversions() + { + testWithAllFileFormats((session, fileFormat) -> { + String tableName = "test_alter_unsupported_" + fileFormat.name().toLowerCase(ENGLISH); + try { + assertUpdate(session, format( + "CREATE TABLE %s (" + + "bigint_col BIGINT, " + + "double_col DOUBLE, " + + "decimal_col DECIMAL(20, 5)" + + ") WITH (format = '%s')", + tableName, fileFormat)); + + assertUpdate(session, format( + "INSERT INTO %s VALUES (1000, 1.5, DECIMAL '123.45')", + tableName), 1); + + // Test unsupported: BIGINT → INTEGER (narrowing) + assertQueryFails( + session, + format("ALTER TABLE %s ALTER COLUMN bigint_col SET DATA TYPE INTEGER", tableName), + "Failed to set column type: Cannot change column type: bigint_col: long -> int"); + + // Test unsupported: DOUBLE → REAL (narrowing) + assertQueryFails( + session, + format("ALTER TABLE %s ALTER COLUMN double_col SET DATA TYPE REAL", tableName), + "Failed to set column type: Cannot change column type: double_col: double -> float"); + + // Test unsupported: Decimal scale change + assertQueryFails( + session, + format("ALTER TABLE %s ALTER COLUMN decimal_col SET DATA TYPE DECIMAL(20, 3)", tableName), + "Failed to set column type: Cannot change column type: decimal_col: decimal\\(20, 5\\) -> decimal\\(20, 3\\)"); + + // Test unsupported: Decimal precision decrease + assertQueryFails( + session, + format("ALTER TABLE %s ALTER COLUMN decimal_col SET DATA TYPE DECIMAL(15, 5)", tableName), + "Failed to set column type: Cannot change column type: decimal_col: decimal\\(20, 5\\) -> decimal\\(15, 5\\)"); + } + finally { + dropTable(session, tableName); + } + }); + } + + @Test + public void testAlterColumnTypeMultipleRowsWithNulls() + { + testWithAllFileFormats((session, fileFormat) -> { + String tableName = "test_alter_multiple_nulls_" + fileFormat.name().toLowerCase(ENGLISH); + try { + assertUpdate(session, format( + "CREATE TABLE %s (" + + "id INTEGER, " + + "int_col INTEGER, " + + "float_col REAL, " + + "decimal_col DECIMAL(10, 2)" + + ") WITH (format = '%s')", + tableName, fileFormat)); + + // Insert multiple rows with various NULL patterns + assertUpdate(session, format( + "INSERT INTO %s VALUES " + + "(1, 100, REAL '1.5', DECIMAL '123.45'), " + + "(2, NULL, REAL '2.5', DECIMAL '234.56'), " + + "(3, 300, NULL, DECIMAL '345.67'), " + + "(4, 400, REAL '4.5', NULL), " + + "(5, NULL, NULL, NULL), " + + "(6, 600, REAL '6.5', DECIMAL '678.90')", + tableName), 6); + + // Convert all columns + assertUpdate(session, format("ALTER TABLE %s ALTER COLUMN int_col SET DATA TYPE BIGINT", tableName)); + assertUpdate(session, format("ALTER TABLE %s ALTER COLUMN float_col SET DATA TYPE DOUBLE", tableName)); + assertUpdate(session, format("ALTER TABLE %s ALTER COLUMN decimal_col SET DATA TYPE DECIMAL(20, 2)", tableName)); + + // Verify all data including NULLs are preserved + assertQuery(session, format("SELECT * FROM %s ORDER BY id", tableName), + "VALUES " + + "(1, CAST(100 AS BIGINT), CAST(1.5 AS DOUBLE), CAST(123.45 AS DECIMAL(20,2))), " + + "(2, NULL, CAST(2.5 AS DOUBLE), CAST(234.56 AS DECIMAL(20,2))), " + + "(3, CAST(300 AS BIGINT), NULL, CAST(345.67 AS DECIMAL(20,2))), " + + "(4, CAST(400 AS BIGINT), CAST(4.5 AS DOUBLE), NULL), " + + "(5, NULL, NULL, NULL), " + + "(6, CAST(600 AS BIGINT), CAST(6.5 AS DOUBLE), CAST(678.90 AS DECIMAL(20,2)))"); + + // Verify NULL counts + assertQuery(session, format("SELECT COUNT(*) FROM %s WHERE int_col IS NULL", tableName), "VALUES (2)"); + assertQuery(session, format("SELECT COUNT(*) FROM %s WHERE float_col IS NULL", tableName), "VALUES (2)"); + assertQuery(session, format("SELECT COUNT(*) FROM %s WHERE decimal_col IS NULL", tableName), "VALUES (2)"); + } + finally { + dropTable(session, tableName); + } + }); + } + private void testWithAllFileFormats(BiConsumer test) { test.accept(getSession(), FileFormat.PARQUET); @@ -2470,7 +2838,9 @@ private void validateShowCreateTableInner(String catalog, String schema, String Optional commentDescription, Map propertyDescriptions) { - MaterializedResult showCreateTable = computeActual(format("SHOW CREATE TABLE %s.%s.%s", catalog, schema, table)); + // Quote schema name if it contains dots (nested namespace) and is not already quoted + String schemaIdentifier = schema.contains(".") && !schema.startsWith("\"") ? format("\"%s\"", schema) : schema; + MaterializedResult showCreateTable = computeActual(format("SHOW CREATE TABLE %s.%s.%s", catalog, schemaIdentifier, table)); String createTableSql = (String) getOnlyElement(showCreateTable.getOnlyColumnAsSet()); SqlParser parser = new SqlParser(); @@ -2498,11 +2868,13 @@ protected Void visitColumnDefinition(ColumnDefinition node, Void context) assertEquals(comment, node.getComment().get()); }); - ImmutableMap.Builder propertiesBuilder = ImmutableMap.builder(); - node.getProperties().forEach(property -> { - propertiesBuilder.put(property.getName().getValue(), property.getValue().toString()); - }); - assertEquals(propertyDescriptions, propertiesBuilder.build()); + if (propertyDescriptions != null) { + ImmutableMap.Builder propertiesBuilder = ImmutableMap.builder(); + node.getProperties().forEach(property -> { + propertiesBuilder.put(property.getName().getValue(), property.getValue().toString()); + }); + assertEquals(propertyDescriptions, propertiesBuilder.build()); + } return null; } }, null); diff --git a/presto-main-base/src/main/java/com/facebook/presto/catalogserver/RemoteMetadataManager.java b/presto-main-base/src/main/java/com/facebook/presto/catalogserver/RemoteMetadataManager.java index 6a7d166597061..65fb724c54b28 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/catalogserver/RemoteMetadataManager.java +++ b/presto-main-base/src/main/java/com/facebook/presto/catalogserver/RemoteMetadataManager.java @@ -17,6 +17,7 @@ import com.facebook.presto.Session; import com.facebook.presto.common.CatalogSchemaName; import com.facebook.presto.common.QualifiedObjectName; +import com.facebook.presto.common.type.Type; import com.facebook.presto.metadata.CatalogMetadata; import com.facebook.presto.metadata.DelegatingMetadataManager; import com.facebook.presto.metadata.MetadataManager; @@ -108,6 +109,9 @@ public List listTables(Session session, QualifiedTablePrefi : readValue(tableListJson, new TypeReference>() {}); } + @Override + public void setColumnType(Session session, TableHandle tableHandle, ColumnHandle column, Type type) {} + @Override public List listViews(Session session, QualifiedTablePrefix prefix) { diff --git a/presto-main-base/src/main/java/com/facebook/presto/execution/SetColumnTypeTask.java b/presto-main-base/src/main/java/com/facebook/presto/execution/SetColumnTypeTask.java new file mode 100644 index 0000000000000..680831e81dfd8 --- /dev/null +++ b/presto-main-base/src/main/java/com/facebook/presto/execution/SetColumnTypeTask.java @@ -0,0 +1,109 @@ +/* + * 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.common.type.Type; +import com.facebook.presto.metadata.Metadata; +import com.facebook.presto.spi.ColumnHandle; +import com.facebook.presto.spi.MaterializedViewDefinition; +import com.facebook.presto.spi.TableHandle; +import com.facebook.presto.spi.WarningCollector; +import com.facebook.presto.spi.security.AccessControl; +import com.facebook.presto.sql.analyzer.SemanticErrorCode; +import com.facebook.presto.sql.analyzer.SemanticException; +import com.facebook.presto.sql.tree.Expression; +import com.facebook.presto.sql.tree.SetColumnType; +import com.facebook.presto.transaction.TransactionManager; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.inject.Inject; + +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import static com.facebook.presto.common.type.TypeSignature.parseTypeSignature; +import static com.facebook.presto.common.type.UnknownType.UNKNOWN; +import static com.facebook.presto.metadata.MetadataUtil.createQualifiedObjectName; +import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_COLUMN; +import static com.facebook.presto.sql.analyzer.SemanticErrorCode.MISSING_TABLE; +import static com.facebook.presto.sql.analyzer.SemanticErrorCode.TYPE_MISMATCH; +import static com.google.common.util.concurrent.Futures.immediateFuture; +import static java.util.Objects.requireNonNull; + +public class SetColumnTypeTask + implements DDLDefinitionTask +{ + private final Metadata metadata; + + @Inject + public SetColumnTypeTask(Metadata metadata) + { + this.metadata = requireNonNull(metadata, "metadata is null"); + } + + @Override + public String getName() + { + return "SET COLUMN TYPE"; + } + + public ListenableFuture execute(SetColumnType statement, TransactionManager transactionManager, Metadata metadata, AccessControl accessControl, Session session, List parameters, WarningCollector warningCollector, String query) + { + QualifiedObjectName tableName = createQualifiedObjectName(session, statement, statement.getTableName(), metadata); + Optional tableHandle = metadata.getMetadataResolver(session).getTableHandle(tableName); + if (!tableHandle.isPresent()) { + if (!statement.isTableExists()) { + throw new SemanticException(MISSING_TABLE, statement, "Table '%s' does not exist", tableName); + } + return immediateFuture(null); + } + + Optional optionalMaterializedView = metadata.getMetadataResolver(session).getMaterializedView(tableName); + if (optionalMaterializedView.isPresent()) { + if (!statement.isTableExists()) { + throw new SemanticException(SemanticErrorCode.NOT_SUPPORTED, statement, "'%s' is a materialized view, and alter column is not supported", tableName); + } + return immediateFuture(null); + } + + accessControl.checkCanAlterColumn(session.getRequiredTransactionId(), session.getIdentity(), session.getAccessControlContext(), tableName); + + TableHandle tableHandleOptional = tableHandle.get(); + Map columnHandles = metadata.getColumnHandles(session, tableHandleOptional); + ColumnHandle column = columnHandles.get(statement.getColumnName().getValue()); + if (column == null) { + throw new SemanticException(MISSING_COLUMN, statement, "Column '%s' does not exist", statement.getColumnName()); + } + metadata.setColumnType(session, tableHandleOptional, column, getColumnType(statement)); + + return immediateFuture(null); + } + + private Type getColumnType(SetColumnType statement) + { + Type type; + try { + type = metadata.getType(parseTypeSignature(statement.getType())); + } + catch (IllegalArgumentException e) { + throw new SemanticException(TYPE_MISMATCH, statement, "Unknown type '%s' for column '%s'", statement.getType(), statement.getColumnName()); + } + if (type.equals(UNKNOWN)) { + throw new SemanticException(TYPE_MISMATCH, statement, "Unknown type '%s' for column '%s'", statement.getType(), statement.getColumnName()); + } + return type; + } +} diff --git a/presto-main-base/src/main/java/com/facebook/presto/metadata/Metadata.java b/presto-main-base/src/main/java/com/facebook/presto/metadata/Metadata.java index 25c736829fc5f..580fe279b8d85 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/metadata/Metadata.java +++ b/presto-main-base/src/main/java/com/facebook/presto/metadata/Metadata.java @@ -243,6 +243,11 @@ public interface Metadata */ void addColumn(Session session, TableHandle tableHandle, ColumnMetadata column); + /** + * Set the specified type to the column. + */ + void setColumnType(Session session, TableHandle tableHandle, ColumnHandle column, Type type); + /** * Drop the specified column. */ diff --git a/presto-main-base/src/main/java/com/facebook/presto/metadata/MetadataManager.java b/presto-main-base/src/main/java/com/facebook/presto/metadata/MetadataManager.java index 3d8e1ca08b784..81cb161efe51e 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/metadata/MetadataManager.java +++ b/presto-main-base/src/main/java/com/facebook/presto/metadata/MetadataManager.java @@ -826,6 +826,14 @@ public void dropColumn(Session session, TableHandle tableHandle, ColumnHandle co metadata.dropColumn(session.toConnectorSession(connectorId), tableHandle.getConnectorHandle(), column); } + @Override + public void setColumnType(Session session, TableHandle tableHandle, ColumnHandle column, Type type) + { + ConnectorId connectorId = tableHandle.getConnectorId(); + ConnectorMetadata metadata = getMetadataForWrite(session, connectorId); + metadata.setColumnType(session.toConnectorSession(connectorId), tableHandle.getConnectorHandle(), column, type); + } + @Override public void dropTable(Session session, TableHandle tableHandle) { diff --git a/presto-main-base/src/main/java/com/facebook/presto/metadata/MetadataManagerStats.java b/presto-main-base/src/main/java/com/facebook/presto/metadata/MetadataManagerStats.java index 2678398c94328..3ecae6568d86d 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/metadata/MetadataManagerStats.java +++ b/presto-main-base/src/main/java/com/facebook/presto/metadata/MetadataManagerStats.java @@ -124,6 +124,7 @@ public class MetadataManagerStats private final AtomicLong dropTagCalls = new AtomicLong(); private final AtomicLong dropConstraintCalls = new AtomicLong(); private final AtomicLong addConstraintCalls = new AtomicLong(); + private final AtomicLong setColumnTypeCalls = new AtomicLong(); private final AtomicLong renameTableCalls = new AtomicLong(); private final AtomicLong setTablePropertiesCalls = new AtomicLong(); private final AtomicLong addColumnCalls = new AtomicLong(); @@ -235,6 +236,7 @@ public class MetadataManagerStats private final TimeStat createTagTime = new TimeStat(TimeUnit.NANOSECONDS); private final TimeStat dropTagTime = new TimeStat(TimeUnit.NANOSECONDS); private final TimeStat dropConstraintTime = new TimeStat(TimeUnit.NANOSECONDS); + private final TimeStat setColumnTypeTime = new TimeStat(TimeUnit.NANOSECONDS); private final TimeStat addConstraintTime = new TimeStat(TimeUnit.NANOSECONDS); private final TimeStat renameTableTime = new TimeStat(TimeUnit.NANOSECONDS); private final TimeStat setTablePropertiesTime = new TimeStat(TimeUnit.NANOSECONDS); @@ -1807,6 +1809,12 @@ public void recordAddColumnCall(long duration) addColumnTime.add(duration, TimeUnit.NANOSECONDS); } + public void recordSetColumnTypeCall(long duration) + { + setColumnTypeCalls.incrementAndGet(); + setColumnTypeTime.add(duration, TimeUnit.NANOSECONDS); + } + public void recordDropColumnCall(long duration) { dropColumnCalls.incrementAndGet(); diff --git a/presto-main-base/src/main/java/com/facebook/presto/metadata/StatsRecordingMetadataManager.java b/presto-main-base/src/main/java/com/facebook/presto/metadata/StatsRecordingMetadataManager.java index b33ed39dac07f..2ecf29e18ba85 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/metadata/StatsRecordingMetadataManager.java +++ b/presto-main-base/src/main/java/com/facebook/presto/metadata/StatsRecordingMetadataManager.java @@ -1146,6 +1146,18 @@ public void addColumn(Session session, TableHandle tableHandle, ColumnMetadata c } } + @Override + public void setColumnType(Session session, TableHandle tableHandle, ColumnHandle column, Type type) + { + long startTime = System.nanoTime(); + try { + delegate.setColumnType(session, tableHandle, column, type); + } + finally { + stats.recordSetColumnTypeCall(System.nanoTime() - startTime); + } + } + @Override public void dropColumn(Session session, TableHandle tableHandle, ColumnHandle column) { diff --git a/presto-main-base/src/main/java/com/facebook/presto/security/AccessControlManager.java b/presto-main-base/src/main/java/com/facebook/presto/security/AccessControlManager.java index 5b7fe3b943485..253021e4585e4 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/security/AccessControlManager.java +++ b/presto-main-base/src/main/java/com/facebook/presto/security/AccessControlManager.java @@ -463,6 +463,22 @@ public void checkCanAddColumns(TransactionId transactionId, Identity identity, A } } + @Override + public void checkCanAlterColumn(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName) + { + requireNonNull(identity, "identity is null"); + requireNonNull(tableName, "tableName is null"); + + authenticationCheck(() -> checkCanAccessCatalog(identity, context, tableName.getCatalogName())); + + authorizationCheck(() -> systemAccessControl.checkCanAlterColumn(identity, context, toCatalogSchemaTableName(tableName))); + + CatalogAccessControlEntry entry = getConnectorAccessControl(transactionId, tableName.getCatalogName()); + if (entry != null) { + authorizationCheck(() -> entry.getAccessControl().checkCanAlterColumn(entry.getTransactionHandle(transactionId), identity.toConnectorIdentity(tableName.getCatalogName()), context, toSchemaTableName(tableName))); + } + } + @Override public void checkCanDropColumn(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName) { diff --git a/presto-main-base/src/main/java/com/facebook/presto/security/AllowAllSystemAccessControl.java b/presto-main-base/src/main/java/com/facebook/presto/security/AllowAllSystemAccessControl.java index b5c3441bb88d7..eef3182a9c366 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/security/AllowAllSystemAccessControl.java +++ b/presto-main-base/src/main/java/com/facebook/presto/security/AllowAllSystemAccessControl.java @@ -108,6 +108,11 @@ public void checkCanDropSchema(Identity identity, AccessControlContext context, { } + @Override + public void checkCanAlterColumn(Identity identity, AccessControlContext context, CatalogSchemaTableName table) + { + } + @Override public void checkCanRenameSchema(Identity identity, AccessControlContext context, CatalogSchemaName schema, String newSchemaName) { diff --git a/presto-main-base/src/main/java/com/facebook/presto/security/FileBasedSystemAccessControl.java b/presto-main-base/src/main/java/com/facebook/presto/security/FileBasedSystemAccessControl.java index cbea6676a0dce..f9f105e9e28a9 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/security/FileBasedSystemAccessControl.java +++ b/presto-main-base/src/main/java/com/facebook/presto/security/FileBasedSystemAccessControl.java @@ -55,6 +55,7 @@ import static com.facebook.presto.spi.StandardErrorCode.CONFIGURATION_INVALID; import static com.facebook.presto.spi.security.AccessDeniedException.denyAddColumn; import static com.facebook.presto.spi.security.AccessDeniedException.denyAddConstraint; +import static com.facebook.presto.spi.security.AccessDeniedException.denyAlterColumn; import static com.facebook.presto.spi.security.AccessDeniedException.denyCallProcedure; import static com.facebook.presto.spi.security.AccessDeniedException.denyCatalogAccess; import static com.facebook.presto.spi.security.AccessDeniedException.denyCreateBranch; @@ -264,6 +265,14 @@ public void checkCanDropSchema(Identity identity, AccessControlContext context, } } + @Override + public void checkCanAlterColumn(Identity identity, AccessControlContext context, CatalogSchemaTableName table) + { + if (!canAccessCatalog(identity, table.getCatalogName(), ALL)) { + denyAlterColumn(table.toString()); + } + } + @Override public void checkCanRenameSchema(Identity identity, AccessControlContext context, CatalogSchemaName schema, String newSchemaName) { diff --git a/presto-main-base/src/main/java/com/facebook/presto/security/StatsRecordingSystemAccessControl.java b/presto-main-base/src/main/java/com/facebook/presto/security/StatsRecordingSystemAccessControl.java index 82865ce6de482..b777d798ffbe4 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/security/StatsRecordingSystemAccessControl.java +++ b/presto-main-base/src/main/java/com/facebook/presto/security/StatsRecordingSystemAccessControl.java @@ -440,6 +440,24 @@ public void checkCanAddColumn(Identity identity, AccessControlContext context, C } } + @Override + public void checkCanAlterColumn(Identity identity, AccessControlContext context, CatalogSchemaTableName table) + { + long start = System.nanoTime(); + try { + delegate.get().checkCanAlterColumn(identity, context, table); + } + catch (RuntimeException e) { + stats.checkCanAlterColumn.recordFailure(); + throw e; + } + finally { + long duration = System.nanoTime() - start; + context.getRuntimeStats().addMetricValue("systemAccessControl.checkCanAlterColumn", RuntimeUnit.NANO, duration); + stats.checkCanAlterColumn.record(duration); + } + } + @Override public void checkCanDropColumn(Identity identity, AccessControlContext context, CatalogSchemaTableName table) { @@ -878,6 +896,7 @@ public static class Stats final SystemAccessControlStats filterColumns = new SystemAccessControlStats(); final SystemAccessControlStats checkCanAddColumn = new SystemAccessControlStats(); final SystemAccessControlStats checkCanDropColumn = new SystemAccessControlStats(); + final SystemAccessControlStats checkCanAlterColumn = new SystemAccessControlStats(); final SystemAccessControlStats checkCanRenameColumn = new SystemAccessControlStats(); final SystemAccessControlStats checkCanSelectFromColumns = new SystemAccessControlStats(); final SystemAccessControlStats checkCanCallProcedure = new SystemAccessControlStats(); diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java b/presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java index e536f23a20296..f3fc455d2a14f 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java @@ -189,6 +189,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.SetColumnType; import com.facebook.presto.sql.tree.SetOperation; import com.facebook.presto.sql.tree.SetProperties; import com.facebook.presto.sql.tree.SetSession; @@ -1346,6 +1347,12 @@ protected Scope visitRollback(Rollback node, Optional scope) return createAndAssignScope(node, scope); } + @Override + protected Scope visitSetColumnType(SetColumnType node, Optional scope) + { + return createAndAssignScope(node, scope); + } + @Override protected Scope visitPrepare(Prepare node, Optional scope) { diff --git a/presto-main-base/src/main/java/com/facebook/presto/testing/TestingAccessControlManager.java b/presto-main-base/src/main/java/com/facebook/presto/testing/TestingAccessControlManager.java index 7ccec9adafbc0..c3134dc465d70 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/testing/TestingAccessControlManager.java +++ b/presto-main-base/src/main/java/com/facebook/presto/testing/TestingAccessControlManager.java @@ -41,6 +41,7 @@ import static com.facebook.presto.spi.security.AccessDeniedException.denyAddColumn; import static com.facebook.presto.spi.security.AccessDeniedException.denyAddConstraint; +import static com.facebook.presto.spi.security.AccessDeniedException.denyAlterColumn; import static com.facebook.presto.spi.security.AccessDeniedException.denyCallProcedure; import static com.facebook.presto.spi.security.AccessDeniedException.denyCreateSchema; import static com.facebook.presto.spi.security.AccessDeniedException.denyCreateTable; @@ -67,6 +68,7 @@ import static com.facebook.presto.spi.security.AccessDeniedException.denyUpdateTableColumns; import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.ADD_COLUMN; import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.ADD_CONSTRAINT; +import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.ALTER_COLUMN; import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.CREATE_SCHEMA; import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.CREATE_TABLE; import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.CREATE_VIEW; @@ -275,6 +277,15 @@ public void checkCanRenameColumn(TransactionId transactionId, Identity identity, super.checkCanRenameColumn(transactionId, identity, context, tableName); } + @Override + public void checkCanAlterColumn(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName) + { + if (shouldDenyPrivilege(identity.getUser(), tableName.getObjectName(), ALTER_COLUMN)) { + denyAlterColumn(tableName.toString()); + } + super.checkCanAlterColumn(transactionId, identity, context, tableName); + } + @Override public void checkCanInsertIntoTable(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName) { @@ -468,7 +479,7 @@ public enum TestingPrivilegeType SET_USER, CREATE_SCHEMA, DROP_SCHEMA, RENAME_SCHEMA, SHOW_CREATE_TABLE, CREATE_TABLE, DROP_TABLE, RENAME_TABLE, INSERT_TABLE, DELETE_TABLE, TRUNCATE_TABLE, UPDATE_TABLE, - ADD_COLUMN, DROP_COLUMN, RENAME_COLUMN, SELECT_COLUMN, + ADD_COLUMN, DROP_COLUMN, RENAME_COLUMN, SELECT_COLUMN, ALTER_COLUMN, DROP_BRANCH, DROP_TAG, ADD_CONSTRAINT, DROP_CONSTRAINT, CREATE_VIEW, RENAME_VIEW, DROP_VIEW, CREATE_VIEW_WITH_SELECT_COLUMNS, SET_TABLE_PROPERTIES, diff --git a/presto-main-base/src/main/java/com/facebook/presto/testing/TestingMetadata.java b/presto-main-base/src/main/java/com/facebook/presto/testing/TestingMetadata.java index 0cdd1bdbd1641..a336e35eb19ee 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/testing/TestingMetadata.java +++ b/presto-main-base/src/main/java/com/facebook/presto/testing/TestingMetadata.java @@ -323,6 +323,15 @@ public void dropMaterializedView(ConnectorSession session, SchemaTableName viewN materializedViews.remove(viewName); } + @Override + public void setColumnType(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnHandle column, Type type) + { + ConnectorTableMetadata tableMetadata = getTableMetadata(session, tableHandle); + SchemaTableName tableName = getTableName(tableHandle); + List columns = new ArrayList<>(tableMetadata.getColumns()); + tables.put(tableName, new ConnectorTableMetadata(tableName, ImmutableList.copyOf(columns), tableMetadata.getProperties(), tableMetadata.getComment())); + } + @Override public void createMaterializedView(ConnectorSession session, ConnectorTableMetadata viewMetadata, MaterializedViewDefinition viewDefinition, boolean ignoreExisting) { diff --git a/presto-main-base/src/main/java/com/facebook/presto/util/PrestoDataDefBindingHelper.java b/presto-main-base/src/main/java/com/facebook/presto/util/PrestoDataDefBindingHelper.java index efcdc6e9d16ed..a9ee931ab0eeb 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/util/PrestoDataDefBindingHelper.java +++ b/presto-main-base/src/main/java/com/facebook/presto/util/PrestoDataDefBindingHelper.java @@ -51,6 +51,7 @@ import com.facebook.presto.execution.RevokeRolesTask; import com.facebook.presto.execution.RevokeTask; import com.facebook.presto.execution.RollbackTask; +import com.facebook.presto.execution.SetColumnTypeTask; import com.facebook.presto.execution.SetPropertiesTask; import com.facebook.presto.execution.SetRoleTask; import com.facebook.presto.execution.SetSessionTask; @@ -94,6 +95,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.SetColumnType; import com.facebook.presto.sql.tree.SetProperties; import com.facebook.presto.sql.tree.SetRole; import com.facebook.presto.sql.tree.SetSession; @@ -155,6 +157,7 @@ private PrestoDataDefBindingHelper() {} dataDefBuilder.put(RevokeRoles.class, RevokeRolesTask.class); dataDefBuilder.put(Grant.class, GrantTask.class); dataDefBuilder.put(Revoke.class, RevokeTask.class); + dataDefBuilder.put(SetColumnType.class, SetColumnTypeTask.class); transactionDefBuilder.put(Use.class, UseTask.class); transactionDefBuilder.put(SetSession.class, SetSessionTask.class); diff --git a/presto-main-base/src/test/java/com/facebook/presto/connector/MockConnectorFactory.java b/presto-main-base/src/test/java/com/facebook/presto/connector/MockConnectorFactory.java index 43c26a2447636..f9dc7aa89fd80 100644 --- a/presto-main-base/src/test/java/com/facebook/presto/connector/MockConnectorFactory.java +++ b/presto-main-base/src/test/java/com/facebook/presto/connector/MockConnectorFactory.java @@ -13,6 +13,7 @@ */ package com.facebook.presto.connector; +import com.facebook.presto.common.type.Type; import com.facebook.presto.spi.ColumnHandle; import com.facebook.presto.spi.ColumnMetadata; import com.facebook.presto.spi.ConnectorHandleResolver; @@ -295,6 +296,8 @@ public Builder withGetViews(BiFunction> getColumnHandles) { this.getColumnHandles = requireNonNull(getColumnHandles, "getColumnHandles is null"); diff --git a/presto-main-base/src/test/java/com/facebook/presto/execution/TestSetColumnTypeTask.java b/presto-main-base/src/test/java/com/facebook/presto/execution/TestSetColumnTypeTask.java new file mode 100644 index 0000000000000..d3ba53335b97e --- /dev/null +++ b/presto-main-base/src/test/java/com/facebook/presto/execution/TestSetColumnTypeTask.java @@ -0,0 +1,306 @@ +/* + * 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.common.transaction.TransactionId; +import com.facebook.presto.common.type.Type; +import com.facebook.presto.common.type.TypeSignature; +import com.facebook.presto.connector.informationSchema.InformationSchemaTableHandle; +import com.facebook.presto.connector.informationSchema.InformationSchemaTransactionHandle; +import com.facebook.presto.metadata.AbstractMockMetadata; +import com.facebook.presto.metadata.Catalog; +import com.facebook.presto.metadata.CatalogManager; +import com.facebook.presto.metadata.ColumnPropertyManager; +import com.facebook.presto.metadata.FunctionAndTypeManager; +import com.facebook.presto.metadata.TablePropertyManager; +import com.facebook.presto.spi.ColumnHandle; +import com.facebook.presto.spi.ColumnMetadata; +import com.facebook.presto.spi.ConnectorId; +import com.facebook.presto.spi.ConnectorTableHandle; +import com.facebook.presto.spi.ConnectorTableMetadata; +import com.facebook.presto.spi.PrestoException; +import com.facebook.presto.spi.SchemaTableName; +import com.facebook.presto.spi.TableHandle; +import com.facebook.presto.spi.TableMetadata; +import com.facebook.presto.spi.TestingColumnHandle; +import com.facebook.presto.spi.WarningCollector; +import com.facebook.presto.spi.connector.ConnectorCapabilities; +import com.facebook.presto.spi.connector.ConnectorContext; +import com.facebook.presto.spi.connector.ConnectorTransactionHandle; +import com.facebook.presto.spi.security.AccessControl; +import com.facebook.presto.sql.tree.Identifier; +import com.facebook.presto.sql.tree.NodeLocation; +import com.facebook.presto.sql.tree.QualifiedName; +import com.facebook.presto.sql.tree.SetColumnType; +import com.facebook.presto.testing.TestingConnectorContext; +import com.facebook.presto.transaction.TransactionManager; +import com.google.common.collect.ImmutableList; +import com.google.common.util.concurrent.ListenableFuture; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; + +import static com.facebook.airlift.concurrent.MoreFutures.getFutureValue; +import static com.facebook.presto.common.type.BigintType.BIGINT; +import static com.facebook.presto.common.type.IntegerType.INTEGER; +import static com.facebook.presto.metadata.FunctionAndTypeManager.createTestFunctionAndTypeManager; +import static com.facebook.presto.spi.StandardErrorCode.ALREADY_EXISTS; +import static com.facebook.presto.spi.session.PropertyMetadata.stringProperty; +import static com.facebook.presto.sql.QueryUtil.identifier; +import static com.facebook.presto.testing.TestingSession.createBogusTestingCatalog; +import static com.facebook.presto.testing.TestingSession.testSessionBuilder; +import static com.facebook.presto.transaction.InMemoryTransactionManager.createTestTransactionManager; +import static com.google.common.collect.ImmutableMap.toImmutableMap; +import static com.google.common.collect.MoreCollectors.onlyElement; +import static com.google.common.collect.Sets.immutableEnumSet; +import static java.util.Collections.emptySet; +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; + +@Test(singleThreaded = true) +public class TestSetColumnTypeTask +{ + public static final String SCHEMA = "schema"; + private static final String CATALOG_NAME = "catalog"; + private Session testSession; + + protected TransactionManager transactionManager; + protected MockMetadata metadata; + protected AccessControl accessControl; + protected ConnectorContext context; + protected WarningCollector warningCollector; + + protected static QualifiedObjectName qualifiedObjectName(String objectName) + { + return new QualifiedObjectName(CATALOG_NAME, SCHEMA, objectName); + } + + protected static QualifiedName asQualifiedName(QualifiedObjectName qualifiedObjectName) + { + return QualifiedName.of(qualifiedObjectName.getCatalogName(), qualifiedObjectName.getSchemaName(), qualifiedObjectName.getObjectName()); + } + + @BeforeMethod + public void setUp() + { + CatalogManager catalogManager = new CatalogManager(); + FunctionAndTypeManager functionAndTypeManager = createTestFunctionAndTypeManager(); + transactionManager = createTestTransactionManager(catalogManager); + TablePropertyManager tablePropertyManager = new TablePropertyManager(); + ColumnPropertyManager columnPropertyManager = new ColumnPropertyManager(); + Catalog testCatalog = createBogusTestingCatalog(CATALOG_NAME); + catalogManager.registerCatalog(testCatalog); + tablePropertyManager.addProperties(testCatalog.getConnectorId(), + ImmutableList.of(stringProperty("baz", "test property", null, false))); + columnPropertyManager.addProperties(testCatalog.getConnectorId(), ImmutableList.of()); + testSession = testSessionBuilder() + .setTransactionId(transactionManager.beginTransaction(false)) + .build(); + context = new TestingConnectorContext(); + metadata = new MockMetadata( + functionAndTypeManager, + tablePropertyManager, + columnPropertyManager, + testCatalog.getConnectorId(), + emptySet(), testCatalog); + } + @Test + public void testSetDataType() + { + QualifiedObjectName tableName = qualifiedObjectName("existing_table"); + ConnectorId connectorId = new ConnectorId("test"); + ConnectorTableHandle connectorHandle = new InformationSchemaTableHandle("catalog", "schema", "existing_table"); + UUID uuid = UUID.fromString("ffe9ae3e-60de-4175-a0b5-d635767085fa"); + ConnectorTransactionHandle connectorTransactionHandle = new InformationSchemaTransactionHandle(new TransactionId(uuid)); + TableHandle tableHandle = new TableHandle(connectorId, connectorHandle, connectorTransactionHandle, Optional.empty()); + ColumnMetadata columnMetadata = ColumnMetadata.builder().setName("test").setType(BIGINT).build(); + ConnectorTableMetadata ctMetaData = new ConnectorTableMetadata(((InformationSchemaTableHandle) tableHandle.getConnectorHandle()).getSchemaTableName(), ImmutableList.of(columnMetadata)); + metadata.createTable(null, "catalog", ctMetaData, true); + ConnectorTableMetadata tableMetadata = metadata.getTableMetadata(testSession, tableHandle).getMetadata(); + assertEquals(tableMetadata.getColumns(), ImmutableList.of( + columnMetadata)); + + columnMetadata = ColumnMetadata.builder().setName("test").setType(INTEGER).build(); + ctMetaData = new ConnectorTableMetadata(((InformationSchemaTableHandle) tableHandle.getConnectorHandle()).getSchemaTableName(), ImmutableList.of(columnMetadata)); + metadata.createTable(null, "catalog", ctMetaData, true); + + // Change the column type to integer from bigint + getFutureValue(executeSetColumnType(asQualifiedName(tableName), identifier("test"), "INTEGER", true)); + assertThat(metadata.getTableMetadata(testSession, tableHandle).getColumns()) + .isEqualTo(ImmutableList.of(ColumnMetadata.builder().setName("test").setType(INTEGER).build())); + + // Specify the same column type + getFutureValue(executeSetColumnType(asQualifiedName(tableName), identifier("test"), "INTEGER", true)); + assertThat(metadata.getTableMetadata(testSession, tableHandle).getColumns()) + .isEqualTo(ImmutableList.of(ColumnMetadata.builder().setName("test").setType(INTEGER).build())); + } + + @Test + public void testSetDataTypeNotExistingTable() + { + QualifiedObjectName tableName = qualifiedObjectName("not_existing_table"); + + assertThatThrownBy(() -> getFutureValue(executeSetColumnType(asQualifiedName(tableName), identifier("test"), "INTEGER", false))); + } + + @Test + public void testSetDataTypeNotExistingTableIfExists() + { + QualifiedObjectName tableName = qualifiedObjectName("not_existing_table"); + + getFutureValue(executeSetColumnType(asQualifiedName(tableName), identifier("test"), "INTEGER", true)); + // no exception + } + + @Test + public void testSetDataTypeNotExistingColumn() + { + QualifiedObjectName tableName = qualifiedObjectName("existing_table"); + assertThatThrownBy(() -> getFutureValue(executeSetColumnType(asQualifiedName(tableName), identifier("not_existing_column"), "INTEGER", false))); + } + + private ListenableFuture executeSetColumnType(QualifiedName table, Identifier column, String type, boolean exists) + { + return new SetColumnTypeTask(metadata) + .execute(new SetColumnType(new NodeLocation(1, 1), table, column, type, exists), transactionManager, metadata, accessControl, testSession, ImmutableList.of(), warningCollector, null); + } + + private static class MockMetadata + extends AbstractMockMetadata + { + private final FunctionAndTypeManager functionAndTypeManager; + private final TablePropertyManager tablePropertyManager; + private final ColumnPropertyManager columnPropertyManager; + private final ConnectorId catalogHandle; + private final Map tables = new ConcurrentHashMap<>(); + private final Set connectorCapabilities; + private final Catalog catalog; + public MockMetadata( + FunctionAndTypeManager functionAndTypeManager, + TablePropertyManager tablePropertyManager, + ColumnPropertyManager columnPropertyManager, + ConnectorId catalogHandle, + Set connectorCapabilities, Catalog testCatalog) + { + this.functionAndTypeManager = requireNonNull(functionAndTypeManager, "functionAndTypeManager is null"); + this.tablePropertyManager = requireNonNull(tablePropertyManager, "tablePropertyManager is null"); + this.columnPropertyManager = requireNonNull(columnPropertyManager, "columnPropertyManager is null"); + this.catalogHandle = requireNonNull(catalogHandle, "catalogHandle is null"); + this.catalog = testCatalog; + this.connectorCapabilities = requireNonNull(immutableEnumSet(connectorCapabilities), "connectorCapabilities is null"); + } + + @Override + public void setColumnType(Session session, TableHandle tableHandle, ColumnHandle columnHandle, Type type) + { + SchemaTableName tableName = getTableName(tableHandle); + ConnectorTableMetadata metadata = tables.get(tableName); + + ImmutableList.Builder columns = ImmutableList.builderWithExpectedSize(metadata.getColumns().size()); + for (ColumnMetadata column : metadata.getColumns()) { + if (column.getName().equals(((TestingColumnHandle) columnHandle).getName())) { + columns.add(ColumnMetadata.builder().setName(column.getName()).setType(type).build()); + } + else { + columns.add(column); + } + } + tables.put(tableName, new ConnectorTableMetadata(tableName, columns.build())); + } + + @Override + public void createTable(Session session, String catalogName, ConnectorTableMetadata tableMetadata, boolean ignoreExisting) + { + tables.put(tableMetadata.getTable(), tableMetadata); + if (!ignoreExisting) { + throw new PrestoException(ALREADY_EXISTS, "Table already exists"); + } + } + + @Override + public TablePropertyManager getTablePropertyManager() + { + return tablePropertyManager; + } + + @Override + public ColumnPropertyManager getColumnPropertyManager() + { + return columnPropertyManager; + } + + @Override + public TableMetadata getTableMetadata(Session session, TableHandle tableHandle) + { + return new TableMetadata(catalog.getConnectorId(), getTableMetadata(tableHandle)); + } + + private ConnectorTableMetadata getTableMetadata(TableHandle tableHandle) + { + return tables.get(((InformationSchemaTableHandle) tableHandle.getConnectorHandle()).getSchemaTableName()); + } + private SchemaTableName getTableName(TableHandle tableHandle) + { + return ((InformationSchemaTableHandle) tableHandle.getConnectorHandle()).getSchemaTableName(); + } + + @Override + public Map getColumnHandles(Session session, TableHandle tableHandle) + { + return getTableMetadata(tableHandle).getColumns().stream() + .collect(toImmutableMap( + ColumnMetadata::getName, + column -> new TestingColumnHandle(column.getName()))); + } + + @Override + public ColumnMetadata getColumnMetadata(Session session, TableHandle tableHandle, ColumnHandle columnHandle) + { + String columnName = ((TestingColumnHandle) columnHandle).getName(); + return getTableMetadata(tableHandle).getColumns().stream() + .filter(column -> column.getName().equals(columnName)) + .collect(onlyElement()); + } + + @Override + public Type getType(TypeSignature signature) + { + return functionAndTypeManager.getType(signature); + } + + @Override + public Optional getCatalogHandle(Session session, String catalogName) + { + if (catalogHandle.getCatalogName().equals(catalogName)) { + return Optional.of(catalogHandle); + } + return Optional.empty(); + } + + @Override + public Set getConnectorCapabilities(Session session, ConnectorId catalogName) + { + return connectorCapabilities; + } + } +} diff --git a/presto-main-base/src/test/java/com/facebook/presto/metadata/AbstractMockMetadata.java b/presto-main-base/src/test/java/com/facebook/presto/metadata/AbstractMockMetadata.java index db0d91d897684..9e933b2b1f5e2 100644 --- a/presto-main-base/src/test/java/com/facebook/presto/metadata/AbstractMockMetadata.java +++ b/presto-main-base/src/test/java/com/facebook/presto/metadata/AbstractMockMetadata.java @@ -337,6 +337,12 @@ public Optional getNewTableLayout(Session session, String catalo throw new UnsupportedOperationException(); } + @Override + public void setColumnType(Session session, TableHandle tableHandle, ColumnHandle column, Type type) + { + throw new UnsupportedOperationException(); + } + @Override public OutputTableHandle beginCreateTable(Session session, String catalogName, ConnectorTableMetadata tableMetadata, Optional layout) { diff --git a/presto-orc/src/main/java/com/facebook/presto/orc/reader/FloatBatchStreamReader.java b/presto-orc/src/main/java/com/facebook/presto/orc/reader/FloatBatchStreamReader.java index 85bfc7849a593..14d2b63975524 100644 --- a/presto-orc/src/main/java/com/facebook/presto/orc/reader/FloatBatchStreamReader.java +++ b/presto-orc/src/main/java/com/facebook/presto/orc/reader/FloatBatchStreamReader.java @@ -16,7 +16,7 @@ import com.facebook.presto.common.block.Block; import com.facebook.presto.common.block.BlockBuilder; import com.facebook.presto.common.block.RunLengthEncodedBlock; -import com.facebook.presto.common.type.RealType; +import com.facebook.presto.common.type.DoubleType; import com.facebook.presto.common.type.Type; import com.facebook.presto.orc.OrcCorruptionException; import com.facebook.presto.orc.StreamDescriptor; @@ -30,6 +30,7 @@ import java.io.IOException; +import static com.facebook.presto.common.type.DoubleType.DOUBLE; import static com.facebook.presto.common.type.RealType.REAL; import static com.facebook.presto.orc.metadata.Stream.StreamKind.DATA; import static com.facebook.presto.orc.metadata.Stream.StreamKind.PRESENT; @@ -46,6 +47,7 @@ public class FloatBatchStreamReader private static final int INSTANCE_SIZE = ClassLayout.parseClass(FloatBatchStreamReader.class).instanceSize(); private final StreamDescriptor streamDescriptor; + private final Type type; private int readOffset; private int nextBatchSize; @@ -63,8 +65,8 @@ public class FloatBatchStreamReader public FloatBatchStreamReader(Type type, StreamDescriptor streamDescriptor) throws OrcCorruptionException { - requireNonNull(type, "type is null"); - verifyStreamType(streamDescriptor, type, RealType.class::isInstance); + this.type = requireNonNull(type, "type is null"); + verifyStreamType(streamDescriptor, type, t -> t == REAL || t == DOUBLE); this.streamDescriptor = requireNonNull(streamDescriptor, "stream is null"); } @@ -99,23 +101,28 @@ public Block readBlock() if (dataStream == null && presentStream != null) { presentStream.skip(nextBatchSize); - Block nullValueBlock = RunLengthEncodedBlock.create(REAL, null, nextBatchSize); + Block nullValueBlock = RunLengthEncodedBlock.create(type, null, nextBatchSize); readOffset = 0; nextBatchSize = 0; return nullValueBlock; } - BlockBuilder builder = REAL.createBlockBuilder(null, nextBatchSize); + BlockBuilder builder = type.createBlockBuilder(null, nextBatchSize); if (presentStream == null) { if (dataStream == null) { throw new OrcCorruptionException(streamDescriptor.getOrcDataSourceId(), "Value is not null but data stream is not present"); } - dataStream.nextVector(REAL, nextBatchSize, builder); + dataStream.nextVector(type, nextBatchSize, builder); } else { for (int i = 0; i < nextBatchSize; i++) { if (presentStream.nextBit()) { - REAL.writeLong(builder, floatToRawIntBits(dataStream.next())); + if (type instanceof DoubleType) { + type.writeDouble(builder, Float.valueOf(dataStream.next()).doubleValue()); + } + else { + type.writeLong(builder, floatToRawIntBits(dataStream.next())); + } } else { builder.appendNull(); diff --git a/presto-orc/src/main/java/com/facebook/presto/orc/stream/FloatInputStream.java b/presto-orc/src/main/java/com/facebook/presto/orc/stream/FloatInputStream.java index 6c1d1c800a38a..71e394c4576fa 100644 --- a/presto-orc/src/main/java/com/facebook/presto/orc/stream/FloatInputStream.java +++ b/presto-orc/src/main/java/com/facebook/presto/orc/stream/FloatInputStream.java @@ -14,6 +14,7 @@ package com.facebook.presto.orc.stream; import com.facebook.presto.common.block.BlockBuilder; +import com.facebook.presto.common.type.DoubleType; import com.facebook.presto.common.type.Type; import com.facebook.presto.orc.checkpoint.FloatStreamCheckpoint; @@ -63,7 +64,12 @@ public void nextVector(Type type, int items, BlockBuilder builder) throws IOException { for (int i = 0; i < items; i++) { - type.writeLong(builder, floatToRawIntBits(next())); + if (type instanceof DoubleType) { + type.writeDouble(builder, Float.valueOf(next()).doubleValue()); + } + else { + type.writeLong(builder, floatToRawIntBits(next())); + } } } } diff --git a/presto-parquet/src/main/java/com/facebook/presto/parquet/reader/ShortDecimalColumnReader.java b/presto-parquet/src/main/java/com/facebook/presto/parquet/reader/ShortDecimalColumnReader.java index 7e22e0528e97a..02d4049e0dc18 100644 --- a/presto-parquet/src/main/java/com/facebook/presto/parquet/reader/ShortDecimalColumnReader.java +++ b/presto-parquet/src/main/java/com/facebook/presto/parquet/reader/ShortDecimalColumnReader.java @@ -14,9 +14,11 @@ package com.facebook.presto.parquet.reader; import com.facebook.presto.common.block.BlockBuilder; +import com.facebook.presto.common.type.LongDecimalType; import com.facebook.presto.common.type.Type; import com.facebook.presto.parquet.RichColumnDescriptor; +import static com.facebook.presto.common.type.Decimals.encodeUnscaledValue; import static com.facebook.presto.parquet.ParquetTypeUtils.getShortDecimalValue; import static org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.INT32; import static org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.INT64; @@ -44,7 +46,12 @@ else if (columnDescriptor.getPrimitiveType().getPrimitiveTypeName().equals(INT64 else { decimalValue = getShortDecimalValue(valuesReader.readBytes().getBytes()); } - type.writeLong(blockBuilder, decimalValue); + if (type instanceof LongDecimalType) { + type.writeSlice(blockBuilder, encodeUnscaledValue(decimalValue)); + } + else { + type.writeLong(blockBuilder, decimalValue); + } } else if (isValueNull()) { blockBuilder.appendNull(); 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 4fd1773f35a81..4b612915598c7 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 @@ -81,6 +81,8 @@ statement DROP BRANCH (IF EXISTS)? name=string #dropBranch | ALTER TABLE (IF EXISTS)? tableName=qualifiedName DROP TAG (IF EXISTS)? name=string #dropTag + | ALTER TABLE (IF EXISTS)? tableName=qualifiedName + ALTER COLUMN columnName=identifier SET DATA TYPE type #setColumnType | 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 06fcf5dd6bbdf..3b6fbefba922c 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 @@ -103,6 +103,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.SetColumnType; import com.facebook.presto.sql.tree.SetProperties; import com.facebook.presto.sql.tree.SetRole; import com.facebook.presto.sql.tree.SetSession; @@ -2086,6 +2087,21 @@ private void processRelation(Relation relation, Integer indent) } } + @Override + protected Void visitSetColumnType(SetColumnType node, Integer context) + { + builder.append("ALTER TABLE "); + if (node.isTableExists()) { + builder.append("IF EXISTS "); + } + builder.append(formatName(node.getTableName())) + .append(" ALTER COLUMN ") + .append(formatName(node.getColumnName())) + .append(" SET DATA TYPE ") + .append(node.getType().toString()); + return null; + } + private StringBuilder append(int indent, String value) { return builder.append(indentString(indent)) 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 6f00cd3042ebd..c936b1a56ed61 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 @@ -157,6 +157,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.SetColumnType; import com.facebook.presto.sql.tree.SetProperties; import com.facebook.presto.sql.tree.SetRole; import com.facebook.presto.sql.tree.SetSession; @@ -1074,6 +1075,17 @@ public Node visitNamedQuery(SqlBaseParser.NamedQueryContext context) columns); } + @Override + public Node visitSetColumnType(SqlBaseParser.SetColumnTypeContext context) + { + return new SetColumnType( + getLocation(context), + getQualifiedName(context.tableName), + (Identifier) visit(context.columnName), + getType(context.type()), + context.EXISTS() != null); + } + @Override public Node visitQueryNoWith(SqlBaseParser.QueryNoWithContext 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 bb76598937052..964f6553c0120 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 @@ -827,6 +827,10 @@ protected R visitIsolationLevel(Isolation node, C context) return visitTransactionMode(node, context); } + protected R visitSetColumnType(SetColumnType node, C context) + { + return visitStatement(node, context); + } protected R visitTransactionAccessMode(TransactionAccessMode node, C context) { return visitTransactionMode(node, context); diff --git a/presto-parser/src/main/java/com/facebook/presto/sql/tree/SetColumnType.java b/presto-parser/src/main/java/com/facebook/presto/sql/tree/SetColumnType.java new file mode 100644 index 0000000000000..9b9a2537fe9d2 --- /dev/null +++ b/presto-parser/src/main/java/com/facebook/presto/sql/tree/SetColumnType.java @@ -0,0 +1,106 @@ +/* + * 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 SetColumnType + extends Statement +{ + private final QualifiedName tableName; + private final Identifier columnName; + private final String type; + private final boolean tableExists; + + public SetColumnType(NodeLocation location, QualifiedName tableName, Identifier columnName, String type, boolean tableExists) + { + super(Optional.of(location)); + this.tableName = requireNonNull(tableName, "tableName is null"); + this.columnName = requireNonNull(columnName, "columnName is null"); + this.type = requireNonNull(type, "type is null"); + this.tableExists = tableExists; + } + + public QualifiedName getTableName() + { + return tableName; + } + + public Identifier getColumnName() + { + return columnName; + } + + public String getType() + { + return type; + } + + public boolean isTableExists() + { + return tableExists; + } + + @Override + public R accept(AstVisitor visitor, C context) + { + return visitor.visitSetColumnType(this, context); + } + + @Override + public List getChildren() + { + return ImmutableList.of(); + } + + @Override + public int hashCode() + { + return Objects.hash(tableName, columnName, type, tableExists); + } + + @Override + public boolean equals(Object obj) + { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + SetColumnType o = (SetColumnType) obj; + return Objects.equals(tableName, o.tableName) && + Objects.equals(columnName, o.columnName) && + Objects.equals(type, o.type) && + Objects.equals(tableExists, o.tableExists); + } + + @Override + public String toString() + { + return toStringHelper(this) + .add("name", tableName) + .add("column", columnName) + .add("type", type) + .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 1c9438e565c79..1a249392bc328 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 @@ -134,6 +134,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.SetColumnType; import com.facebook.presto.sql.tree.SetProperties; import com.facebook.presto.sql.tree.SetRole; import com.facebook.presto.sql.tree.SetSession; @@ -1979,6 +1980,24 @@ public void testAddColumn() new ColumnDefinition(identifier("country"), "varchar", true, emptyList(), Optional.empty(), Optional.of(new StringLiteral("UK"))), true, false)); } + @Test + public void testAlterColumnSetDataType() + { + assertStatement("ALTER TABLE foo.t ALTER COLUMN c SET DATA TYPE BIGINT", new SetColumnType( + new NodeLocation(1, 1), + QualifiedName.of("foo", "t"), + new Identifier("c"), + "BIGINT", + false)); + + assertStatement("ALTER TABLE IF EXISTS foo.t ALTER COLUMN b SET DATA TYPE DOUBLE", new SetColumnType( + new NodeLocation(1, 1), + QualifiedName.of("foo", "t"), + new Identifier("b"), + "DOUBLE", + true)); + } + @Test public void testDropColumn() { 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 8b1dcc5845d03..00786a4b18bc5 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 @@ -263,6 +263,9 @@ public void testStatementBuilder() printStatement("drop view foo"); + printStatement("alter table foo alter column x set data type bigint"); + printStatement("alter table a.b.c alter column x set data type bigint"); + printStatement("insert into t select * from t"); printStatement("insert into t (c1, c2) select * from t"); 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 3150a1cf42d9c..3144b67210ce4 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 @@ -122,6 +122,11 @@ public void checkCanRenameColumn(ConnectorTransactionHandle transaction, Connect { } + @Override + public void checkCanAlterColumn(ConnectorTransactionHandle transaction, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName) + { + } + @Override public void checkCanSelectFromColumns(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName, Set columnOrSubfieldNames) { 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 dfe595ddf248d..52e446764703c 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 @@ -45,6 +45,7 @@ import static com.facebook.presto.plugin.base.security.TableAccessControlRule.TablePrivilege.UPDATE; import static com.facebook.presto.spi.security.AccessDeniedException.denyAddColumn; import static com.facebook.presto.spi.security.AccessDeniedException.denyAddConstraint; +import static com.facebook.presto.spi.security.AccessDeniedException.denyAlterColumn; import static com.facebook.presto.spi.security.AccessDeniedException.denyCallProcedure; import static com.facebook.presto.spi.security.AccessDeniedException.denyCreateBranch; import static com.facebook.presto.spi.security.AccessDeniedException.denyCreateSchema; @@ -308,6 +309,14 @@ public void checkCanCreateViewWithSelectFromColumns(ConnectorTransactionHandle t } } + @Override + public void checkCanAlterColumn(ConnectorTransactionHandle transaction, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName) + { + if (!checkTablePermission(identity, tableName, OWNERSHIP)) { + denyAlterColumn(tableName.toString()); + } + } + @Override public void checkCanSetCatalogSessionProperty(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, String propertyName) { 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 000d577c39622..bfbac8fae24d9 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 @@ -98,6 +98,12 @@ public void checkCanDropTable(ConnectorTransactionHandle transactionHandle, Conn delegate().checkCanDropTable(transactionHandle, identity, context, tableName); } + @Override + public void checkCanAlterColumn(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName) + { + delegate().checkCanAlterColumn(transactionHandle, identity, context, tableName); + } + @Override public void checkCanRenameTable(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName, SchemaTableName newTableName) { 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 aa64cb7f8e016..d650a9cf8848a 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 @@ -104,6 +104,12 @@ public void checkCanDropSchema(Identity identity, AccessControlContext context, delegate().checkCanDropSchema(identity, context, schema); } + @Override + public void checkCanAlterColumn(Identity identity, AccessControlContext context, CatalogSchemaTableName table) + { + delegate().checkCanAlterColumn(identity, context, table); + } + @Override public void checkCanRenameSchema(Identity identity, AccessControlContext context, CatalogSchemaName schema, String newSchemaName) { diff --git a/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ReadOnlyAccessControl.java b/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ReadOnlyAccessControl.java index c3dba7b133e20..b685d072cceef 100644 --- a/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ReadOnlyAccessControl.java +++ b/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ReadOnlyAccessControl.java @@ -26,6 +26,7 @@ import java.util.List; import java.util.Set; +import static com.facebook.presto.spi.security.AccessDeniedException.denyAlterColumn; import static com.facebook.presto.spi.security.AccessDeniedException.denyGrantTablePrivilege; import static com.facebook.presto.spi.security.AccessDeniedException.denyRenameView; import static com.facebook.presto.spi.security.AccessDeniedException.denyRevokeTablePrivilege; @@ -51,6 +52,11 @@ public void checkCanShowCreateTable(ConnectorTransactionHandle transactionHandle // allow } + public void checkCanAlterColumn(ConnectorTransactionHandle transaction, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName) + { + denyAlterColumn(tableName.toString()); + } + @Override public void checkCanShowTablesMetadata(ConnectorTransactionHandle transactionHandle, ConnectorIdentity identity, AccessControlContext context, String schemaName) { 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 bc49a71f5fd71..c845634b58dbc 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 @@ -77,6 +77,7 @@ public void testTableRules() assertDenied(() -> accessControl.checkCanRenameTable(TRANSACTION_HANDLE, user("bob"), CONTEXT, new SchemaTableName("bobschema", "bobtable"), new SchemaTableName("bobschema", "newbobtable"))); 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()); + accessControl.checkCanAlterColumn(TRANSACTION_HANDLE, user("admin"), CONTEXT, new SchemaTableName("bobschema", "bobtable")); 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())); @@ -86,6 +87,7 @@ public void testTableRules() assertDenied(() -> accessControl.checkCanCreateViewWithSelectFromColumns(TRANSACTION_HANDLE, user("joe"), CONTEXT, new SchemaTableName("bobschema", "bobtable"), ImmutableSet.of())); assertDenied(() -> accessControl.checkCanRenameView(TRANSACTION_HANDLE, user("bob"), CONTEXT, new SchemaTableName("bobschema", "bobview"), new SchemaTableName("bobschema", "newbobview"))); assertDenied(() -> accessControl.checkCanRenameView(TRANSACTION_HANDLE, user("alice"), CONTEXT, new SchemaTableName("aliceschema", "alicetable"), new SchemaTableName("bobschema", "newalicetable"))); + assertDenied(() -> accessControl.checkCanAlterColumn(TRANSACTION_HANDLE, user("alice"), CONTEXT, new SchemaTableName("bobschema", "bobtable"))); } @Test 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 cc2e55cbede1b..e82ba43a6cf18 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 @@ -30,6 +30,7 @@ import static com.facebook.presto.spi.security.AccessDeniedException.denyAddColumn; import static com.facebook.presto.spi.security.AccessDeniedException.denyAddConstraint; +import static com.facebook.presto.spi.security.AccessDeniedException.denyAlterColumn; import static com.facebook.presto.spi.security.AccessDeniedException.denyCallProcedure; import static com.facebook.presto.spi.security.AccessDeniedException.denyCreateBranch; import static com.facebook.presto.spi.security.AccessDeniedException.denyCreateRole; @@ -354,6 +355,16 @@ default void checkCanCreateViewWithSelectFromColumns(ConnectorTransactionHandle denyCreateViewWithSelect(tableName.toString(), identity); } + /** + * Check if identity is allowed to set the specified property in this catalog. + * + * @throws com.facebook.presto.spi.security.AccessDeniedException if not allowed + */ + default void checkCanAlterColumn(ConnectorTransactionHandle transaction, ConnectorIdentity identity, AccessControlContext context, SchemaTableName tableName) + { + denyAlterColumn(tableName.toString()); + } + /** * Check if identity is allowed to set the specified property 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 8e7be0e171b5c..cb9ec536b6443 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 @@ -454,6 +454,15 @@ default Optional getInsertLayout(ConnectorSession sessi return Optional.of(new ConnectorNewTableLayout(partitioningHandle, partitionColumns)); } + /** + * Set the specified column type + */ + + default void setColumnType(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnHandle column, Type type) + { + throw new PrestoException(NOT_SUPPORTED, "This connector does not support setting column types"); + } + /** * Describes statistics that must be collected during a write. */ 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 7e7c6c99bf4e7..2b3892ee546ed 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 @@ -970,4 +970,11 @@ public Optional> applyTable return delegate.applyTableFunction(session, handle); } } + + public void setColumnType(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnHandle columnHandle, Type type) + { + try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) { + delegate.setColumnType(session, tableHandle, columnHandle, type); + } + } } 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 e864e3ac0699a..1265237d197a5 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 @@ -178,6 +178,13 @@ default AuthorizedIdentity selectAuthorizedIdentity(Identity identity, AccessCon */ void checkCanDropColumn(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName); + /** + * Check if identity is allowed to alter columns to the specified table. + * + * @throws AccessDeniedException if not allowed + */ + void checkCanAlterColumn(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName); + /** * Check if identity is allowed to rename a column in the specified table. * 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 03afcc8d4e97d..f0e4568a712b0 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 @@ -187,6 +187,16 @@ public static void denyDropColumn(String tableName, String extraInfo) throw new AccessDeniedException(format("Cannot drop a column from table %s%s", tableName, formatExtraInfo(extraInfo))); } + public static void denyAlterColumn(String tableName) + { + denyAlterColumn(tableName, null); + } + + public static void denyAlterColumn(String tableName, String extraInfo) + { + throw new AccessDeniedException(format("Cannot alter a column for table %s%s", tableName, formatExtraInfo(extraInfo))); + } + public static void denyRenameColumn(String tableName) { denyRenameColumn(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 5f5cdd14ef407..a8e5fe96bd930 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 @@ -130,6 +130,11 @@ public void checkCanAddColumns(TransactionId transactionId, Identity identity, A { } + @Override + public void checkCanAlterColumn(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName) + { + } + @Override public void checkCanDropColumn(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName) { 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 39a66bb9adf4e..ad3040bbb6576 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 @@ -31,6 +31,7 @@ import static com.facebook.presto.spi.security.AccessDeniedException.denyAddColumn; import static com.facebook.presto.spi.security.AccessDeniedException.denyAddConstraint; +import static com.facebook.presto.spi.security.AccessDeniedException.denyAlterColumn; import static com.facebook.presto.spi.security.AccessDeniedException.denyCallProcedure; import static com.facebook.presto.spi.security.AccessDeniedException.denyCatalogAccess; import static com.facebook.presto.spi.security.AccessDeniedException.denyCreateBranch; @@ -196,6 +197,12 @@ public void checkCanAddColumns(TransactionId transactionId, Identity identity, A denyAddColumn(tableName.toString()); } + @Override + public void checkCanAlterColumn(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName) + { + denyAlterColumn(tableName.toString()); + } + @Override public void checkCanRenameColumn(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName) { 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 1090ed8d6a6e6..b79c54ef48437 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 @@ -31,6 +31,7 @@ import static com.facebook.presto.spi.security.AccessDeniedException.denyAddColumn; import static com.facebook.presto.spi.security.AccessDeniedException.denyAddConstraint; +import static com.facebook.presto.spi.security.AccessDeniedException.denyAlterColumn; import static com.facebook.presto.spi.security.AccessDeniedException.denyCallProcedure; import static com.facebook.presto.spi.security.AccessDeniedException.denyCatalogAccess; import static com.facebook.presto.spi.security.AccessDeniedException.denyCreateBranch; @@ -129,6 +130,16 @@ default void checkCanDropSchema(Identity identity, AccessControlContext context, denyDropSchema(schema.toString()); } + /** + * Check if identity is allowed to alter columns for the specified table in a catalog. + * + * @throws AccessDeniedException if not allowed + */ + default void checkCanAlterColumn(Identity identity, AccessControlContext context, CatalogSchemaTableName table) + { + denyAlterColumn(table.toString()); + } + /** * Check if identity is allowed to rename the specified schema in a catalog. * diff --git a/presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestDistributedQueries.java b/presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestDistributedQueries.java index dd6100cbc8f6a..23b52faf2222b 100644 --- a/presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestDistributedQueries.java +++ b/presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestDistributedQueries.java @@ -61,6 +61,7 @@ import static com.facebook.presto.spi.security.ViewSecurity.INVOKER; import static com.facebook.presto.testing.MaterializedResult.resultBuilder; import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.ADD_COLUMN; +import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.ALTER_COLUMN; import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.CREATE_TABLE; import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.CREATE_VIEW; import static com.facebook.presto.testing.TestingAccessControlManager.TestingPrivilegeType.CREATE_VIEW_WITH_SELECT_COLUMNS; @@ -1210,6 +1211,7 @@ public void testNonQueryAccessControl() assertAccessDenied("ALTER TABLE orders ADD COLUMN foo bigint", "Cannot add a column to table .*.orders.*", privilege("orders", ADD_COLUMN)); assertAccessDenied("ALTER TABLE orders DROP COLUMN foo", "Cannot drop a column from table .*.orders.*", privilege("orders", DROP_COLUMN)); assertAccessDenied("ALTER TABLE orders RENAME COLUMN orderkey TO foo", "Cannot rename a column in table .*.orders.*", privilege("orders", RENAME_COLUMN)); + assertAccessDenied("ALTER TABLE orders ALTER COLUMN orderkey SET DATA TYPE char(100)", "Cannot alter a column for table .*.orders.*", privilege("orders", ALTER_COLUMN)); assertAccessDenied("CREATE VIEW foo as SELECT * FROM orders", "Cannot create view .*.foo.*", privilege("foo", CREATE_VIEW)); // todo add DROP VIEW test... not all connectors have view support