Skip to content

Commit db2bb37

Browse files
LuciferYangclaude
andcommitted
test: pin the chunk-size boundary, and say only what the narrowing does
The "4g" case re-entered the same comparison as "2g", so it pinned nothing; mutating the guard from > to >= went undetected. Assert the boundary itself instead. The comment also claimed more than the code does. Chunk size 0 is a supported mode (ChunkedDictionaryTest builds one deliberately) and 1g is a value the guard accepts, so neither "4g" nor "5g" produces a wrong index: narrowing substitutes a different size than the one configured, and only the negative case fails. Co-Authored-By: Claude Code <noreply@anthropic.com>
1 parent 75a2774 commit db2bb37

2 files changed

Lines changed: 12 additions & 19 deletions

File tree

‎paimon-common/src/main/java/org/apache/paimon/fileindex/rangebitmap/RangeBitmapFileIndex.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,9 @@ public Writer(DataType dataType, Options options) {
7373
KeyFactory factory = KeyFactory.create(dataType);
7474
String chunkSize = options.getString(CHUNK_SIZE, factory.defaultChunkSize());
7575
long bytes = MemorySize.parse(chunkSize).getBytes();
76-
// the chunk size becomes an eagerly allocated per-chunk buffer, so anything beyond
77-
// int range cannot work. Truncating is worse than rejecting: "2g" narrows to a
78-
// negative int and crashes deep inside the writer, while "4g" narrows to 0 and
79-
// "5g" to 1g, which build a silently wrong index instead of failing at all.
76+
// the chunk size becomes an eagerly allocated per-chunk buffer, so it has to fit an
77+
// int. Narrowing it silently substitutes a different size: "2g" becomes negative and
78+
// fails only once the writer allocates, "4g" becomes 0 and "5g" becomes 1g.
8079
if (bytes > Integer.MAX_VALUE) {
8180
throw new IllegalArgumentException(
8281
String.format(

‎paimon-common/src/test/java/org/apache/paimon/fileindex/rangebitmap/RangeBitmapFileIndexTest.java‎

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -62,28 +62,22 @@ public class RangeBitmapFileIndexTest {
6262
public void testChunkSizeBeyondIntRangeRejected() {
6363
VarCharType varCharType = new VarCharType();
6464

65-
// "2g" narrows to a negative int, which crashes on the writer's first chunk allocation
66-
Options negativeAfterNarrowing = new Options();
67-
negativeAfterNarrowing.setString(RangeBitmapFileIndex.CHUNK_SIZE, "2g");
65+
// the boundary is the whole guard: one byte past int range is rejected, int range itself
66+
// is accepted. "2g" and "4g" would both only re-test the same comparison
67+
Options justPastIntRange = new Options();
68+
justPastIntRange.setString(RangeBitmapFileIndex.CHUNK_SIZE, "2147483648 bytes");
6869
assertThatThrownBy(
6970
() ->
70-
new RangeBitmapFileIndex(varCharType, negativeAfterNarrowing)
71+
new RangeBitmapFileIndex(varCharType, justPastIntRange)
7172
.createWriter())
7273
.isInstanceOf(IllegalArgumentException.class)
7374
.hasMessageContaining("chunk-size");
7475

75-
// "4g" narrows to 0, which is the case worth guarding: it does not crash at all, it
76-
// gives every key its own chunk and builds a silently bloated index
77-
Options zeroAfterNarrowing = new Options();
78-
zeroAfterNarrowing.setString(RangeBitmapFileIndex.CHUNK_SIZE, "4g");
79-
assertThatThrownBy(
80-
() ->
81-
new RangeBitmapFileIndex(varCharType, zeroAfterNarrowing)
82-
.createWriter())
83-
.isInstanceOf(IllegalArgumentException.class)
84-
.hasMessageContaining("chunk-size");
76+
Options atIntRange = new Options();
77+
atIntRange.setString(RangeBitmapFileIndex.CHUNK_SIZE, "2147483647 bytes");
78+
assertThat(new RangeBitmapFileIndex(varCharType, atIntRange).createWriter()).isNotNull();
8579

86-
// a large but in-range chunk size still works
80+
// a large but in-range chunk size still writes and serializes
8781
Options valid = new Options();
8882
valid.setString(RangeBitmapFileIndex.CHUNK_SIZE, "16mb");
8983
FileIndexWriter writer = new RangeBitmapFileIndex(varCharType, valid).createWriter();

0 commit comments

Comments
 (0)