From 044489969a6b5afe07a32bc9197a193ae1719cb1 Mon Sep 17 00:00:00 2001
From: Eduardo Ramirez <edyr94@gmail.com>
Date: Mon, 13 Jan 2025 05:51:50 -0800
Subject: [PATCH] fix: if the max ordinal for a map value is zero then reserve
 1 bit for 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 <sstattla@yahoo.com>
---
 .../engine/map/HollowMapDeltaApplicator.java  | 19 ++++++++++++++++---
 .../core/write/HollowMapTypeWriteState.java   |  4 ++--
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapDeltaApplicator.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapDeltaApplicator.java
index 1994926a91..c322dc88d0 100644
--- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapDeltaApplicator.java
+++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/map/HollowMapDeltaApplicator.java
@@ -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;
 
@@ -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;
diff --git a/hollow/src/main/java/com/netflix/hollow/core/write/HollowMapTypeWriteState.java b/hollow/src/main/java/com/netflix/hollow/core/write/HollowMapTypeWriteState.java
index ec27c3d692..2b94f63e1a 100644
--- a/hollow/src/main/java/com/netflix/hollow/core/write/HollowMapTypeWriteState.java
+++ b/hollow/src/main/java/com/netflix/hollow/core/write/HollowMapTypeWriteState.java
@@ -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);
 
@@ -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);