working LongArrayFieldReaderTest

LakshSingla · Sep 7, 2023 · b707bce · b707bce
1 parent 18a6c7c
commit b707bce
Show file tree

Hide file tree

Showing 6 changed files with 103 additions and 25 deletions.
diff --git a/processing/src/main/java/org/apache/druid/frame/field/NumericArrayFieldReader.java b/processing/src/main/java/org/apache/druid/frame/field/NumericArrayFieldReader.java
@@ -39,7 +39,7 @@ public DimensionSelector makeDimensionSelector(
       @Nullable ExtractionFn extractionFn
   )
   {
-    // TODO: Should I throw an exception here
+    // TODO(laksh): Should I throw an exception here
     return DimensionSelector.nilSelector();
   }
 

diff --git a/processing/src/main/java/org/apache/druid/frame/field/NumericArrayFieldWriter.java b/processing/src/main/java/org/apache/druid/frame/field/NumericArrayFieldWriter.java
@@ -121,8 +121,8 @@ public void inspectRuntimeShape(RuntimeShapeInspector inspector)
         @Override
         public boolean isNull()
         {
-          // TODO: Add a comment why NullHandling.replaceWithDefault() is not required here
-          return !NullHandling.replaceWithDefault() && getObject() == null;
+          // TODO(laksh): Add a comment why NullHandling.replaceWithDefault() is not required here
+          return getObject() == null;
         }
 
         @Nullable

diff --git a/processing/src/main/java/org/apache/druid/frame/field/NumericFieldWriterFactory.java b/processing/src/main/java/org/apache/druid/frame/field/NumericFieldWriterFactory.java
@@ -21,6 +21,7 @@
 
 import org.apache.druid.segment.ColumnValueSelector;
 
+// TODO(laksh): Javadoc
 public interface NumericFieldWriterFactory
 {
   NumericFieldWriter get(ColumnValueSelector<Number> selector);

diff --git a/processing/src/test/java/org/apache/druid/frame/field/IndexArrayFieldPointer.java b/processing/src/test/java/org/apache/druid/frame/field/IndexArrayFieldPointer.java
@@ -33,7 +33,7 @@ public IndexArrayFieldPointer(final List<Long> indices)
     this.indices = new LongArrayList(indices);
   }
 
-  public int numIndices()
+  private int numIndices()
   {
     return indices.size();
   }
@@ -47,6 +47,6 @@ public void setPointer(int newPointer)
   @Override
   public long position()
   {
-    return indices.indexOf(pointer);
+    return indices.getLong(pointer);
   }
 }
diff --git a/processing/src/test/java/org/apache/druid/frame/field/LongArrayFieldReaderTest.java b/processing/src/test/java/org/apache/druid/frame/field/LongArrayFieldReaderTest.java
@@ -21,7 +21,6 @@
 
 import com.google.common.collect.ImmutableList;
 import org.apache.datasketches.memory.WritableMemory;
-import org.apache.druid.common.config.NullHandling;
 import org.apache.druid.java.util.common.ISE;
 import org.apache.druid.segment.ColumnValueSelector;
 import org.apache.druid.testing.InitializedNullHandlingTest;
@@ -37,7 +36,10 @@
 import org.mockito.quality.Strictness;
 
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
+import java.util.stream.Collectors;
 
 public class LongArrayFieldReaderTest extends InitializedNullHandlingTest
 {
@@ -52,6 +54,32 @@ public class LongArrayFieldReaderTest extends InitializedNullHandlingTest
   private WritableMemory memory;
   private FieldWriter fieldWriter;
 
+  private static final Object[] LONGS_ARRAY_1 = new Object[]{
+      Long.MIN_VALUE,
+      Long.MAX_VALUE,
+      null,
+      0L,
+      123L,
+      -123L
+  };
+
+  private static final Object[] LONGS_ARRAY_2 = new Object[]{
+      234L,
+      Long.MAX_VALUE,
+      null,
+      Long.MIN_VALUE,
+      0L,
+      -234L
+  };
+
+  private static final List<Long> LONGS_LIST_1;
+  private static final List<Long> LONGS_LIST_2;
+
+  static {
+    LONGS_LIST_1 = Arrays.stream(LONGS_ARRAY_1).map(val -> (Long) val).collect(Collectors.toList());
+    LONGS_LIST_2 = Arrays.stream(LONGS_ARRAY_2).map(val -> (Long) val).collect(Collectors.toList());
+  }
+
   @Before
   public void setUp()
   {
@@ -66,56 +94,104 @@ public void tearDown()
   }
 
   @Test
-  public void test_isNull_defaultOrNull()
+  public void test_isNull_null()
   {
-    writeToMemory(null);
+    writeToMemory(null, MEMORY_POSITION);
     Assert.assertTrue(new LongArrayFieldReader().isNull(memory, MEMORY_POSITION));
   }
 
   @Test
   public void test_isNull_aValue()
   {
-    writeToMemory(new Object[]{5});
+    writeToMemory(LONGS_ARRAY_1, MEMORY_POSITION);
     Assert.assertFalse(new LongArrayFieldReader().isNull(memory, MEMORY_POSITION));
   }
 
   @Test
-  public void test_makeColumnValueSelector_defaultOrNull()
+  public void test_isNull_emptyArray()
   {
-    writeToMemory(NullHandling.defaultLongValue());
+    writeToMemory(new Object[]{}, MEMORY_POSITION);
+    Assert.assertFalse(new LongArrayFieldReader().isNull(memory, MEMORY_POSITION));
+  }
 
-    final ColumnValueSelector<?> readSelector =
-        LongFieldReader.forPrimitive().makeColumnValueSelector(memory, new ConstantFieldPointer(MEMORY_POSITION));
+  @Test
+  public void test_isNull_arrayWithSingleNullElement()
+  {
+    writeToMemory(new Object[]{null}, MEMORY_POSITION);
+    Assert.assertFalse(new LongArrayFieldReader().isNull(memory, MEMORY_POSITION));
+  }
+
+  @Test
+  public void test_makeColumnValueSelector_null()
+  {
+    writeToMemory(null, MEMORY_POSITION);
 
-    Assert.assertEquals(!NullHandling.replaceWithDefault(), readSelector.isNull());
+    final ColumnValueSelector<?> readSelector =
+        new LongArrayFieldReader().makeColumnValueSelector(memory, new ConstantFieldPointer(MEMORY_POSITION));
 
-    if (NullHandling.replaceWithDefault()) {
-      Assert.assertEquals((long) NullHandling.defaultLongValue(), readSelector.getLong());
-    }
+    Assert.assertTrue(readSelector.isNull());
   }
 
+  // TODO(laksh): Add a comment about why null doesn't change to default value
   @Test
   public void test_makeColumnValueSelector_aValue()
   {
-    writeToMemory(new Object[]{5, 6, 7});
+    writeToMemory(LONGS_ARRAY_1, MEMORY_POSITION);
 
     final ColumnValueSelector<?> readSelector =
         new LongArrayFieldReader().makeColumnValueSelector(memory, new ConstantFieldPointer(MEMORY_POSITION));
 
-    assertResults(ImmutableList.of(5L, 6L, 7L), readSelector.getObject());
+    assertResults(LONGS_LIST_1, readSelector.getObject());
   }
 
-  private void writeToMemory(final Object value)
+  @Test
+  public void test_makeColumnValueSelector_multipleValues()
   {
-    Mockito.when(writeSelector.isNull()).thenReturn(value == null);
+    long sz = writeToMemory(LONGS_ARRAY_1, MEMORY_POSITION);
+    writeToMemory(LONGS_ARRAY_2, MEMORY_POSITION + sz);
+    IndexArrayFieldPointer pointer = new IndexArrayFieldPointer(ImmutableList.of(MEMORY_POSITION, MEMORY_POSITION + sz));
 
-    if (value != null) {
-      Mockito.when(writeSelector.getObject()).thenReturn(value);
-    }
 
-    if (fieldWriter.writeTo(memory, MEMORY_POSITION, memory.getCapacity() - MEMORY_POSITION) < 0) {
+    final ColumnValueSelector<?> readSelector = new LongArrayFieldReader().makeColumnValueSelector(memory, pointer);
+
+    pointer.setPointer(0);
+    assertResults(LONGS_LIST_1, readSelector.getObject());
+
+    pointer.setPointer(1);
+    assertResults(LONGS_LIST_2, readSelector.getObject());
+  }
+
+  @Test
+  public void test_makeColumnValueSelector_emptyArray()
+  {
+    writeToMemory(new Object[]{}, MEMORY_POSITION);
+
+    final ColumnValueSelector<?> readSelector =
+        new LongArrayFieldReader().makeColumnValueSelector(memory, new ConstantFieldPointer(MEMORY_POSITION));
+
+    assertResults(Collections.emptyList(), readSelector.getObject());
+  }
+
+  @Test
+  public void test_makeColumnValueSelector_arrayWithSingleNullElement()
+  {
+    writeToMemory(new Object[]{null}, MEMORY_POSITION);
+
+    final ColumnValueSelector<?> readSelector =
+        new LongArrayFieldReader().makeColumnValueSelector(memory, new ConstantFieldPointer(MEMORY_POSITION));
+
+    assertResults(Collections.singletonList(null), readSelector.getObject());
+  }
+
+  private long writeToMemory(final Object value, final long initialPosition)
+  {
+    Mockito.when(writeSelector.getObject()).thenReturn(value);
+
+    long bytesWritten = fieldWriter.writeTo(memory, initialPosition, memory.getCapacity() - initialPosition);
+    if (bytesWritten < 0) {
       throw new ISE("Could not write");
     }
+    return bytesWritten;
   }
 
   private void assertResults(List<Long> expected, Object actual)

diff --git a/processing/src/test/java/org/apache/druid/frame/field/TransformUtilsTest.java b/processing/src/test/java/org/apache/druid/frame/field/TransformUtilsTest.java
@@ -26,6 +26,7 @@
 
 import java.util.List;
 
+// TODO(laksh): Javadoc for this method and its implementation class
 public class TransformUtilsTest
 {
-Original file line number
+Diff line change
@@ Expand Up / @@ -26,6 +26,7 @@ @@
     import java.util.List;
+    // TODO(laksh): Javadoc for this method and its implementation class
     public class TransformUtilsTest
     {
@@ Expand Down @@