Skip to content

Replace return with continue in SET DATA TYPE tests #27958

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
Jan 20, 2026
+58 −36
Merged

Replace return with continue in SET DATA TYPE tests #27958

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
32 changes: 21 additions & 11 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 @@ -9379,7 +9379,10 @@ protected Optional<SetColumnTypeSetup> filterSetColumnTypesDataProvider(SetColum
switch ("%s -> %s".formatted(setup.sourceColumnType(), setup.newColumnType())) {
case "tinyint -> smallint":
case "bigint -> integer":
case "bigint -> smallint":
case "bigint -> tinyint":
case "decimal(5,3) -> decimal(5,2)":
case "char(25) -> char(20)":
case "varchar -> char(20)":
case "time(6) -> time(3)":
case "timestamp(6) -> timestamp(3)":
Expand All @@ -9403,29 +9406,36 @@ 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: smallint|char\\(20\\)).*");
"|Type not supported for Iceberg: tinyint|smallint|char\\(20\\)).*");
}

@Override
protected Optional<SetColumnTypeSetup> filterSetFieldTypesDataProvider(SetColumnTypeSetup setup)
{
if (setup.sourceColumnType().equals("timestamp(3) with time zone")) {
// The connector returns UTC instead of the given time zone
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 "bigint -> smallint":
case "bigint -> tinyint":
case "decimal(5,3) -> decimal(5,2)":
case "char(25) -> char(20)":
case "varchar -> char(20)":
case "time(6) -> time(3)":
case "timestamp(6) -> timestamp(3)":
case "array(integer) -> array(bigint)":
case "row(x integer) -> row(x bigint)":
case "row(x integer) -> row(y integer)":
case "row(x integer, y integer) -> row(x integer, z integer)":
case "row(x integer) -> row(x integer, y integer)":
case "row(x integer, y integer) -> row(x integer)":
case "row(x integer, y integer) -> row(y integer, x integer)":
case "row(x integer, y integer) -> row(z integer, y integer, x integer)":
case "row(x row(nested integer)) -> row(x row(nested bigint))":
case "row(x row(a integer, b integer)) -> row(x row(b integer, a integer))":
case "row(x integer) -> row(\"x\" bigint)":
case "row(x integer) -> row(\"y\" integer)":
case "row(x integer, y integer) -> row(\"x\" integer, \"z\" integer)":
case "row(x integer) -> row(\"x\" integer, \"y\" integer)":
case "row(x integer, y integer) -> row(\"x\" integer)":
case "row(x integer, y integer) -> row(\"y\" integer, \"x\" integer)":
case "row(x integer, y integer) -> row(\"z\" integer, \"y\" integer, \"x\" integer)":
case "row(x row(nested integer)) -> row(\"x\" row(\"nested\" bigint))":
case "row(x row(a integer, b integer)) -> row(\"x\" row(\"b\" integer, \"a\" integer))":
// Iceberg allows updating column types if the update is safe. Safe updates are:
// - int to bigint
// - float to double
Expand All @@ -9445,7 +9455,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: smallint|char\\(20\\)" +
"|Type not supported for Iceberg: tinyint|smallint|char\\(20\\)" +
"|Iceberg doesn't support changing field type (from|to) non-primitive types).*");
}

Expand Down
2 changes: 1 addition & 1 deletion .../trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergParquetConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public void testRowGroupResetDictionary()
protected Optional<SetColumnTypeSetup> filterSetColumnTypesDataProvider(SetColumnTypeSetup setup)
{
switch ("%s -> %s".formatted(setup.sourceColumnType(), setup.newColumnType())) {
case "row(x integer) -> row(y integer)":
case "row(x integer) -> row(\"y\" integer)":
// TODO https://github.com/trinodb/trino/issues/15822 The connector returns incorrect NULL when a field in row type doesn't exist in Parquet files
return Optional.of(setup.withNewValueLiteral("NULL"));
}
Expand Down
34 changes: 22 additions & 12 deletions ...n/trino-lakehouse/src/test/java/io/trino/plugin/lakehouse/TestLakehouseConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,15 +205,15 @@ 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: smallint|char\\(20\\)).*");
"|Type not supported for Iceberg: tinyint|smallint|char\\(20\\)).*");
}

@Override
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: smallint|char\\(20\\)" +
"|Type not supported for Iceberg: tinyint|smallint|char\\(20\\)" +
"|Iceberg doesn't support changing field type (from|to) non-primitive types).*");
}

Expand Down Expand Up @@ -245,11 +245,14 @@ 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 "row(x integer) -> row(y integer)":
case "row(x integer) -> row(\"y\" integer)":
return Optional.of(setup.withNewValueLiteral("NULL"));
case "tinyint -> smallint":
case "bigint -> integer":
case "bigint -> smallint":
case "bigint -> tinyint":
case "decimal(5,3) -> decimal(5,2)":
case "char(25) -> char(20)":
case "varchar -> char(20)":
case "time(6) -> time(3)":
case "timestamp(6) -> timestamp(3)":
Expand All @@ -264,23 +267,30 @@ protected Optional<SetColumnTypeSetup> filterSetColumnTypesDataProvider(SetColum
@Override
protected Optional<SetColumnTypeSetup> filterSetFieldTypesDataProvider(SetColumnTypeSetup setup)
{
if (setup.sourceColumnType().equals("timestamp(3) with time zone")) {
// The connector returns UTC instead of the given time zone
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 "bigint -> smallint":
case "bigint -> tinyint":
case "decimal(5,3) -> decimal(5,2)":
case "char(25) -> char(20)":
case "varchar -> char(20)":
case "time(6) -> time(3)":
case "timestamp(6) -> timestamp(3)":
case "array(integer) -> array(bigint)":
case "row(x integer) -> row(x bigint)":
case "row(x integer) -> row(y integer)":
case "row(x integer, y integer) -> row(x integer, z integer)":
case "row(x integer) -> row(x integer, y integer)":
case "row(x integer, y integer) -> row(x integer)":
case "row(x integer, y integer) -> row(y integer, x integer)":
case "row(x integer, y integer) -> row(z integer, y integer, x integer)":
case "row(x row(nested integer)) -> row(x row(nested bigint))":
case "row(x row(a integer, b integer)) -> row(x row(b integer, a integer))":
case "row(x integer) -> row(\"x\" bigint)":
case "row(x integer) -> row(\"y\" integer)":
case "row(x integer, y integer) -> row(\"x\" integer, \"z\" integer)":
case "row(x integer) -> row(\"x\" integer, \"y\" integer)":
case "row(x integer, y integer) -> row(\"x\" integer)":
case "row(x integer, y integer) -> row(\"y\" integer, \"x\" integer)":
case "row(x integer, y integer) -> row(\"z\" integer, \"y\" integer, \"x\" integer)":
case "row(x row(nested integer)) -> row(\"x\" row(\"nested\" bigint))":
case "row(x row(a integer, b integer)) -> row(\"x\" row(\"b\" integer, \"a\" integer))":
return Optional.of(setup.asUnsupported());
case "varchar(100) -> varchar(50)":
return Optional.empty();
Expand Down
2 changes: 2 additions & 0 deletions plugin/trino-mongodb/src/test/java/io/trino/plugin/mongodb/TestMongoConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1920,6 +1920,8 @@ protected Optional<SetColumnTypeSetup> filterSetColumnTypesDataProvider(SetColum
{
switch ("%s -> %s".formatted(setup.sourceColumnType(), setup.newColumnType())) {
case "bigint -> integer":
case "bigint -> smallint":
case "bigint -> tinyint":
case "decimal(5,3) -> decimal(5,2)":
case "time(3) -> time(6)":
case "time(6) -> time(3)":
Expand Down
24 changes: 12 additions & 12 deletions testing/trino-testing/src/main/java/io/trino/testing/BaseConnectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3440,7 +3440,7 @@ public void testSetColumnTypes()
if (setup.unsupportedType) {
assertThatThrownBy(setColumnType::run)
.satisfies(this::verifySetColumnTypeFailurePermissible);
return;
Copy link
Member

@findepi findepi Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good find!

continue;
}
setColumnType.run();

Expand Down Expand Up @@ -3496,15 +3496,15 @@ private List<SetColumnTypeSetup> setColumnTypeSetupData()
.add(new SetColumnTypeSetup("char(20)", "'char-to-varchar'", "varchar"))
.add(new SetColumnTypeSetup("varchar", "'varchar-to-char'", "char(20)"))
.add(new SetColumnTypeSetup("array(integer)", "array[1]", "array(bigint)"))
.add(new SetColumnTypeSetup("row(x integer)", "row(1)", "row(x bigint)"))
.add(new SetColumnTypeSetup("row(x integer)", "row(1)", "row(y integer)", "cast(row(NULL) as row(x integer))")) // rename a field
.add(new SetColumnTypeSetup("row(x integer, y integer)", "row(1, 2)", "row(x integer, z integer)", "cast(row(1, NULL) as row(x integer, z integer))")) // rename a field, but not all fields
.add(new SetColumnTypeSetup("row(x integer)", "row(1)", "row(x integer, y integer)", "cast(row(1, NULL) as row(x integer, y integer))")) // add a new field
.add(new SetColumnTypeSetup("row(x integer, y integer)", "row(1, 2)", "row(x integer)", "cast(row(1) as row(x integer))")) // remove an existing field
.add(new SetColumnTypeSetup("row(x integer, y integer)", "row(1, 2)", "row(y integer, x integer)", "cast(row(2, 1) as row(y integer, x integer))")) // reorder fields
.add(new SetColumnTypeSetup("row(x integer, y integer)", "row(1, 2)", "row(z integer, y integer, x integer)", "cast(row(null, 2, 1) as row(z integer, y integer, x integer))")) // reorder fields with a new field
.add(new SetColumnTypeSetup("row(x row(nested integer))", "row(row(1))", "row(x row(nested bigint))", "cast(row(row(1)) as row(x row(nested bigint)))")) // update a nested field
.add(new SetColumnTypeSetup("row(x row(a integer, b integer))", "row(row(1, 2))", "row(x row(b integer, a integer))", "cast(row(row(2, 1)) as row(x row(b integer, a integer)))")) // reorder a nested field
.add(new SetColumnTypeSetup("row(x integer)", "row(1)", "row(\"x\" bigint)"))
.add(new SetColumnTypeSetup("row(x integer)", "row(1)", "row(\"y\" integer)", "cast(row(NULL) as row(x integer))")) // rename a field
.add(new SetColumnTypeSetup("row(x integer, y integer)", "row(1, 2)", "row(\"x\" integer, \"z\" integer)", "cast(row(1, NULL) as row(x integer, z integer))")) // rename a field, but not all fields
.add(new SetColumnTypeSetup("row(x integer)", "row(1)", "row(\"x\" integer, \"y\" integer)", "cast(row(1, NULL) as row(x integer, y integer))")) // add a new field
.add(new SetColumnTypeSetup("row(x integer, y integer)", "row(1, 2)", "row(\"x\" integer)", "cast(row(1) as row(x integer))")) // remove an existing field
.add(new SetColumnTypeSetup("row(x integer, y integer)", "row(1, 2)", "row(\"y\" integer, \"x\" integer)", "cast(row(2, 1) as row(y integer, x integer))")) // reorder fields
.add(new SetColumnTypeSetup("row(x integer, y integer)", "row(1, 2)", "row(\"z\" integer, \"y\" integer, \"x\" integer)", "cast(row(null, 2, 1) as row(z integer, y integer, x integer))")) // reorder fields with a new field
.add(new SetColumnTypeSetup("row(x row(nested integer))", "row(row(1))", "row(\"x\" row(\"nested\" bigint))", "cast(row(row(1)) as row(x row(nested bigint)))")) // update a nested field
.add(new SetColumnTypeSetup("row(x row(a integer, b integer))", "row(row(1, 2))", "row(\"x\" row(\"b\" integer, \"a\" integer))", "cast(row(row(2, 1)) as row(x row(b integer, a integer)))")) // reorder a nested field
.build();
}

Expand Down Expand Up @@ -3659,11 +3659,11 @@ public void testSetFieldTypes()
if (setup.unsupportedType) {
assertThatThrownBy(setFieldType::run)
.satisfies(this::verifySetFieldTypeFailurePermissible);
return;
continue;
}
setFieldType.run();

assertThat(getColumnType(table.getName(), "col")).isEqualTo("row(field " + setup.newColumnType + ")");
assertThat(getColumnType(table.getName(), "col")).isEqualTo("row(\"field\" " + setup.newColumnType + ")");
assertThat(query("SELECT * FROM " + table.getName()))
.skippingTypesCheck()
.matches("SELECT row(" + setup.newValueLiteral + ")");
Expand Down