Skip to content

Commit

Permalink
Revert "Allow changing field type in records wrapped in an array"
Browse files Browse the repository at this point in the history
This change introduces behavior incompatible with #16959
  • Loading branch information
martint committed May 31, 2024
1 parent 99b9b37 commit 7239574
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import io.trino.spi.TrinoException;
import io.trino.spi.connector.ColumnHandle;
import io.trino.spi.connector.ColumnMetadata;
import io.trino.spi.type.ArrayType;
import io.trino.spi.type.RowType;
import io.trino.spi.type.Type;
import io.trino.spi.type.TypeManager;
Expand Down Expand Up @@ -140,11 +139,7 @@ else if (metadata.isView(session, qualifiedObjectName)) {

private static List<RowType.Field> getCandidates(Type type, String fieldName)
{
Type analyzedType = type;
if (type instanceof ArrayType arrayType) {
analyzedType = arrayType.getElementType();
}
if (!(analyzedType instanceof RowType rowType)) {
if (!(type instanceof RowType rowType)) {
throw new TrinoException(NOT_SUPPORTED, "Unsupported type: " + type);
}
List<RowType.Field> candidates = rowType.getFields().stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2132,19 +2132,7 @@ public void setFieldType(ConnectorSession session, ConnectorTableHandle tableHan
NestedField parent = icebergTable.schema().caseInsensitiveFindField(parentPath);

String caseSensitiveParentName = icebergTable.schema().findColumnName(parent.fieldId());

Types.StructType structType;
if (parent.type().isListType()) {
// list(struct...)
structType = parent.type().asListType().elementType().asStructType();
caseSensitiveParentName += ".element";
}
else {
// just struct
structType = parent.type().asStructType();
}
NestedField field = structType.caseInsensitiveField(getLast(fieldPath));

NestedField field = parent.type().asStructType().caseInsensitiveField(getLast(fieldPath));
// TODO: Add support for changing non-primitive field type
if (!field.type().isPrimitiveType()) {
throw new TrinoException(NOT_SUPPORTED, "Iceberg doesn't support changing field type from non-primitive types");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,6 @@
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_ROW_TYPE;
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_SET_COLUMN_TYPE;
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_SET_FIELD_TYPE;
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_SET_FIELD_TYPE_IN_ARRAY;
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_TOPN_PUSHDOWN;
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_TRUNCATE;
import static io.trino.testing.TestingConnectorBehavior.SUPPORTS_UPDATE;
Expand Down Expand Up @@ -3160,49 +3159,6 @@ public void testSetFieldOutOfRangeType()
}
}

@Test
public void testSetFieldTypeInArray()
{
skipTestUnless(hasBehavior(SUPPORTS_CREATE_TABLE_WITH_DATA) && hasBehavior(SUPPORTS_ARRAY) && hasBehavior(SUPPORTS_ROW_TYPE));

if (!hasBehavior(SUPPORTS_SET_FIELD_TYPE_IN_ARRAY)) {
try (TestTable table = new TestTable(getQueryRunner()::execute, "test_set_field_type_in_array_", "(col array(row(field int)))")) {
assertQueryFails(
"ALTER TABLE " + table.getName() + " ALTER COLUMN col.field SET DATA TYPE bigint",
".*does not support.*");
}
return;
}

try (TestTable table = new TestTable(getQueryRunner()::execute, "test_set_field_type_in_array_", "AS SELECT CAST(array[row(123)] AS array(row(field integer))) AS col")) {
assertThat(getColumnType(table.getName(), "col")).isEqualTo("array(row(field integer))");

assertUpdate("ALTER TABLE " + table.getName() + " ALTER COLUMN col.field SET DATA TYPE bigint");

assertThat(getColumnType(table.getName(), "col")).isEqualTo("array(row(field bigint))");
assertThat(query("SELECT * FROM " + table.getName()))
.skippingTypesCheck()
.matches("SELECT array[row(bigint '123')]");
}
}

@Test
public void testSetFieldTypeInNestedArray()
{
skipTestUnless(hasBehavior(SUPPORTS_SET_FIELD_TYPE_IN_ARRAY) && hasBehavior(SUPPORTS_CREATE_TABLE_WITH_DATA) && hasBehavior(SUPPORTS_ARRAY) && hasBehavior(SUPPORTS_ROW_TYPE));

try (TestTable table = new TestTable(getQueryRunner()::execute, "test_set_field_type_in_nested_array_", "AS SELECT CAST(array[row(array[row(123)])] AS array(row(field array(row(a integer))))) AS col")) {
assertThat(getColumnType(table.getName(), "col")).isEqualTo("array(row(field array(row(a integer))))");

assertUpdate("ALTER TABLE " + table.getName() + " ALTER COLUMN col.field.a SET DATA TYPE bigint");

assertThat(getColumnType(table.getName(), "col")).isEqualTo("array(row(field array(row(a bigint))))");
assertThat(query("SELECT * FROM " + table.getName()))
.skippingTypesCheck()
.matches("SELECT array[row(array[row(bigint '123')])]");
}
}

protected void verifySetFieldTypeFailurePermissible(Throwable e)
{
throw new AssertionError("Unexpected set field type failure", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ public enum TestingConnectorBehavior
SUPPORTS_RENAME_FIELD(fallback -> fallback.test(SUPPORTS_RENAME_COLUMN) && fallback.test(SUPPORTS_ROW_TYPE)),
SUPPORTS_SET_COLUMN_TYPE,
SUPPORTS_SET_FIELD_TYPE(fallback -> fallback.test(SUPPORTS_SET_COLUMN_TYPE) && fallback.test(SUPPORTS_ROW_TYPE)),
SUPPORTS_SET_FIELD_TYPE_IN_ARRAY(fallback -> fallback.test(SUPPORTS_SET_FIELD_TYPE) && fallback.test(SUPPORTS_ADD_FIELD_IN_ARRAY)),

SUPPORTS_COMMENT_ON_TABLE,
SUPPORTS_COMMENT_ON_COLUMN(SUPPORTS_COMMENT_ON_TABLE),
Expand Down

0 comments on commit 7239574

Please sign in to comment.