Skip to content

Icebrg alter table set datatype testcase fix#27707

Draft
NivinCS wants to merge 9 commits intoprestodb:masterfrom
NivinCS:icebrg_alter_table_set_datatype_testcase_fix
Draft

Icebrg alter table set datatype testcase fix#27707
NivinCS wants to merge 9 commits intoprestodb:masterfrom
NivinCS:icebrg_alter_table_set_datatype_testcase_fix

Conversation

@NivinCS
Copy link
Copy Markdown
Contributor

@NivinCS NivinCS commented May 2, 2026

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.
  • If adding new dependencies, verified they have an OpenSSF Scorecard score of 5.0 or higher (or obtained explicit TSC approval for lower scores).

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* ... 
* ... 

Hive Connector Changes
* ... 
* ... 

If release note is NOT required, use:

== NO RELEASE NOTE ==

Summary by Sourcery

Add support for ALTER TABLE ... ALTER COLUMN SET DATA TYPE, including metadata plumbing, access control, Iceberg and Blackhole connector implementations, and associated statistics and error handling.

New Features:

  • Introduce a SetColumnType statement and task to change a column's data type via ALTER TABLE ... ALTER COLUMN SET DATA TYPE.
  • Extend connector metadata and catalog/remote metadata layers to support setting column types.
  • Implement column type alteration for Iceberg and Blackhole connectors.

Bug Fixes:

  • Handle Iceberg table property value extraction correctly for string literals in tests to avoid quoted value mismatches.
  • Fix DOUBLE type value handling when reading REAL-backed data blocks by correctly interpreting IntArrayBlock as floats.
  • Allow the ORC float batch reader to work with both REAL and DOUBLE types for schema evolution scenarios.

Enhancements:

  • Add access control hooks and implementations for ALTER COLUMN operations across system and connector access control layers, including file-based, allow-all, deny-all, legacy, and read-only modes.
  • Record timing and call-count statistics for set-column-type operations in metadata and system access control stats collectors.
  • Update SQL parser grammar, AST visitors, SQL formatter, and statement classification to recognize and format ALTER COLUMN SET DATA TYPE statements.
  • Wire SetColumnTypeTask into the DDL execution framework and transaction binding helper.
  • Extend testing infrastructure (mock metadata, mock connector, testing metadata, access control tests, distributed query tests, parser tests, and Iceberg smoke tests) to cover column type alteration behavior.

Documentation:

  • Document ALTER TABLE column type changes in the ALTER TABLE SQL reference.

Tests:

  • Add unit and integration tests for SetColumnTypeTask, SQL parsing/formatting, access control behavior, Iceberg column type evolution, and file-based access control table rules.

@prestodb-ci prestodb-ci added the from:IBM PR from IBM label May 2, 2026
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented May 2, 2026

CLA Missing ID CLA Not Signed

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented May 2, 2026

Reviewer's Guide

Adds full support for ALTER TABLE ... ALTER COLUMN ... SET DATA TYPE across Presto (parser, analyzer, metadata, security, connectors), including Iceberg and BlackHole, and introduces tests and stats wiring plus a new Iceberg error code and double/float handling fixes.

Sequence diagram for ALTER TABLE ALTER COLUMN SET DATA TYPE execution

sequenceDiagram
    actor User
    participant Client
    participant Coordinator
    participant Parser
    participant Analyzer
    participant SetColumnTypeTask
    participant AccessControlManager
    participant SystemAccessControl
    participant ConnectorAccessControl
    participant MetadataManager
    participant ConnectorMetadata_Iceberg
    participant IcebergTable

    User->>Client: submit ALTER TABLE t ALTER COLUMN c SET DATA TYPE DOUBLE
    Client->>Coordinator: send SQL

    Coordinator->>Parser: parse SQL
    Parser-->>Coordinator: SetColumnType AST

    Coordinator->>Analyzer: analyze SetColumnType
    Analyzer->>MetadataManager: resolve table handle
    MetadataManager-->>Analyzer: TableHandle
    Analyzer-->>Coordinator: analyzed statement

    Coordinator->>SetColumnTypeTask: execute(SetColumnType, session)

    SetColumnTypeTask->>MetadataManager: get table handle
    MetadataManager-->>SetColumnTypeTask: Optional TableHandle

    SetColumnTypeTask->>AccessControlManager: checkCanAlterColumn
    AccessControlManager->>SystemAccessControl: checkCanAlterColumn
    SystemAccessControl-->>AccessControlManager: ok

    AccessControlManager->>ConnectorAccessControl: checkCanAlterColumn
    ConnectorAccessControl-->>AccessControlManager: ok

    AccessControlManager-->>SetColumnTypeTask: authorization ok

    SetColumnTypeTask->>MetadataManager: getColumnHandles
    MetadataManager-->>SetColumnTypeTask: column name to ColumnHandle

    SetColumnTypeTask->>MetadataManager: setColumnType(Session, TableHandle, ColumnHandle, Type)
    MetadataManager->>ConnectorMetadata_Iceberg: setColumnType(ConnectorSession, ConnectorTableHandle, ColumnHandle, Type)

    ConnectorMetadata_Iceberg->>IcebergTable: updateSchema()
    IcebergTable-->>ConnectorMetadata_Iceberg: UpdateSchema
    ConnectorMetadata_Iceberg->>IcebergTable: updateColumn(name, newType)
    ConnectorMetadata_Iceberg->>IcebergTable: commit()
    IcebergTable-->>ConnectorMetadata_Iceberg: success

    ConnectorMetadata_Iceberg-->>MetadataManager: ok
    MetadataManager-->>SetColumnTypeTask: ok
    SetColumnTypeTask-->>Coordinator: completed
    Coordinator-->>Client: success
    Client-->>User: statement succeeded
Loading

Class diagram for SetColumnType AST and execution task

classDiagram
    class Statement {
    }

    class SetColumnType {
        +QualifiedName tableName
        +Identifier columnName
        +String type
        +boolean tableExists
        +QualifiedName getTableName()
        +Identifier getColumnName()
        +String getType()
        +boolean isTableExists()
        +<R,C> R accept(AstVisitor visitor, C context)
    }

    class AstVisitor {
        +<R,C> R visitSetColumnType(SetColumnType node, C context)
    }

    class AstBuilder {
        +Node visitSetColumnType(SqlBaseParser_SetColumnTypeContext context)
    }

    class SqlFormatter_FormatterVisitor {
        +Void visitSetColumnType(SetColumnType node, Integer context)
    }

    class StatementAnalyzer_Visitor {
        +Scope visitSetColumnType(SetColumnType node, Optional_scope scope)
    }

    class StatementUtils {
        -Map statementQueryTypes
    }

    class SetColumnTypeTask {
        -Metadata metadata
        +String getName()
        +ListenableFuture execute(SetColumnType statement, TransactionManager transactionManager, Metadata metadata, AccessControl accessControl, Session session, List parameters, WarningCollector warningCollector, String query)
        -Type getColumnType(SetColumnType statement)
    }

    class Metadata {
        +Optional getTableHandle(Session session, QualifiedObjectName tableName)
        +Map getColumnHandles(Session session, TableHandle tableHandle)
        +void setColumnType(Session session, TableHandle tableHandle, ColumnHandle column, Type type)
        +Type getType(TypeSignature typeSignature)
    }

    class AccessControl {
        +void checkCanAlterColumn(TransactionId transactionId, Identity identity, AccessControlContext context, QualifiedObjectName tableName)
    }

    class PrestoDataDefBindingHelper {
        -Map dataDefBuilder
    }

    class SqlBaseParser_SetColumnTypeContext {
    }

    class Optional_scope {
    }

    class TransactionManager {
    }

    class Session {
        +TransactionId getRequiredTransactionId()
        +Identity getIdentity()
        +AccessControlContext getAccessControlContext()
    }

    class WarningCollector {
    }

    class ListenableFuture {
    }

    class TypeSignature {
        +static TypeSignature parseTypeSignature(String signature)
    }

    class Type {
    }

    class Identity {
    }

    class AccessControlContext {
    }

    class QualifiedObjectName {
    }

    class TableHandle {
    }

    class ColumnHandle {
    }

    class TransactionId {
    }

    class SemanticException {
    }

    class SemanticErrorCode {
    }

    Statement <|-- SetColumnType

    AstVisitor <.. SetColumnType : accept
    AstVisitor <|.. StatementAnalyzer_Visitor
    AstVisitor <|.. SqlFormatter_FormatterVisitor

    AstBuilder --> SetColumnType : creates
    AstBuilder --> SqlBaseParser_SetColumnTypeContext

    SqlFormatter_FormatterVisitor --> SetColumnType : formats

    StatementAnalyzer_Visitor --> SetColumnType : analyzes

    StatementUtils --> SetColumnType : mapsToQueryType

    SetColumnTypeTask ..|> DDLDefinitionTask_SetColumnType
    SetColumnTypeTask --> Metadata : uses
    SetColumnTypeTask --> AccessControl : uses
    SetColumnTypeTask --> Session : uses
    SetColumnTypeTask --> SetColumnType : executes
    SetColumnTypeTask --> TableHandle : resolves
    SetColumnTypeTask --> ColumnHandle : resolves
    SetColumnTypeTask --> Type : parses

    Metadata <.. PrestoDataDefBindingHelper : referenced

    TypeSignature <.. SetColumnTypeTask : parseTypeSignature

    class DDLDefinitionTask_SetColumnType {
        <<interface>>
        +String getName()
        +ListenableFuture execute(SetColumnType statement, TransactionManager transactionManager, Metadata metadata, AccessControl accessControl, Session session, List parameters, WarningCollector warningCollector, String query)
    }
Loading

Class diagram for connector metadata setColumnType implementations

classDiagram
    class Metadata {
        +void setColumnType(Session session, TableHandle tableHandle, ColumnHandle column, Type type)
    }

    class MetadataManager {
        +void setColumnType(Session session, TableHandle tableHandle, ColumnHandle column, Type type)
    }

    class MetadataManagerStats {
        -AtomicLong setColumnTypeCalls
        -TimeStat setColumnTypeTime
        +void recordSetColumnTypeCall(long duration)
    }

    class StatsRecordingMetadataManager {
        -Metadata delegate
        -MetadataManagerStats stats
        +void setColumnType(Session session, TableHandle tableHandle, ColumnHandle column, Type type)
    }

    class ConnectorMetadata {
        +void setColumnType(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnHandle column, Type type)
    }

    class ClassLoaderSafeConnectorMetadata {
        -ConnectorMetadata delegate
        -ClassLoader classLoader
        +void setColumnType(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnHandle columnHandle, Type type)
    }

    class BlackHoleMetadata {
        -Map tables
        +void setColumnType(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnHandle columnHandle, Type type)
    }

    class BlackHoleTableHandle {
        +List getColumnHandles()
        +String getSchemaName()
        +String getTableName()
        +int getSplitCount()
        +int getPagesPerSplit()
        +int getRowsPerPage()
        +int getFieldsLength()
        +Duration getPageProcessingDelay()
    }

    class BlackHoleColumnHandle {
        +String getName()
    }

    class IcebergAbstractMetadata {
        +void setColumnType(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnHandle columnHandle, Type type)
    }

    class IcebergTableHandle {
        +SchemaTableName getSchemaTableName()
    }

    class TestingMetadata {
        +void setColumnType(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnHandle column, Type type)
    }

    class RemoteMetadataManager {
        +void setColumnType(Session session, TableHandle tableHandle, ColumnHandle column, Type type)
    }

    class ConnectorSession {
    }

    class TableHandle {
        +ConnectorId getConnectorId()
        +ConnectorTableHandle getConnectorHandle()
    }

    class ConnectorTableHandle {
    }

    class ColumnHandle {
    }

    class Session {
        +ConnectorSession toConnectorSession(ConnectorId connectorId)
    }

    class ConnectorId {
    }

    class Type {
    }

    class Table {
        +UpdateSchema updateSchema()
    }

    class UpdateSchema {
        +UpdateSchema updateColumn(String name, Type type)
        +void commit()
    }

    class IcebergErrorCode {
        +ICEBERG_INCOMPATIBLE_COLUMN_TYPE
    }

    class PrestoException {
    }

    class AtomicLong {
    }

    class TimeStat {
        +void add(long duration, TimeUnit unit)
    }

    class TimeUnit {
    }

    Metadata <|.. MetadataManager
    MetadataManager --> ConnectorMetadata : getMetadataForWrite

    MetadataManager --> MetadataManagerStats : uses
    MetadataManagerStats <.. StatsRecordingMetadataManager

    Metadata <|.. StatsRecordingMetadataManager
    StatsRecordingMetadataManager --> Metadata : delegate

    ConnectorMetadata <|.. BlackHoleMetadata
    ConnectorMetadata <|.. IcebergAbstractMetadata
    ConnectorMetadata <|.. TestingMetadata

    MetadataManager --> ConnectorMetadata : setColumnType

    ClassLoaderSafeConnectorMetadata --> ConnectorMetadata : delegate

    BlackHoleMetadata --> BlackHoleTableHandle : casts
    BlackHoleMetadata --> BlackHoleColumnHandle : uses

    IcebergAbstractMetadata --> IcebergTableHandle : casts
    IcebergAbstractMetadata --> Table : getIcebergTable
    IcebergAbstractMetadata --> IcebergErrorCode : uses
    IcebergAbstractMetadata --> PrestoException : throws

    TestingMetadata --> ConnectorTableMetadata : uses

    RemoteMetadataManager ..|> Metadata

    class ConnectorTableMetadata {
        +SchemaTableName getTable()
        +List getColumns()
        +Map getProperties()
        +Optional getComment()
    }
Loading

File-Level Changes

Change Details Files
Introduce SetColumnType statement and execution path to implement ALTER TABLE ... ALTER COLUMN ... SET DATA TYPE.
  • Add SetColumnType AST node, visitor support, and parsing rule in SqlBase.g4 and AstBuilder
  • Extend SqlFormatter to pretty-print ALTER COLUMN SET DATA TYPE statements
  • Register SetColumnType as a DATA_DEFINITION statement and bind it to SetColumnTypeTask via PrestoDataDefBindingHelper
  • Implement SetColumnTypeTask to resolve table/column, perform access control checks, resolve target type, and delegate to Metadata.setColumnType, including materialized view and IF EXISTS handling
  • Add unit tests for SetColumnTypeTask including table existence, column existence, and idempotent same-type updates
presto-parser/src/main/antlr4/com/facebook/presto/sql/parser/SqlBase.g4
presto-parser/src/main/java/com/facebook/presto/sql/parser/AstBuilder.java
presto-parser/src/main/java/com/facebook/presto/sql/tree/SetColumnType.java
presto-parser/src/main/java/com/facebook/presto/sql/tree/AstVisitor.java
presto-parser/src/main/java/com/facebook/presto/sql/SqlFormatter.java
presto-analyzer/src/main/java/com/facebook/presto/sql/analyzer/utils/StatementUtils.java
presto-main-base/src/main/java/com/facebook/presto/execution/SetColumnTypeTask.java
presto-main-base/src/main/java/com/facebook/presto/util/PrestoDataDefBindingHelper.java
presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/StatementAnalyzer.java
presto-main-base/src/test/java/com/facebook/presto/execution/TestSetColumnTypeTask.java
Wire column type alteration through Metadata, connector metadata, and stats/forwarding layers.
  • Extend Metadata interface and MetadataManager/StatsRecordingMetadataManager/RemoteMetadataManager to expose setColumnType
  • Add default setColumnType implementation to ConnectorMetadata and classloader-safe wrapper
  • Update AbstractMockMetadata and TestingMetadata to implement or stub setColumnType
  • Track setColumnType metrics in MetadataManagerStats
presto-main-base/src/main/java/com/facebook/presto/metadata/Metadata.java
presto-main-base/src/main/java/com/facebook/presto/metadata/MetadataManager.java
presto-main-base/src/main/java/com/facebook/presto/metadata/StatsRecordingMetadataManager.java
presto-main-base/src/main/java/com/facebook/presto/metadata/MetadataManagerStats.java
presto-main-base/src/main/java/com/facebook/presto/catalogserver/RemoteMetadataManager.java
presto-main-base/src/test/java/com/facebook/presto/metadata/AbstractMockMetadata.java
presto-main-base/src/main/java/com/facebook/presto/testing/TestingMetadata.java
presto-spi/src/main/java/com/facebook/presto/spi/connector/ConnectorMetadata.java
presto-spi/src/main/java/com/facebook/presto/spi/connector/classloader/ClassLoaderSafeConnectorMetadata.java
presto-main-base/src/test/java/com/facebook/presto/connector/MockConnectorFactory.java
Add connector-specific implementations for ALTER COLUMN type, particularly for Iceberg and BlackHole, and adjust ORC float reader and DoubleType for REAL->DOUBLE compatibility.
  • Implement IcebergAbstractMetadata.setColumnType to translate Presto type to Iceberg and update schema, with ICEBERG_INCOMPATIBLE_COLUMN_TYPE error handling
  • Implement BlackHoleMetadata.setColumnType to update in-memory table handles
  • Add TestingMetadata.setColumnType stub to keep tests compiling
  • Allow FloatBatchStreamReader to accept REAL or DOUBLE and update DoubleType.getObjectValue to handle IntArrayBlock (REAL stored as int bits)
  • Add new ICEBERG_INCOMPATIBLE_COLUMN_TYPE error code
presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergAbstractMetadata.java
presto-iceberg/src/main/java/com/facebook/presto/iceberg/IcebergErrorCode.java
presto-blackhole/src/main/java/com/facebook/presto/plugin/blackhole/BlackHoleMetadata.java
presto-main-base/src/main/java/com/facebook/presto/testing/TestingMetadata.java
presto-orc/src/main/java/com/facebook/presto/orc/reader/FloatBatchStreamReader.java
presto-common/src/main/java/com/facebook/presto/common/type/DoubleType.java
Introduce access-control APIs and enforcement for ALTER COLUMN operations at system and connector levels, plus testing hooks.
  • Add checkCanAlterColumn to AccessControl, AccessControlManager, TestingAccessControlManager, and DenyAll/AllowAll implementations
  • Add corresponding system-level method to SystemAccessControl and wrapper StatsRecordingSystemAccessControl with metrics
  • Implement connector-level checkCanAlterColumn in SqlStandardAccessControl, FileBasedAccessControl, Forwarding access controls, AllowAll/ReadOnly/Legacy access controls
  • Add ALTER_COLUMN to TestingPrivilegeType and wire access tests in AbstractTestDistributedQueries and TestFileBasedAccessControl
presto-spi/src/main/java/com/facebook/presto/spi/security/AccessControl.java
presto-main-base/src/main/java/com/facebook/presto/security/AccessControlManager.java
presto-main-base/src/main/java/com/facebook/presto/security/StatsRecordingSystemAccessControl.java
presto-main-base/src/main/java/com/facebook/presto/security/AllowAllSystemAccessControl.java
presto-main-base/src/main/java/com/facebook/presto/security/FileBasedSystemAccessControl.java
presto-spi/src/main/java/com/facebook/presto/spi/security/SystemAccessControl.java
presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/FileBasedAccessControl.java
presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ForwardingConnectorAccessControl.java
presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ForwardingSystemAccessControl.java
presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/AllowAllAccessControl.java
presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ReadOnlyAccessControl.java
presto-hive/src/main/java/com/facebook/presto/hive/security/SqlStandardAccessControl.java
presto-hive/src/main/java/com/facebook/presto/hive/security/LegacyAccessControl.java
presto-spi/src/main/java/com/facebook/presto/spi/security/AllowAllAccessControl.java
presto-spi/src/main/java/com/facebook/presto/spi/security/DenyAllAccessControl.java
presto-main-base/src/main/java/com/facebook/presto/testing/TestingAccessControlManager.java
presto-main-base/src/test/java/com/facebook/presto/tests/AbstractTestDistributedQueries.java
presto-plugin-toolkit/src/test/java/com/facebook/presto/plugin/base/security/TestFileBasedAccessControl.java
presto-main-base/src/main/java/com/facebook/presto/testing/TestingAccessControlManager.java
Extend parser and metadata tests, including Iceberg smoke tests, to validate ALTER COLUMN type behavior and ensure property handling correctness.
  • Add IcebergDistributedSmokeTestBase.testAlterColumnType to exercise INTEGER->BIGINT, SMALLINT->INTEGER, and REAL->DOUBLE transitions across file formats and validate SHOW CREATE TABLE output and data integrity
  • Fix property parsing in IcebergDistributedSmokeTestBase to extract StringLiteral values without quotes when building expected property maps
  • Add parser tests for ALTER TABLE ... ALTER COLUMN ... SET DATA TYPE and statement builder print tests for the new syntax
  • Include ALTER TABLE docs stub and ensure alter-table docs file exists
presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergDistributedSmokeTestBase.java
presto-parser/src/test/java/com/facebook/presto/sql/parser/TestSqlParser.java
presto-parser/src/test/java/com/facebook/presto/sql/parser/TestStatementBuilder.java
presto-docs/src/main/sphinx/sql/alter-table.rst

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

from:IBM PR from IBM

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants