ebyhr commented on code in PR #11755: URL: https://github.com/apache/iceberg/pull/11755#discussion_r1881237568
########## core/src/test/java/org/apache/iceberg/TestSchemaUpdate.java: ########## @@ -731,6 +731,17 @@ public void testAmbiguousAdd() { .hasMessageStartingWith("Cannot add column with ambiguous name: preferences.booleans"); } + @Test + public void testAddStructTypeWithEmptyFields() { + assertThatThrownBy( + () -> { + UpdateSchema update = new SchemaUpdate(SCHEMA, SCHEMA_LAST_COLUMN_ID); + update.addColumn("empty_struct", Types.StructType.of()); + }) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot add struct with empty fields: struct<>"); + } Review Comment: Do we have a test that drops the last field from a column? ########## core/src/main/java/org/apache/iceberg/SchemaUpdate.java: ########## @@ -168,6 +168,17 @@ private void internalAddColumn( fullName = name; } + // when adding structs with empty fields, Parquet fails complaining + // Cannot write a schema with an empty group, so fail on add column + // More details: https://issues.apache.org/jira/browse/SPARK-20593 Review Comment: What's the expected behavior when the nested field (not root type) is an empty? e.g. ```java Types.StructType.of(Types.NestedField.optional(1, "x", Types.StructType.of())) ``` Can we add such a test? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org