Skip to content

test(plugin-iceberg): Test for alter column types for Iceberg #27744

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hantangwangd wants to merge 4 commits into from
Closed

test(plugin-iceberg): Test for alter column types for Iceberg #27744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f37cfdd
Add support for ALTER COLUMN SET DATA TYPE in the Iceberg connector
NivinCS Jan 26, 2024
4447446
Fix for decimal/float, and adjust the test case
hantangwangd May 7, 2026
7845378
changes to improve the test coverage
May 9, 2026
f7878bc
Fix for parquet batch read mode
hantangwangd May 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/utils/StatementUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down
20 changes: 20 additions & 0 deletions presto-blackhole/src/main/java/com/facebook/presto/plugin/blackhole/BlackHoleMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<BlackHoleColumnHandle> 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()));
}
}
16 changes: 15 additions & 1 deletion presto-common/src/main/java/com/facebook/presto/common/type/DoubleType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -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));
}

Expand Down
25 changes: 21 additions & 4 deletions presto-common/src/main/java/com/facebook/presto/common/type/LongDecimalType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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();
}
}
}

Expand All @@ -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));
Expand Down
1 change: 1 addition & 0 deletions presto-docs/src/main/sphinx/sql/alter-table.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions presto-hive/src/main/java/com/facebook/presto/hive/security/LegacyAccessControl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
19 changes: 19 additions & 0 deletions presto-hive/src/main/java/com/facebook/presto/hive/security/SqlStandardAccessControl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down
19 changes: 19 additions & 0 deletions presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergErrorCode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading
Loading