rdblue commented on code in PR #18544:
URL: https://github.com/apache/iceberg/pull/18544#discussion_r4365124082
##########
core/src/test/java/org/apache/iceberg/mumbling/TestMumblingBitmap.java:
##########
@@ -290,4 +343,439 @@ void testBufferWithOffset() {
assertThat(bitmap.isSet(42)).isTrue();
assertThat(bitmap.isSet(43)).isFalse();
}
+
+ @Test
+ public void testMutableValidatesPositions() {
+ MutableBitmap bitmap = MumblingBitmap.newMutableBitmap();
+
+ assertThat(bitmap.set(0)).isTrue();
+ assertThatThrownBy(() -> bitmap.set(-1))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot set invalid bit position: -1 < 0");
+
+ assertThat(bitmap.set(2_097_151)).isTrue();
+ assertThatThrownBy(() -> bitmap.set(2_097_152))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot set invalid bit position: 2097152 (must be <
2,097,152)");
+ }
+
+ @Test
+ public void testMutableOverlayPassthrough() {
+ MutableBitmap bitmap = MUTABLE_TEST_BASE.asMutable();
+
+ assertThat(bitmap.cardinality()).isEqualTo(34);
+
+ assertThat(bitmap.isSet(0)).isTrue();
+ assertThat(bitmap.isSet(1)).isFalse();
+
+ assertThat(bitmap.isSet(263)).isFalse();
+ assertThat(bitmap.isSet(289)).isFalse();
Review Comment:
Yes, this was meant to check the positions just before the bytes that are
set (the pattern is 0x00, 0xFF, 0x00, 0xFF, 0x00, 0xFF, 0x00, 0xFF followed by
0x00 for the rest of the container). I initially had the wrong offsets and
fixed them, but accidentally left these unchanged.
##########
core/src/test/java/org/apache/iceberg/mumbling/TestMumblingBitmap.java:
##########
@@ -290,4 +343,439 @@ void testBufferWithOffset() {
assertThat(bitmap.isSet(42)).isTrue();
assertThat(bitmap.isSet(43)).isFalse();
}
+
+ @Test
+ public void testMutableValidatesPositions() {
+ MutableBitmap bitmap = MumblingBitmap.newMutableBitmap();
+
+ assertThat(bitmap.set(0)).isTrue();
+ assertThatThrownBy(() -> bitmap.set(-1))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot set invalid bit position: -1 < 0");
+
+ assertThat(bitmap.set(2_097_151)).isTrue();
+ assertThatThrownBy(() -> bitmap.set(2_097_152))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot set invalid bit position: 2097152 (must be <
2,097,152)");
+ }
+
+ @Test
+ public void testMutableOverlayPassthrough() {
+ MutableBitmap bitmap = MUTABLE_TEST_BASE.asMutable();
+
+ assertThat(bitmap.cardinality()).isEqualTo(34);
+
+ assertThat(bitmap.isSet(0)).isTrue();
+ assertThat(bitmap.isSet(1)).isFalse();
+
+ assertThat(bitmap.isSet(263)).isFalse();
+ assertThat(bitmap.isSet(289)).isFalse();
Review Comment:
Yes, this was meant to check the positions just before the bytes that are
set (the pattern is 0x00, 0xFF, 0x00, 0xFF, 0x00, 0xFF, 0x00, 0xFF followed by
0x00 for the rest of the container). I initially had the wrong offsets and
fixed them, but accidentally left these unchanged.
I'll push a fix.
--
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]