amogh-jahagirdar commented on code in PR #13697:
URL: https://github.com/apache/iceberg/pull/13697#discussion_r2246507052
##########
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:
I'm good with this but It'd be nice if we can detect this in a different way
and fail with a better error message that dosen't really mention anything about
the internal indexing we do. since it's really no worse then the
case-sensitive case (and it's consistent) where if someone does
```add("data"....).add("data"...)``` it fails with the same message, I'm good
with it.
##########
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()
Review Comment:
Minor: `allowIncompatibleChanges` isn't strictly required to repro this
issue right? I think in this test case it was just set because we're adding
required columns but if we were to remove this and just do `addOptionalColumn`
for both "data" and "DATA", it should still just fail the same way.
--
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
