fix spelling, address comments

LakshSingla · Jan 5, 2024 · b712355 · b712355
1 parent c88adc2
commit b712355
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 9 deletions.
diff --git a/...org/apache/druid/query/aggregation/SerializablePairLongStringDeltaEncodedStagedSerde.java b/...org/apache/druid/query/aggregation/SerializablePairLongStringDeltaEncodedStagedSerde.java
@@ -36,6 +36,14 @@
  *
  * otherwise
  * Long:Integer:bytes
+ *
+ * The StringSize can be following:
+ * -1 : Denotes an empty string
+ * 0  : Denotes a null string
+ * >0 : Denotes a non-empty string
+ *
+ * Mapping of null and empty string is done weirdly to preserve backward compatibility when nulls were returned all the
+ * time, and there was no distinction between empty and null string
  */
 public class SerializablePairLongStringDeltaEncodedStagedSerde implements StagedSerde<SerializablePairLongString>
 {
@@ -76,6 +84,8 @@ public void store(ByteBuffer byteBuffer)
         }
 
         if (rhsString == null) {
+          byteBuffer.putInt(0);
+        } else if (rhsBytes.length == 0) {
           byteBuffer.putInt(-1);
         } else {
           byteBuffer.putInt(rhsBytes.length);
@@ -116,11 +126,13 @@ public SerializablePairLongString deserialize(ByteBuffer byteBuffer)
     int stringSize = readOnlyBuffer.getInt();
     String lastString = null;
 
-    if (stringSize >= 0) {
+    if (stringSize > 0) {
       byte[] stringBytes = new byte[stringSize];
 
       readOnlyBuffer.get(stringBytes, 0, stringSize);
       lastString = StringUtils.fromUtf8(stringBytes);
+    } else if (stringSize < 0) {
+      lastString = "";
     }
 
     return new SerializablePairLongString(lhs, lastString);

diff --git a/.../java/org/apache/druid/query/aggregation/SerializablePairLongStringSimpleStagedSerde.java b/.../java/org/apache/druid/query/aggregation/SerializablePairLongStringSimpleStagedSerde.java
@@ -34,6 +34,14 @@
  * <p>
  * or
  * Long:StringSize:StringData
+ *
+ * The StringSize can be following:
+ * -1 : Denotes an empty string
+ * 0  : Denotes a null string
+ * >0 : Denotes a non-empty string
+ *
+ * Mapping of null and empty string is done weirdly to preserve backward compatibility when nulls were returned all the
+ * time, and there was no distinction between empty and null string
  */
 public class SerializablePairLongStringSimpleStagedSerde implements StagedSerde<SerializablePairLongString>
 {
@@ -56,6 +64,8 @@ public void store(ByteBuffer byteBuffer)
 
         byteBuffer.putLong(value.lhs);
         if (rhsString == null) {
+          byteBuffer.putInt(0);
+        } else if (rhsBytes.length == 0) {
           byteBuffer.putInt(-1);
         } else {
           byteBuffer.putInt(rhsBytes.length);
@@ -87,11 +97,13 @@ public SerializablePairLongString deserialize(ByteBuffer byteBuffer)
     int stringSize = readOnlyBuffer.getInt();
     String lastString = null;
 
-    if (stringSize >= 0) {
+    if (stringSize > 0) {
       byte[] stringBytes = new byte[stringSize];
 
       readOnlyBuffer.get(stringBytes, 0, stringSize);
       lastString = StringUtils.fromUtf8(stringBytes);
+    } else if (stringSize < 0) {
+      lastString = "";
     }
 
     return new SerializablePairLongString(lhs, lastString);

diff --git a/...ongStringDeltaEncodedStagedSerdeTest.java → ...ongStringDeltaEncodedStagedSerdeTest.java b/...ongStringDeltaEncodedStagedSerdeTest.java → ...ongStringDeltaEncodedStagedSerdeTest.java
@@ -33,7 +33,7 @@
 import java.nio.ByteOrder;
 import java.util.Random;
 
-public class BackwordCompatibleSerializablePairLongStringDeltaEncodedStagedSerdeTest
+public class BackwardCompatibleSerializablePairLongStringDeltaEncodedStagedSerdeTest
 {
   private static final OlderSerializablePairLongStringDeltaEncodedStagedSerde OLDER_INTEGER_SERDE =
       new OlderSerializablePairLongStringDeltaEncodedStagedSerde(0L, true);
@@ -71,11 +71,11 @@ public void testNullString()
 
     // Write using the older serde, read using the newer serde
     Assert.assertEquals(
-        new SerializablePairLongString(TIMESTAMP, ""),
+        new SerializablePairLongString(TIMESTAMP, null),
         readUsingSerde(writeUsingSerde(value, OLDER_INTEGER_SERDE), INTEGER_SERDE)
     );
     Assert.assertEquals(
-        new SerializablePairLongString(TIMESTAMP, ""),
+        new SerializablePairLongString(TIMESTAMP, null),
         readUsingSerde(writeUsingSerde(value, OLDER_LONG_SERDE), LONG_SERDE)
     );
 
@@ -104,11 +104,11 @@ public void testEmptyString()
 
     // Write using the older serde, read using the newer serde
     Assert.assertEquals(
-        new SerializablePairLongString(TIMESTAMP, ""),
+        new SerializablePairLongString(TIMESTAMP, null),
         readUsingSerde(writeUsingSerde(value, OLDER_INTEGER_SERDE), INTEGER_SERDE)
     );
     Assert.assertEquals(
-        new SerializablePairLongString(TIMESTAMP, ""),
+        new SerializablePairLongString(TIMESTAMP, null),
         readUsingSerde(writeUsingSerde(value, OLDER_LONG_SERDE), LONG_SERDE)
     );
 

diff --git a/...ePairLongStringSimpleStagedSerdeTest.java → ...ePairLongStringSimpleStagedSerdeTest.java b/...ePairLongStringSimpleStagedSerdeTest.java → ...ePairLongStringSimpleStagedSerdeTest.java
@@ -32,7 +32,7 @@
 import java.nio.ByteOrder;
 import java.util.Random;
 
-public class BackwordCompatibleSerializablePairLongStringSimpleStagedSerdeTest
+public class BackwardCompatibleSerializablePairLongStringSimpleStagedSerdeTest
 {
   private static final OlderSerializablePairLongStringSimpleStagedSerde OLDER_SERDE =
       new OlderSerializablePairLongStringSimpleStagedSerde();

diff --git a/...apache/druid/query/aggregation/SerializablePairLongStringDeltaEncodedStagedSerdeTest.java b/...apache/druid/query/aggregation/SerializablePairLongStringDeltaEncodedStagedSerdeTest.java
@@ -84,7 +84,7 @@ public void testNullStringLong()
   @Test
   public void testEmptyStringLong()
   {
-    assertValueEquals(new SerializablePairLongString(100L, ""), 8, LONG_SERDE);
+    assertValueEquals(new SerializablePairLongString(100L, ""), 12, LONG_SERDE);
   }
-Original file line number
+Diff line change
@@ Expand Up / @@ -84,7 +84,7 @@ public void testNullStringLong() @@
       @Test
       public void testEmptyStringLong()
       {
-        assertValueEquals(new SerializablePairLongString(100L, ""), 8, LONG_SERDE);
+        assertValueEquals(new SerializablePairLongString(100L, ""), 12, LONG_SERDE);
       }
@@ Expand Down @@