rdblue commented on code in PR #16408:
URL: https://github.com/apache/iceberg/pull/16408#discussion_r3337133523
##########
core/src/test/java/org/apache/iceberg/TestDeletionVectorStruct.java:
##########
@@ -140,7 +140,26 @@ void testBuilderValidation() {
.sizeInBytes(1)
.build())
.isInstanceOf(IllegalArgumentException.class)
- .hasMessage("Invalid cardinality: -1 (must be >= 0)");
+ .hasMessage("Missing required value: cardinality");
+ }
+
+ @Test
+ void testBuilderRejectsInvalidValuesAtSetter() {
+ assertThatThrownBy(() -> DeletionVectorStruct.builder().location(null))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Invalid location: null");
+
+ assertThatThrownBy(() -> DeletionVectorStruct.builder().offset(-1))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Invalid offset: -1 (must be >= 0)");
+
+ assertThatThrownBy(() -> DeletionVectorStruct.builder().sizeInBytes(-1))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Invalid size in bytes: -1 (must be >= 0)");
+
+ assertThatThrownBy(() -> DeletionVectorStruct.builder().cardinality(0))
Review Comment:
I understand validating this and it doesn't matter because we're removing
it, but it seems weird not to allow cardinality 0. I think it should just do
the right thing rather than failing.
--
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]
