Skip to content

Commit 4c6319e

Browse files
ebyhrebyhr
committed
Support setting a field type with SET DATA TYPE in Iceberg
1 parent 0c37357 commit 4c6319e

File tree

6 files changed

+245
-0
lines changed

6 files changed

+245
-0
lines changed

‎plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1839,6 +1839,36 @@ private static boolean fieldExists(StructType structType, String fieldName)
18391839
return false;
18401840
}
18411841

1842+
@Override
1843+
public void setFieldType(ConnectorSession session, ConnectorTableHandle tableHandle, List<String> fieldPath, io.trino.spi.type.Type type)
1844+
{
1845+
Table icebergTable = catalog.loadTable(session, ((IcebergTableHandle) tableHandle).getSchemaTableName());
1846+
String parentPath = String.join(".", fieldPath.subList(0, fieldPath.size() - 1));
1847+
NestedField parent = icebergTable.schema().caseInsensitiveFindField(parentPath);
1848+
1849+
String caseSensitiveParentName = icebergTable.schema().findColumnName(parent.fieldId());
1850+
NestedField field = parent.type().asStructType().caseInsensitiveField(getLast(fieldPath));
1851+
// TODO: Add support for changing non-primitive field type
1852+
if (!field.type().isPrimitiveType()) {
1853+
throw new TrinoException(NOT_SUPPORTED, "Iceberg doesn't support changing field type from non-primitive types");
1854+
}
1855+
1856+
String name = caseSensitiveParentName + "." + field.name();
1857+
// Pass dummy AtomicInteger. The field id will be discarded because the subsequent logic disallows non-primitive types.
1858+
Type icebergType = toIcebergTypeForNewColumn(type, new AtomicInteger());
1859+
if (!icebergType.isPrimitiveType()) {
1860+
throw new TrinoException(NOT_SUPPORTED, "Iceberg doesn't support changing field type to non-primitive types");
1861+
}
1862+
try {
1863+
icebergTable.updateSchema()
1864+
.updateColumn(name, icebergType.asPrimitiveType())
1865+
.commit();
1866+
}
1867+
catch (RuntimeException e) {
1868+
throw new TrinoException(ICEBERG_COMMIT_ERROR, "Failed to set field type: " + firstNonNull(e.getMessage(), e), e);
1869+
}
1870+
}
1871+
18421872
private List<ColumnMetadata> getColumnMetadatas(Schema schema)
18431873
{
18441874
ImmutableList.Builder<ColumnMetadata> columns = ImmutableList.builder();

‎plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6951,6 +6951,48 @@ protected void verifySetColumnTypeFailurePermissible(Throwable e)
69516951
"|Type not supported for Iceberg: char\\(20\\)).*");
69526952
}
69536953

6954+
@Override
6955+
protected Optional<SetColumnTypeSetup> filterSetFieldTypesDataProvider(SetColumnTypeSetup setup)
6956+
{
6957+
switch ("%s -> %s".formatted(setup.sourceColumnType(), setup.newColumnType())) {
6958+
case "bigint -> integer":
6959+
case "decimal(5,3) -> decimal(5,2)":
6960+
case "varchar -> char(20)":
6961+
case "time(6) -> time(3)":
6962+
case "timestamp(6) -> timestamp(3)":
6963+
case "array(integer) -> array(bigint)":
6964+
case "row(x integer) -> row(x bigint)":
6965+
case "row(x integer) -> row(y integer)":
6966+
case "row(x integer, y integer) -> row(x integer, z integer)":
6967+
case "row(x integer) -> row(x integer, y integer)":
6968+
case "row(x integer, y integer) -> row(x integer)":
6969+
case "row(x integer, y integer) -> row(y integer, x integer)":
6970+
case "row(x integer, y integer) -> row(z integer, y integer, x integer)":
6971+
case "row(x row(nested integer)) -> row(x row(nested bigint))":
6972+
case "row(x row(a integer, b integer)) -> row(x row(b integer, a integer))":
6973+
// Iceberg allows updating column types if the update is safe. Safe updates are:
6974+
// - int to bigint
6975+
// - float to double
6976+
// - decimal(P,S) to decimal(P2,S) when P2 > P (scale cannot change)
6977+
// https://iceberg.apache.org/docs/latest/spark-ddl/#alter-table--alter-column
6978+
return Optional.of(setup.asUnsupported());
6979+
6980+
case "varchar(100) -> varchar(50)":
6981+
// Iceberg connector ignores the varchar length
6982+
return Optional.empty();
6983+
}
6984+
return Optional.of(setup);
6985+
}
6986+
6987+
@Override
6988+
protected void verifySetFieldTypeFailurePermissible(Throwable e)
6989+
{
6990+
assertThat(e).hasMessageMatching(".*(Failed to set field type: Cannot change (column type:|type from .* to )" +
6991+
"|Time(stamp)? precision \\(3\\) not supported for Iceberg. Use \"time(stamp)?\\(6\\)\" instead" +
6992+
"|Type not supported for Iceberg: char\\(20\\)" +
6993+
"|Iceberg doesn't support changing field type (from|to) non-primitive types).*");
6994+
}
6995+
69546996
@Override
69556997
protected boolean supportsPhysicalPushdown()
69566998
{

‎plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/TestMongoConnectorTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ protected boolean hasBehavior(TestingConnectorBehavior connectorBehavior)
113113
case SUPPORTS_ADD_FIELD:
114114
case SUPPORTS_RENAME_FIELD:
115115
case SUPPORTS_DROP_FIELD:
116+
case SUPPORTS_SET_FIELD_TYPE:
116117
return false;
117118

118119
case SUPPORTS_CREATE_VIEW:

‎testing/trino-product-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergSparkCompatibility.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2748,6 +2748,26 @@ private void testTrinoSetColumnType(boolean partitioned, StorageFormat storageFo
27482748
onTrino().executeQuery("DROP TABLE " + trinoTableName);
27492749
}
27502750

2751+
@Test(groups = {ICEBERG, PROFILE_SPECIFIC_TESTS}, dataProvider = "testSetColumnTypeDataProvider")
2752+
public void testTrinoSetFieldType(StorageFormat storageFormat, String sourceFieldType, String sourceValueLiteral, String newFieldType, Object newValue)
2753+
{
2754+
String baseTableName = "test_set_field_type_" + randomNameSuffix();
2755+
String trinoTableName = trinoTableName(baseTableName);
2756+
String sparkTableName = sparkTableName(baseTableName);
2757+
2758+
onTrino().executeQuery("CREATE TABLE " + trinoTableName + " " +
2759+
"WITH (format = '" + storageFormat + "')" +
2760+
"AS SELECT CAST(row(" + sourceValueLiteral + ") AS row(field " + sourceFieldType + ")) AS col");
2761+
2762+
onTrino().executeQuery("ALTER TABLE " + trinoTableName + " ALTER COLUMN col.field SET DATA TYPE " + newFieldType);
2763+
2764+
assertEquals(getColumnType(baseTableName, "col"), "row(field " + newFieldType + ")");
2765+
assertThat(onTrino().executeQuery("SELECT col.field FROM " + trinoTableName)).containsOnly(row(newValue));
2766+
assertThat(onSpark().executeQuery("SELECT col.field FROM " + sparkTableName)).containsOnly(row(newValue));
2767+
2768+
onTrino().executeQuery("DROP TABLE " + trinoTableName);
2769+
}
2770+
27512771
@DataProvider
27522772
public static Object[][] testSetColumnTypeDataProvider()
27532773
{

‎testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@
147147
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_ROW_LEVEL_DELETE;
148148
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_ROW_TYPE;
149149
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_SET_COLUMN_TYPE;
150+
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_SET_FIELD_TYPE;
150151
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_TOPN_PUSHDOWN;
151152
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_TRUNCATE;
152153
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_UPDATE;
@@ -3041,6 +3042,156 @@ protected void verifySetColumnTypeFailurePermissible(Throwable e)
30413042
throw new AssertionError("Unexpected set column type failure", e);
30423043
}
30433044

3045+
@Test
3046+
public void testSetFieldType()
3047+
{
3048+
skipTestUnless(hasBehavior(SUPPORTS_CREATE_TABLE_WITH_DATA) && hasBehavior(SUPPORTS_ROW_TYPE));
3049+
3050+
if (!hasBehavior(SUPPORTS_SET_FIELD_TYPE)) {
3051+
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_set_field_type_", "(col row(field int))")) {
3052+
assertQueryFails(
3053+
"ALTER TABLE " + table.getName() + " ALTER COLUMN col.field SET DATA TYPE bigint",
3054+
"This connector does not support setting field types");
3055+
}
3056+
return;
3057+
}
3058+
3059+
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_set_field_type_", "AS SELECT CAST(row(123) AS row(field integer)) AS col")) {
3060+
assertEquals(getColumnType(table.getName(), "col"), "row(field integer)");
3061+
3062+
assertUpdate("ALTER TABLE " + table.getName() + " ALTER COLUMN col.field SET DATA TYPE bigint");
3063+
3064+
assertEquals(getColumnType(table.getName(), "col"), "row(field bigint)");
3065+
assertThat(query("SELECT * FROM " + table.getName()))
3066+
.skippingTypesCheck()
3067+
.matches("SELECT row(bigint '123')");
3068+
}
3069+
}
3070+
3071+
@Test(dataProvider = "setFieldTypesDataProvider")
3072+
public void testSetFieldTypes(SetColumnTypeSetup setup)
3073+
{
3074+
skipTestUnless(hasBehavior(SUPPORTS_SET_FIELD_TYPE) && hasBehavior(SUPPORTS_CREATE_TABLE_WITH_DATA));
3075+
3076+
TestTable table;
3077+
try {
3078+
table = new TestTable(
3079+
getQueryRunner()::execute,
3080+
"test_set_field_type_",
3081+
" AS SELECT CAST(row(" + setup.sourceValueLiteral + ") AS row(field " + setup.sourceColumnType + ")) AS col");
3082+
}
3083+
catch (Exception e) {
3084+
verifyUnsupportedTypeException(e, setup.sourceColumnType);
3085+
throw new SkipException("Unsupported column type: " + setup.sourceColumnType);
3086+
}
3087+
try (table) {
3088+
Runnable setFieldType = () -> assertUpdate("ALTER TABLE " + table.getName() + " ALTER COLUMN col.field SET DATA TYPE " + setup.newColumnType);
3089+
if (setup.unsupportedType) {
3090+
assertThatThrownBy(setFieldType::run)
3091+
.satisfies(this::verifySetFieldTypeFailurePermissible);
3092+
return;
3093+
}
3094+
setFieldType.run();
3095+
3096+
assertEquals(getColumnType(table.getName(), "col"), "row(field " + setup.newColumnType + ")");
3097+
assertThat(query("SELECT * FROM " + table.getName()))
3098+
.skippingTypesCheck()
3099+
.matches("SELECT row(" + setup.newValueLiteral + ")");
3100+
}
3101+
}
3102+
3103+
@DataProvider
3104+
public Object[][] setFieldTypesDataProvider()
3105+
{
3106+
return setColumnTypeSetupData().stream()
3107+
.map(this::filterSetFieldTypesDataProvider)
3108+
.flatMap(Optional::stream)
3109+
.collect(toDataProvider());
3110+
}
3111+
3112+
protected Optional<SetColumnTypeSetup> filterSetFieldTypesDataProvider(SetColumnTypeSetup setup)
3113+
{
3114+
return Optional.of(setup);
3115+
}
3116+
3117+
@Test
3118+
public void testSetFieldTypeCaseSensitivity()
3119+
{
3120+
skipTestUnless(hasBehavior(SUPPORTS_SET_FIELD_TYPE) && hasBehavior(SUPPORTS_NOT_NULL_CONSTRAINT));
3121+
3122+
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_set_field_type_case_", " AS SELECT CAST(row(1) AS row(\"UPPER\" integer)) col")) {
3123+
assertEquals(getColumnType(table.getName(), "col"), "row(UPPER integer)");
3124+
3125+
assertUpdate("ALTER TABLE " + table.getName() + " ALTER COLUMN col.upper SET DATA TYPE bigint");
3126+
assertEquals(getColumnType(table.getName(), "col"), "row(UPPER bigint)");
3127+
assertThat(query("SELECT * FROM " + table.getName()))
3128+
.matches("SELECT CAST(row(1) AS row(UPPER bigint))");
3129+
}
3130+
}
3131+
3132+
@Test
3133+
public void testSetFieldTypeWithNotNull()
3134+
{
3135+
skipTestUnless(hasBehavior(SUPPORTS_SET_FIELD_TYPE) && hasBehavior(SUPPORTS_NOT_NULL_CONSTRAINT));
3136+
3137+
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_set_field_type_null_", "(col row(field int) NOT NULL)")) {
3138+
assertFalse(columnIsNullable(table.getName(), "col"));
3139+
3140+
assertUpdate("ALTER TABLE " + table.getName() + " ALTER COLUMN col.field SET DATA TYPE bigint");
3141+
assertFalse(columnIsNullable(table.getName(), "col"));
3142+
}
3143+
}
3144+
3145+
@Test
3146+
public void testSetFieldTypeWithComment()
3147+
{
3148+
skipTestUnless(hasBehavior(SUPPORTS_SET_FIELD_TYPE) && hasBehavior(SUPPORTS_CREATE_TABLE_WITH_COLUMN_COMMENT));
3149+
3150+
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_set_field_type_comment_", "(col row(field int) COMMENT 'test comment')")) {
3151+
assertEquals(getColumnComment(table.getName(), "col"), "test comment");
3152+
3153+
assertUpdate("ALTER TABLE " + table.getName() + " ALTER COLUMN col.field SET DATA TYPE bigint");
3154+
assertEquals(getColumnComment(table.getName(), "col"), "test comment");
3155+
}
3156+
}
3157+
3158+
@Test
3159+
public void testSetFieldIncompatibleType()
3160+
{
3161+
skipTestUnless(hasBehavior(SUPPORTS_SET_FIELD_TYPE) && hasBehavior(SUPPORTS_CREATE_TABLE_WITH_DATA));
3162+
3163+
try (TestTable table = new TestTable(
3164+
getQueryRunner()::execute,
3165+
"test_set_invalid_field_type_",
3166+
"(row_col row(field varchar), nested_col row(field row(nested int)))")) {
3167+
assertThatThrownBy(() -> assertUpdate("ALTER TABLE " + table.getName() + " ALTER COLUMN row_col.field SET DATA TYPE row(nested integer)"))
3168+
.satisfies(this::verifySetFieldTypeFailurePermissible);
3169+
assertThatThrownBy(() -> assertUpdate("ALTER TABLE " + table.getName() + " ALTER COLUMN row_col.field SET DATA TYPE integer"))
3170+
.satisfies(this::verifySetFieldTypeFailurePermissible);
3171+
assertThatThrownBy(() -> assertUpdate("ALTER TABLE " + table.getName() + " ALTER COLUMN nested_col.field SET DATA TYPE integer"))
3172+
.satisfies(this::verifySetFieldTypeFailurePermissible);
3173+
}
3174+
}
3175+
3176+
@Test
3177+
public void testSetFieldOutOfRangeType()
3178+
{
3179+
skipTestUnless(hasBehavior(SUPPORTS_SET_FIELD_TYPE) && hasBehavior(SUPPORTS_CREATE_TABLE_WITH_DATA));
3180+
3181+
try (TestTable table = new TestTable(
3182+
getQueryRunner()::execute,
3183+
"test_set_field_type_invalid_range_",
3184+
"AS SELECT CAST(row(9223372036854775807) AS row(field bigint)) AS col")) {
3185+
assertThatThrownBy(() -> assertUpdate("ALTER TABLE " + table.getName() + " ALTER COLUMN col.field SET DATA TYPE integer"))
3186+
.satisfies(this::verifySetFieldTypeFailurePermissible);
3187+
}
3188+
}
3189+
3190+
protected void verifySetFieldTypeFailurePermissible(Throwable e)
3191+
{
3192+
throw new AssertionError("Unexpected set field type failure", e);
3193+
}
3194+
30443195
protected String getColumnType(String tableName, String columnName)
30453196
{
30463197
return (String) computeScalar(format("SELECT data_type FROM information_schema.columns WHERE table_schema = CURRENT_SCHEMA AND table_name = '%s' AND column_name = '%s'",

‎testing/trino-testing/src/main/java/io/trino/testing/TestingConnectorBehavior.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ public enum TestingConnectorBehavior
8888
SUPPORTS_RENAME_COLUMN,
8989
SUPPORTS_RENAME_FIELD(fallback -> fallback.test(SUPPORTS_RENAME_COLUMN) && fallback.test(SUPPORTS_ROW_TYPE)),
9090
SUPPORTS_SET_COLUMN_TYPE,
91+
SUPPORTS_SET_FIELD_TYPE(fallback -> fallback.test(SUPPORTS_SET_COLUMN_TYPE) && fallback.test(SUPPORTS_ROW_TYPE)),
9192

9293
SUPPORTS_COMMENT_ON_TABLE,
9394
SUPPORTS_COMMENT_ON_COLUMN(SUPPORTS_COMMENT_ON_TABLE),

0 commit comments

Comments
 (0)