Skip to content

Support tinyint/smallint to integer type coercion in Iceberg #23033

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

Merged
ebyhr merged 1 commit into from
Aug 14, 2024
Merged

Support tinyint/smallint to integer type coercion in Iceberg #23033

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,10 +297,13 @@
import static io.trino.spi.type.BigintType.BIGINT;
import static io.trino.spi.type.DateTimeEncoding.unpackMillisUtc;
import static io.trino.spi.type.DateType.DATE;
import static io.trino.spi.type.IntegerType.INTEGER;
import static io.trino.spi.type.SmallintType.SMALLINT;
import static io.trino.spi.type.TimeType.TIME_MICROS;
import static io.trino.spi.type.TimestampType.TIMESTAMP_MICROS;
import static io.trino.spi.type.TimestampWithTimeZoneType.TIMESTAMP_TZ_MICROS;
import static io.trino.spi.type.Timestamps.MICROSECONDS_PER_MILLISECOND;
import static io.trino.spi.type.TinyintType.TINYINT;
import static io.trino.spi.type.VarcharType.VARCHAR;
import static java.lang.Math.floorDiv;
import static java.lang.String.format;
Expand Down Expand Up @@ -953,6 +956,9 @@ public Optional<io.trino.spi.type.Type> getSupportedType(ConnectorSession sessio

private io.trino.spi.type.Type coerceType(io.trino.spi.type.Type type)
{
if (type == TINYINT || type == SMALLINT) {
return INTEGER;
}
if (type instanceof TimestampWithTimeZoneType) {
return TIMESTAMP_TZ_MICROS;
}
Expand Down
38 changes: 31 additions & 7 deletions plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5185,11 +5185,6 @@ protected TestTable createTableWithDefaultColumns()
protected Optional<DataMappingTestSetup> filterDataMappingSmokeTestData(DataMappingTestSetup dataMappingTestSetup)
{
String typeName = dataMappingTestSetup.getTrinoTypeName();
if (typeName.equals("tinyint")
|| typeName.equals("smallint")) {
// These types are not supported by Iceberg
return Optional.of(dataMappingTestSetup.asUnsupported());
}
if (typeName.equals("char(3)")) {
// Use explicitly padded literal in char mapping test due to whitespace padding on coercion to varchar
return Optional.of(new DataMappingTestSetup(typeName, "'ab '", dataMappingTestSetup.getHighValueLiteral()));
Expand Down Expand Up @@ -8169,6 +8164,8 @@ protected Optional<TypeCoercionTestSetup> filterTypeCoercionOnCreateTableAsSelec
private List<TypeCoercionTestSetup> typeCoercionOnCreateTableAsSelectData()
{
return ImmutableList.<TypeCoercionTestSetup>builder()
.add(new TypeCoercionTestSetup("TINYINT '127'", "integer", "INTEGER '127'"))
.add(new TypeCoercionTestSetup("SMALLINT '32767'", "integer", "INTEGER '32767'"))
.add(new TypeCoercionTestSetup("TIMESTAMP '1970-01-01 00:00:00'", "timestamp(6)", "TIMESTAMP '1970-01-01 00:00:00.000000'"))
.add(new TypeCoercionTestSetup("TIMESTAMP '1970-01-01 00:00:00.9'", "timestamp(6)", "TIMESTAMP '1970-01-01 00:00:00.900000'"))
.add(new TypeCoercionTestSetup("TIMESTAMP '1970-01-01 00:00:00.56'", "timestamp(6)", "TIMESTAMP '1970-01-01 00:00:00.560000'"))
Expand Down Expand Up @@ -8230,6 +8227,20 @@ private List<TypeCoercionTestSetup> typeCoercionOnCreateTableAsSelectData()
// TODO Add test case for MAP type with ARRAY keys once https://github.com/trinodb/trino/issues/1146 is resolved
.add(new TypeCoercionTestSetup("CAST(ROW('a') AS ROW(x CHAR))", "row(x varchar)", "CAST(ROW('a') AS ROW(x VARCHAR))"))
.add(new TypeCoercionTestSetup("CAST(ROW(ROW('a')) AS ROW(x ROW(y CHAR)))", "row(x row(y varchar))", "CAST(ROW(ROW('a')) AS ROW(x ROW(y VARCHAR)))"))
// tinyint -> integer
.add(new TypeCoercionTestSetup("ARRAY[TINYINT '127']", "array(integer)", "ARRAY[127]"))
.add(new TypeCoercionTestSetup("ARRAY[ARRAY[TINYINT '127']]", "array(array(integer))", "ARRAY[ARRAY[127]]"))
.add(new TypeCoercionTestSetup("MAP(ARRAY[TINYINT '1'], ARRAY[TINYINT '10'])", "map(integer, integer)", "MAP(ARRAY[1], ARRAY[10])"))
.add(new TypeCoercionTestSetup("MAP(ARRAY[TINYINT '1'], ARRAY[ARRAY[TINYINT '10']])", "map(integer, array(integer))", "MAP(ARRAY[1], ARRAY[ARRAY[10]])"))
.add(new TypeCoercionTestSetup("CAST(ROW(127) AS ROW(x TINYINT))", "row(x integer)", "CAST(ROW(127) AS ROW(x INTEGER))"))
.add(new TypeCoercionTestSetup("CAST(ROW(ROW(127)) AS ROW(x ROW(y TINYINT)))", "row(x row(y integer))", "CAST(ROW(ROW(127)) AS ROW(x ROW(y INTEGER)))"))
// smallint -> integer
.add(new TypeCoercionTestSetup("ARRAY[SMALLINT '32767']", "array(integer)", "ARRAY[32767]"))
.add(new TypeCoercionTestSetup("ARRAY[ARRAY[SMALLINT '32767']]", "array(array(integer))", "ARRAY[ARRAY[32767]]"))
.add(new TypeCoercionTestSetup("MAP(ARRAY[SMALLINT '1'], ARRAY[SMALLINT '10'])", "map(integer, integer)", "MAP(ARRAY[1], ARRAY[10])"))
.add(new TypeCoercionTestSetup("MAP(ARRAY[SMALLINT '1'], ARRAY[ARRAY[SMALLINT '10']])", "map(integer, array(integer))", "MAP(ARRAY[1], ARRAY[ARRAY[10]])"))
.add(new TypeCoercionTestSetup("CAST(ROW(32767) AS ROW(x SMALLINT))", "row(x integer)", "CAST(ROW(32767) AS ROW(x INTEGER))"))
.add(new TypeCoercionTestSetup("CAST(ROW(ROW(32767)) AS ROW(x ROW(y SMALLINT)))", "row(x row(y integer))", "CAST(ROW(ROW(32767)) AS ROW(x ROW(y INTEGER)))"))
.build();
}

Expand All @@ -8251,6 +8262,9 @@ public TypeCoercionTestSetup withNewValueLiteral(String newValueLiteral)
@Test
public void testAddColumnWithTypeCoercion()
{
testAddColumnWithTypeCoercion("tinyint", "integer");
testAddColumnWithTypeCoercion("smallint", "integer");

testAddColumnWithTypeCoercion("timestamp with time zone", "timestamp(6) with time zone");
testAddColumnWithTypeCoercion("timestamp(0) with time zone", "timestamp(6) with time zone");
testAddColumnWithTypeCoercion("timestamp(1) with time zone", "timestamp(6) with time zone");
Expand Down Expand Up @@ -8301,6 +8315,14 @@ public void testAddColumnWithTypeCoercion()
testAddColumnWithTypeCoercion("array(char(10))", "array(varchar)");
testAddColumnWithTypeCoercion("map(char(20), char(30))", "map(varchar, varchar)");
testAddColumnWithTypeCoercion("row(x char(40))", "row(x varchar)");

testAddColumnWithTypeCoercion("array(tinyint)", "array(integer)");
testAddColumnWithTypeCoercion("map(tinyint, tinyint)", "map(integer, integer)");
testAddColumnWithTypeCoercion("row(x tinyint)", "row(x integer)");

testAddColumnWithTypeCoercion("array(smallint)", "array(integer)");
testAddColumnWithTypeCoercion("map(smallint, smallint)", "map(integer, integer)");
testAddColumnWithTypeCoercion("row(x smallint)", "row(x integer)");
}

private void testAddColumnWithTypeCoercion(String columnType, String expectedColumnType)
Expand Down Expand Up @@ -8342,6 +8364,7 @@ protected Optional<SetColumnTypeSetup> filterSetColumnTypesDataProvider(SetColum
return Optional.of(setup.withNewValueLiteral("TIMESTAMP '2020-02-12 14:03:00.123000 +00:00'"));
}
switch ("%s -> %s".formatted(setup.sourceColumnType(), setup.newColumnType())) {
case "tinyint -> smallint":
case "bigint -> integer":
case "decimal(5,3) -> decimal(5,2)":
case "varchar -> char(20)":
Expand All @@ -8367,13 +8390,14 @@ protected void verifySetColumnTypeFailurePermissible(Throwable e)
{
assertThat(e).hasMessageMatching(".*(Failed to set column type: Cannot change (column type:|type from .* to )" +
"|Time(stamp)? precision \\(3\\) not supported for Iceberg. Use \"time(stamp)?\\(6\\)\" instead" +
"|Type not supported for Iceberg: char\\(20\\)).*");
"|Type not supported for Iceberg: smallint|char\\(20\\)).*");
}

@Override
protected Optional<SetColumnTypeSetup> filterSetFieldTypesDataProvider(SetColumnTypeSetup setup)
{
switch ("%s -> %s".formatted(setup.sourceColumnType(), setup.newColumnType())) {
case "tinyint -> smallint":
case "bigint -> integer":
case "decimal(5,3) -> decimal(5,2)":
case "varchar -> char(20)":
Expand Down Expand Up @@ -8408,7 +8432,7 @@ protected void verifySetFieldTypeFailurePermissible(Throwable e)
{
assertThat(e).hasMessageMatching(".*(Failed to set field type: Cannot change (column type:|type from .* to )" +
"|Time(stamp)? precision \\(3\\) not supported for Iceberg. Use \"time(stamp)?\\(6\\)\" instead" +
"|Type not supported for Iceberg: char\\(20\\)" +
"|Type not supported for Iceberg: smallint|char\\(20\\)" +
"|Iceberg doesn't support changing field type (from|to) non-primitive types).*");
}

Expand Down