From cbde6f3052ae7348c9a01ad0f200624aa0d556cb Mon Sep 17 00:00:00 2001
From: Sunjeet Singh <sunjeets@netflix.com>
Date: Mon, 9 Oct 2023 06:06:36 -0700
Subject: [PATCH] Assume non-uniform field width across shards, add tests for
 filtered fields

---
 .../HollowObjectTypeDataElementsJoiner.java   | 10 +++++--
 .../HollowObjectTypeDataElementsSplitter.java |  5 ++--
 ...owObjectTypeDataElementsSplitJoinTest.java | 29 +++++++++++++++++--
 ...owObjectTypeDataElementsSplitJoinTest.java | 24 ++++++++++++++-
 4 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/object/HollowObjectTypeDataElementsJoiner.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/object/HollowObjectTypeDataElementsJoiner.java
index 89d9562ed4..806c11ce53 100644
--- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/object/HollowObjectTypeDataElementsJoiner.java
+++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/object/HollowObjectTypeDataElementsJoiner.java
@@ -76,7 +76,10 @@ void populateStats(HollowObjectTypeDataElements to, HollowObjectTypeDataElements
 
         for(int fieldIdx=0;fieldIdx<to.schema.numFields();fieldIdx++) {
             if(from[0].varLengthData[fieldIdx] == null) {
-                to.bitsPerField[fieldIdx] = from[0].bitsPerField[fieldIdx];
+                // do not assume bitsPerField will be uniform
+                for(int fromIndex=0;fromIndex<from.length;fromIndex++) {
+                    to.bitsPerField[fieldIdx] = Math.max(to.bitsPerField[fieldIdx], from[fromIndex].bitsPerField[fieldIdx]);
+                }
             } else {
                 to.bitsPerField[fieldIdx] = (64 - Long.numberOfLeadingZeros(varLengthSizes[fieldIdx] + 1)) + 1;
             }
@@ -85,7 +88,8 @@ void populateStats(HollowObjectTypeDataElements to, HollowObjectTypeDataElements
             to.bitsPerRecord += to.bitsPerField[fieldIdx];
         }
 
-        to.bitsPerUnfilteredField = from[0].bitsPerUnfilteredField;
-        to.unfilteredFieldIsIncluded = from[0].unfilteredFieldIsIncluded;
+        // unused
+        //  to.bitsPerUnfilteredField
+        //  to.unfilteredFieldIsIncluded
     }
 }
diff --git a/hollow/src/main/java/com/netflix/hollow/core/read/engine/object/HollowObjectTypeDataElementsSplitter.java b/hollow/src/main/java/com/netflix/hollow/core/read/engine/object/HollowObjectTypeDataElementsSplitter.java
index 40a6c1d0f6..b24928218d 100644
--- a/hollow/src/main/java/com/netflix/hollow/core/read/engine/object/HollowObjectTypeDataElementsSplitter.java
+++ b/hollow/src/main/java/com/netflix/hollow/core/read/engine/object/HollowObjectTypeDataElementsSplitter.java
@@ -87,8 +87,9 @@ private void populateStats(HollowObjectTypeDataElements[] to, HollowObjectTypeDa
                 to[toIndex].bitOffsetPerField[fieldIdx] = to[toIndex].bitsPerRecord;
                 to[toIndex].bitsPerRecord += to[toIndex].bitsPerField[fieldIdx];
 
-                to[toIndex].bitsPerUnfilteredField = from.bitsPerUnfilteredField;
-                to[toIndex].unfilteredFieldIsIncluded = from.unfilteredFieldIsIncluded;
+                // unused
+                // to[toIndex].bitsPerUnfilteredField = from.bitsPerUnfilteredField;
+                // to[toIndex].unfilteredFieldIsIncluded = from.unfilteredFieldIsIncluded;
             }
         }
     }
diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/engine/object/AbstractHollowObjectTypeDataElementsSplitJoinTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/engine/object/AbstractHollowObjectTypeDataElementsSplitJoinTest.java
index ecba289ba8..adffb055f8 100644
--- a/hollow/src/test/java/com/netflix/hollow/core/read/engine/object/AbstractHollowObjectTypeDataElementsSplitJoinTest.java
+++ b/hollow/src/test/java/com/netflix/hollow/core/read/engine/object/AbstractHollowObjectTypeDataElementsSplitJoinTest.java
@@ -5,7 +5,10 @@
 
 import com.netflix.hollow.api.objects.generic.GenericHollowObject;
 import com.netflix.hollow.core.AbstractStateEngineTest;
+import com.netflix.hollow.core.read.engine.HollowReadStateEngine;
+import com.netflix.hollow.core.read.filter.HollowFilterConfig;
 import com.netflix.hollow.core.schema.HollowObjectSchema;
+import com.netflix.hollow.core.util.StateEngineRoundTripper;
 import com.netflix.hollow.core.write.HollowObjectTypeWriteState;
 import com.netflix.hollow.core.write.HollowObjectWriteRecord;
 import java.io.IOException;
@@ -21,9 +24,11 @@ public class AbstractHollowObjectTypeDataElementsSplitJoinTest extends AbstractS
 
     @Before
     public void setUp() {
-        schema = new HollowObjectSchema("TestObject", 2);
+        schema = new HollowObjectSchema("TestObject", 4);
         schema.addField("longField", HollowObjectSchema.FieldType.LONG);
         schema.addField("stringField", HollowObjectSchema.FieldType.STRING);
+        schema.addField("intField", HollowObjectSchema.FieldType.INT);
+        schema.addField("doubleField", HollowObjectSchema.FieldType.DOUBLE);
 
         MockitoAnnotations.initMocks(this);
         HollowObjectTypeDataElements[] fakeDataElements = new HollowObjectTypeDataElements[5];
@@ -37,25 +42,45 @@ protected void initializeTypeStates() {
         writeStateEngine.addTypeState(new HollowObjectTypeWriteState(schema));
     }
 
-    protected HollowObjectTypeReadState populateTypeStateWith(int numRecords) throws IOException {
+    private void populateWriteStateEngine(int numRecords) {
         initWriteStateEngine();
         HollowObjectWriteRecord rec = new HollowObjectWriteRecord(schema);
         for(int i=0;i<numRecords;i++) {
             rec.reset();
             rec.setLong("longField", i);
             rec.setString("stringField", "Value" + i);
+            rec.setInt("intField", i);
+            rec.setDouble("doubleField", i);
 
             writeStateEngine.add("TestObject", rec);
         }
+    }
+
+    protected HollowObjectTypeReadState populateTypeStateWith(int numRecords) throws IOException {
+        populateWriteStateEngine(numRecords);
         roundTripSnapshot();
         return (HollowObjectTypeReadState) readStateEngine.getTypeState("TestObject");
     }
 
+    protected HollowObjectTypeReadState populateTypeStateWithFilter(int numRecords) throws IOException {
+        populateWriteStateEngine(numRecords);
+        readStateEngine = new HollowReadStateEngine();
+        HollowFilterConfig readFilter = new HollowFilterConfig(true);
+        readFilter.addField("TestObject", "intField");
+        StateEngineRoundTripper.roundTripSnapshot(writeStateEngine, readStateEngine, readFilter);
+        return (HollowObjectTypeReadState) readStateEngine.getTypeState("TestObject");
+    }
+
     protected void assertDataUnchanged(int numRecords) {
         for(int i=0;i<numRecords;i++) {
             GenericHollowObject obj = new GenericHollowObject(readStateEngine, "TestObject", i);
             assertEquals(i, obj.getLong("longField"));
             assertEquals("Value"+i, obj.getString("stringField"));
+            HollowObjectTypeReadState typeState = (HollowObjectTypeReadState) readStateEngine.getTypeState("TestObject");
+            assertEquals((double)i, obj.getDouble("doubleField"), 0);
+            if (typeState.getSchema().numFields() == 4) {   // filtered
+                assertEquals(i, obj.getInt("intField"));
+            }
         }
     }
 }
diff --git a/hollow/src/test/java/com/netflix/hollow/core/read/engine/object/HollowObjectTypeDataElementsSplitJoinTest.java b/hollow/src/test/java/com/netflix/hollow/core/read/engine/object/HollowObjectTypeDataElementsSplitJoinTest.java
index 2b8b0e8ad4..d87b389ab6 100644
--- a/hollow/src/test/java/com/netflix/hollow/core/read/engine/object/HollowObjectTypeDataElementsSplitJoinTest.java
+++ b/hollow/src/test/java/com/netflix/hollow/core/read/engine/object/HollowObjectTypeDataElementsSplitJoinTest.java
@@ -44,9 +44,31 @@ public void testSplitThenJoin() throws IOException {
         }
     }
 
+    @Test
+    public void testSplitThenJoinWithFilter() throws IOException {
+        HollowObjectTypeDataElementsSplitter splitter = new HollowObjectTypeDataElementsSplitter();
+        HollowObjectTypeDataElementsJoiner joiner = new HollowObjectTypeDataElementsJoiner();
+
+        int numSplits = 2;
+        for (int numRecords=0;numRecords<1*1000;numRecords++) {
+            HollowObjectTypeReadState typeReadState = populateTypeStateWithFilter(numRecords);
+            assertEquals(1, typeReadState.numShards());
+            assertDataUnchanged(numRecords);
+            HollowChecksum origChecksum = typeReadState.getChecksum(typeReadState.getSchema());
+
+            HollowObjectTypeDataElements[] splitElements = splitter.split(typeReadState.currentDataElements()[0], numSplits);
+            HollowObjectTypeDataElements joinedElements = joiner.join(splitElements);
+            typeReadState.setCurrentData(joinedElements);
+
+            assertDataUnchanged(numRecords);
+            HollowChecksum resultChecksum = typeReadState.getChecksum(typeReadState.getSchema());
+            assertEquals(origChecksum, resultChecksum);
+        }
+    }
+
     // manually invoked
     // @Test
-    public void testSplittingAndJoiningWithVms() throws Exception {
+    public void testSplittingAndJoiningWithSnapshotBlob() throws Exception {
 
         String blobPath = null; // dir where snapshot blob exists for e.g. "/tmp/";
         long v = 0l; // snapshot version for e.g. 20230915162636001l;