riemenschneider commented on code in PR #13697:
URL: https://github.com/apache/iceberg/pull/13697#discussion_r2246944587
##########
core/src/test/java/org/apache/iceberg/TestSchemaUpdate.java:
##########
@@ -779,6 +779,22 @@ public void testAddRequiredColumnCaseInsensitive() {
.hasMessage("Cannot add column, name already exists: ID");
}
+ @Test
+ public void testAddMultipleRequiredColumnCaseInsensitive() {
+ Schema schema = new Schema(required(1, "id", Types.IntegerType.get()));
+
+ assertThatThrownBy(
+ () ->
+ new SchemaUpdate(schema, 1)
+ .caseSensitive(false)
+ .allowIncompatibleChanges()
+ .addRequiredColumn("data", Types.StringType.get())
+ .addRequiredColumn("DATA", Types.StringType.get())
+ .apply())
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot build lower case index: data and DATA collide");
Review Comment:
Naming conflicts with the original schema are detected when the
`addColumn()` or `addRequiredColumn()` method is called, at which point an
appropriate exception (`Cannot add column, name already exists: ...`) is
thrown. Conflicts between added columns are detected when the `apply()` method
is called. While the error message provides all the necessary information, the
indexing part is not helpful to the end user. It seems as though some of the
relevant checks are duplicated.
Ideally, the `addColumn()` method would fail at this point with an
appropriate error message, since all the necessary information is already
available. In this case, the relevant check in the `apply()` method could be
skipped entirely (or kept as a safety net). However, this would definitely
require a significant change to the existing code.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
