Skip to content

Commit

Permalink
fix: if the max ordinal for a map value is zero then reserve 1 bit fo…
Browse files Browse the repository at this point in the history
…r it instead of nothing (#713)

* fix: if the max ordinal for a map value is zero then reserve 1 bit for it instead of nothing

* (contd.) fix: if the max ordinal for a map value is zero then reserve 1 bit for it instead of nothing

* add consumer code to account for the bad scenario to allow to exit out of bad blobs

---------

Co-authored-by: Sunjeet Singh <[email protected]>
  • Loading branch information
eduardoramirez and Sunjeet authored Jan 13, 2025
1 parent 3f11f46 commit 0444899
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,14 @@ public void applyDelta() {
target.bitsPerMapPointer = delta.bitsPerMapPointer;
target.bitsPerMapSizeValue = delta.bitsPerMapSizeValue;
target.bitsPerKeyElement = delta.bitsPerKeyElement;
target.bitsPerValueElement = delta.bitsPerValueElement;
// Prior to Jan 2025, the producer was able to generate blobs where it reserved 0 bits for the Map value
// element when all keys stored the same record and that record's ordinal was 0. In this case, when reading the
// Map value of the last bucket, the code would trigger ArrayIndexOutOfBoundsException since there was no space
// allocated for the value element. This is a workaround to avoid the exception when transitioning out of one
// of these bad blobs.
target.bitsPerValueElement = delta.bitsPerValueElement == 0 ? 1 : delta.bitsPerValueElement;
target.bitsPerFixedLengthMapPortion = delta.bitsPerFixedLengthMapPortion;
target.bitsPerMapEntry = delta.bitsPerMapEntry;
target.bitsPerMapEntry = target.bitsPerKeyElement + target.bitsPerValueElement;
target.emptyBucketKeyValue = delta.emptyBucketKeyValue;
target.totalNumberOfBuckets = delta.totalNumberOfBuckets;

Expand Down Expand Up @@ -143,7 +148,15 @@ private void mergeOrdinal(int ordinal) {
if(!removeData) {
for(long bucketIdx=currentFromStateStartBucket; bucketIdx<fromDataEndBucket; bucketIdx++) {
long bucketKey = from.entryData.getElementValue(bucketIdx * from.bitsPerMapEntry, from.bitsPerKeyElement);
long bucketValue = from.entryData.getElementValue(bucketIdx * from.bitsPerMapEntry + from.bitsPerKeyElement, from.bitsPerValueElement);
// Prior to Jan 2025, the producer was able to generate blobs where it reserved 0 bits for the Map value
// element when all keys stored the same record and that record's ordinal was 0. In this case, when reading the
// Map value of the last bucket, the code would trigger ArrayIndexOutOfBoundsException since there was no space
// allocated for the value element. This is a workaround to avoid the exception when transitioning out of one
// of these bad blobs.
long bucketValue =
from.bitsPerValueElement == 0
? 0
: from.entryData.getElementValue(bucketIdx * from.bitsPerMapEntry + from.bitsPerKeyElement, from.bitsPerValueElement);
if(bucketKey == from.emptyBucketKeyValue)
bucketKey = target.emptyBucketKeyValue;
long currentWriteStartBucketBit = currentWriteStartBucket * target.bitsPerMapEntry;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ private void gatherStatistics() {
}

bitsPerKeyElement = 64 - Long.numberOfLeadingZeros(maxKeyOrdinal + 1);
bitsPerValueElement = 64 - Long.numberOfLeadingZeros(maxValueOrdinal);
bitsPerValueElement = maxValueOrdinal == 0 ? 1 : 64 - Long.numberOfLeadingZeros(maxValueOrdinal);

bitsPerMapSizeValue = 64 - Long.numberOfLeadingZeros(maxMapSize);

Expand Down Expand Up @@ -185,7 +185,7 @@ private void calculateNumShards() {
}

long bitsPerKeyElement = 64 - Long.numberOfLeadingZeros(maxKeyOrdinal + 1);
long bitsPerValueElement = 64 - Long.numberOfLeadingZeros(maxValueOrdinal);
long bitsPerValueElement = maxValueOrdinal == 0 ? 1 : 64 - Long.numberOfLeadingZeros(maxValueOrdinal);
long bitsPerMapSizeValue = 64 - Long.numberOfLeadingZeros(maxMapSize);
long bitsPerMapPointer = 64 - Long.numberOfLeadingZeros(totalOfMapBuckets);

Expand Down

0 comments on commit 0444899

Please sign in to comment.