JavaClient: Encode Packed Booleans Instead of Reinterpretted Bytes (#…

…3812)
deephaven · May 10, 2023 · 5793f74 · 5793f74
1 parent b8fcbeb
commit 5793f74
Show file tree

Hide file tree

Showing 17 changed files with 166 additions and 33 deletions.
diff --git a/engine/updategraph/src/main/java/io/deephaven/engine/updategraph/UpdateGraphProcessor.java b/engine/updategraph/src/main/java/io/deephaven/engine/updategraph/UpdateGraphProcessor.java
@@ -566,6 +566,13 @@ public void enableUnitTestMode() {
         unitTestRefreshThreadPool = makeUnitTestRefreshExecutor();
     }
 
+    /**
+     * @return whether unit test mode is allowed
+     */
+    public boolean isUnitTestModeAllowed() {
+        return ALLOW_UNIT_TEST_MODE;
+    }
+
     /**
      * Enable the loop watchdog with the specified timeout. A value of 0 disables the watchdog.
      *

diff --git a/java-client/flight-dagger/build.gradle b/java-client/flight-dagger/build.gradle
@@ -29,7 +29,6 @@ dependencies {
     testImplementation project(':log-to-slf4j')
 }
 
-test {
-}
+test.systemProperty "UpdateGraphProcessor.allowUnitTestMode", false
 
 apply plugin: 'io.deephaven.java-open-nio'
diff --git a/java-client/flight-dagger/src/test/java/io/deephaven/client/DeephavenFlightSessionTest.java b/java-client/flight-dagger/src/test/java/io/deephaven/client/DeephavenFlightSessionTest.java
@@ -6,6 +6,7 @@
 import io.deephaven.api.TableOperations;
 import io.deephaven.api.updateby.UpdateByOperation;
 import io.deephaven.client.impl.TableHandle;
+import io.deephaven.extensions.barrage.util.BarrageUtil;
 import io.deephaven.qst.TableCreator;
 import io.deephaven.qst.column.header.ColumnHeader;
 import io.deephaven.qst.table.NewTable;
@@ -81,25 +82,25 @@ public void updateBy() throws Exception {
         }
     }
 
-    // TODO (deephaven-core#1373): Hook up doPut integration unit testing
-    // @Test
-    // public void doPutStream() throws Exception {
-    // try (
-    // final TableHandle ten = flightSession.session().execute(TableSpec.empty(10).view("I=i"));
-    // // DoGet
-    // final FlightStream tenStream = flightSession.stream(ten);
-    // // DoPut
-    // final TableHandle tenAgain = flightSession.put(tenStream)) {
-    // assertThat(tenAgain.response().getSchemaHeader()).isEqualTo(ten.response().getSchemaHeader());
-    // }
-    // }
-    //
-    // @Test
-    // public void doPutNewTable() throws TableHandleException, InterruptedException {
-    // try (final TableHandle newTableHandle = flightSession.put(newTable(), bufferAllocator)) {
-    // // ignore
-    // }
-    // }
+    @Test
+    public void doPutStream() throws Exception {
+        try (final TableHandle ten = flightSession.session().execute(TableSpec.empty(10).view("I=i"));
+                // DoGet
+                final FlightStream tenStream = flightSession.stream(ten);
+                // DoPut
+                final TableHandle tenAgain = flightSession.putExport(tenStream)) {
+            BarrageUtil.ConvertedArrowSchema tenSchema = BarrageUtil.convertArrowSchema(ten.response());
+            BarrageUtil.ConvertedArrowSchema tenAgainSchema = BarrageUtil.convertArrowSchema(tenAgain.response());
+            assertThat(tenSchema.tableDef).isEqualTo(tenAgainSchema.tableDef);
+        }
+    }
+
+    @Test
+    public void doPutNewTable() throws TableHandle.TableHandleException, InterruptedException {
+        try (final TableHandle newTableHandle = flightSession.putExport(newTable(), bufferAllocator)) {
+            // ignore
+        }
+    }
 
     private static Schema metadataLess(Schema schema) {
         return new Schema(

diff --git a/...lient/flight-dagger/src/test/java/io/deephaven/client/DeephavenFlightSessionTestBase.java b/...lient/flight-dagger/src/test/java/io/deephaven/client/DeephavenFlightSessionTestBase.java
@@ -5,32 +5,31 @@
 
 import io.deephaven.client.impl.DaggerDeephavenFlightRoot;
 import io.deephaven.client.impl.FlightSession;
-import io.deephaven.engine.testutil.junit4.EngineCleanup;
+import io.deephaven.engine.context.TestExecutionContext;
 import io.deephaven.server.runner.DeephavenApiServerTestBase;
+import io.deephaven.util.SafeCloseable;
 import io.grpc.ManagedChannel;
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.memory.RootAllocator;
 import org.junit.After;
 import org.junit.Before;
-import org.junit.Rule;
 
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 
 public abstract class DeephavenFlightSessionTestBase extends DeephavenApiServerTestBase {
 
+    SafeCloseable executionContext;
     BufferAllocator bufferAllocator;
     ScheduledExecutorService sessionScheduler;
     FlightSession flightSession;
 
-    @Rule
-    public final EngineCleanup framework = new EngineCleanup();
-
     @Override
     @Before
     public void setUp() throws Exception {
         super.setUp();
+        executionContext = TestExecutionContext.createForUnitTests().open();
         ManagedChannel channel = channelBuilder().build();
         register(channel);
         sessionScheduler = Executors.newScheduledThreadPool(2);
@@ -49,6 +48,7 @@ public void tearDown() throws Exception {
         if (!sessionScheduler.awaitTermination(5, TimeUnit.SECONDS)) {
             throw new RuntimeException("Scheduler not shutdown within 5 seconds");
         }
+        executionContext.close();
         super.tearDown();
     }
 }
diff --git a/java-client/flight/src/main/java/io/deephaven/client/impl/FieldAdapter.java b/java-client/flight/src/main/java/io/deephaven/client/impl/FieldAdapter.java
@@ -49,8 +49,7 @@ public static Field byteField(String name) {
     }
 
     public static Field booleanField(String name) {
-        // TODO(deephaven-core#43): Do not reinterpret bool as byte
-        return field(name, MinorType.TINYINT.getType(), "boolean");
+        return field(name, MinorType.BIT.getType(), "boolean");
     }
 
     public static Field charField(String name) {

diff --git a/java-client/flight/src/main/java/io/deephaven/client/impl/FieldVectorAdapter.java b/java-client/flight/src/main/java/io/deephaven/client/impl/FieldVectorAdapter.java
@@ -22,6 +22,7 @@
 import io.deephaven.qst.type.StringType;
 import org.apache.arrow.memory.BufferAllocator;
 import org.apache.arrow.vector.BigIntVector;
+import org.apache.arrow.vector.BitVector;
 import org.apache.arrow.vector.FieldVector;
 import org.apache.arrow.vector.Float4Vector;
 import org.apache.arrow.vector.Float8Vector;
@@ -118,10 +119,9 @@ public void visit(ByteArray byteArray) {
 
     @Override
     public void visit(BooleanArray booleanArray) {
-        // TODO(deephaven-core#43): Do not reinterpret bool as byte
         Field field = FieldAdapter.booleanField(name);
-        TinyIntVector vector = new TinyIntVector(field, allocator);
-        VectorHelper.fill(vector, booleanArray.values(), 0, booleanArray.size());
+        BitVector vector = new BitVector(field, allocator);
+        VectorHelper.fill(vector, booleanArray, 0, booleanArray.size());
         out = vector;
     }
 

diff --git a/java-client/flight/src/main/java/io/deephaven/client/impl/VectorHelper.java b/java-client/flight/src/main/java/io/deephaven/client/impl/VectorHelper.java
@@ -3,7 +3,9 @@
  */
 package io.deephaven.client.impl;
 
+import io.deephaven.qst.array.BooleanArray;
 import org.apache.arrow.vector.BigIntVector;
+import org.apache.arrow.vector.BitVector;
 import org.apache.arrow.vector.Float4Vector;
 import org.apache.arrow.vector.Float8Vector;
 import org.apache.arrow.vector.IntVector;
@@ -30,6 +32,19 @@ public static void fill(TinyIntVector vector, byte[] array, int offset, int len)
         vector.setValueCount(len);
     }
 
+    public static void fill(BitVector vector, BooleanArray array, int offset, int len) {
+        vector.allocateNew(len);
+        for (int i = 0; i < len; i++) {
+            Boolean value = array.value(offset + i);
+            if (value == null) {
+                vector.set(i, 0, 0);
+            } else {
+                vector.set(i, value ? 1 : 0);
+            }
+        }
+        vector.setValueCount(len);
+    }
+
     public static void fill(UInt2Vector vector, char[] array, int offset, int len) {
         vector.allocateNew(len);
         for (int i = 0; i < len; i++) {

diff --git a/qst/src/main/java/io/deephaven/qst/array/BooleanArray.java b/qst/src/main/java/io/deephaven/qst/array/BooleanArray.java
@@ -4,6 +4,7 @@
 package io.deephaven.qst.array;
 
 import io.deephaven.qst.type.BooleanType;
+import io.deephaven.util.BooleanUtils;
 
 import java.util.Arrays;
 import java.util.Collection;
@@ -61,6 +62,16 @@ public final byte[] values() {
         return values;
     }
 
+    @Override
+    public final Boolean value(int index) {
+        return BooleanUtils.byteAsBoolean(values[index]);
+    }
+
+    @Override
+    public boolean isNull(int index) {
+        return values[index] == BooleanUtils.NULL_BOOLEAN_AS_BYTE;
+    }
+
     @Override
     public final int size() {
         return values().length;

diff --git a/qst/src/main/java/io/deephaven/qst/array/ByteArray.java b/qst/src/main/java/io/deephaven/qst/array/ByteArray.java
@@ -4,6 +4,7 @@
 package io.deephaven.qst.array;
 
 import io.deephaven.qst.type.ByteType;
+import io.deephaven.util.QueryConstants;
 
 import java.util.Arrays;
 import java.util.Collection;
@@ -60,6 +61,17 @@ public final byte[] values() {
         return values;
     }
 
+    @Override
+    public Byte value(int index) {
+        byte value = values[index];
+        return value == QueryConstants.NULL_BYTE ? null : value;
+    }
+
+    @Override
+    public boolean isNull(int index) {
+        return values[index] == QueryConstants.NULL_BYTE;
+    }
+
     @Override
     public final int size() {
         return values().length;

diff --git a/qst/src/main/java/io/deephaven/qst/array/CharArray.java b/qst/src/main/java/io/deephaven/qst/array/CharArray.java
@@ -4,6 +4,7 @@
 package io.deephaven.qst.array;
 
 import io.deephaven.qst.type.CharType;
+import io.deephaven.util.QueryConstants;
 
 import java.util.Arrays;
 import java.util.Collection;
@@ -60,6 +61,17 @@ public final char[] values() {
         return values;
     }
 
+    @Override
+    public Character value(int index) {
+        char value = values[index];
+        return value == QueryConstants.NULL_CHAR ? null : value;
+    }
+
+    @Override
+    public boolean isNull(int index) {
+        return values[index] == QueryConstants.NULL_CHAR;
+    }
+
     @Override
     public final int size() {
         return values().length;

diff --git a/qst/src/main/java/io/deephaven/qst/array/DoubleArray.java b/qst/src/main/java/io/deephaven/qst/array/DoubleArray.java
@@ -4,6 +4,7 @@
 package io.deephaven.qst.array;
 
 import io.deephaven.qst.type.DoubleType;
+import io.deephaven.util.QueryConstants;
 
 import java.util.Arrays;
 import java.util.Collection;
@@ -60,6 +61,17 @@ public final double[] values() {
         return values;
     }
 
+    @Override
+    public Double value(int index) {
+        double value = values[index];
+        return value == QueryConstants.NULL_DOUBLE ? null : value;
+    }
+
+    @Override
+    public boolean isNull(int index) {
+        return values[index] == QueryConstants.NULL_DOUBLE;
+    }
+
     @Override
     public final int size() {
         return values().length;

diff --git a/qst/src/main/java/io/deephaven/qst/array/FloatArray.java b/qst/src/main/java/io/deephaven/qst/array/FloatArray.java
@@ -4,6 +4,7 @@
 package io.deephaven.qst.array;
 
 import io.deephaven.qst.type.FloatType;
+import io.deephaven.util.QueryConstants;
 
 import java.util.Arrays;
 import java.util.Collection;
@@ -60,6 +61,17 @@ public final float[] values() {
         return values;
     }
 
+    @Override
+    public Float value(int index) {
+        float value = values[index];
+        return value == QueryConstants.NULL_FLOAT ? null : value;
+    }
+
+    @Override
+    public boolean isNull(int index) {
+        return values[index] == QueryConstants.NULL_FLOAT;
+    }
+
     @Override
     public final int size() {
         return values().length;

diff --git a/qst/src/main/java/io/deephaven/qst/array/IntArray.java b/qst/src/main/java/io/deephaven/qst/array/IntArray.java
@@ -4,6 +4,7 @@
 package io.deephaven.qst.array;
 
 import io.deephaven.qst.type.IntType;
+import io.deephaven.util.QueryConstants;
 
 import java.util.Arrays;
 import java.util.Collection;
@@ -60,6 +61,17 @@ public final int[] values() {
         return values;
     }
 
+    @Override
+    public Integer value(int index) {
+        int value = values[index];
+        return value == QueryConstants.NULL_INT ? null : value;
+    }
+
+    @Override
+    public boolean isNull(int index) {
+        return values[index] == QueryConstants.NULL_INT;
+    }
+
     @Override
     public final int size() {
         return values().length;

diff --git a/qst/src/main/java/io/deephaven/qst/array/LongArray.java b/qst/src/main/java/io/deephaven/qst/array/LongArray.java
@@ -4,6 +4,7 @@
 package io.deephaven.qst.array;
 
 import io.deephaven.qst.type.LongType;
+import io.deephaven.util.QueryConstants;
 
 import java.util.Arrays;
 import java.util.Collection;
@@ -60,6 +61,17 @@ public final long[] values() {
         return values;
     }
 
+    @Override
+    public Long value(int index) {
+        long value = values[index];
+        return value == QueryConstants.NULL_LONG ? null : value;
+    }
+
+    @Override
+    public boolean isNull(int index) {
+        return values[index] == QueryConstants.NULL_LONG;
+    }
+
     @Override
     public final int size() {
         return values().length;

diff --git a/qst/src/main/java/io/deephaven/qst/array/PrimitiveArray.java b/qst/src/main/java/io/deephaven/qst/array/PrimitiveArray.java
@@ -50,10 +50,21 @@ static <T> PrimitiveArray<T> of(PrimitiveType<T> type, Collection<T> data) {
         return builder(type, data.size()).add(data).build();
     }
 
+    /**
+     * @return the boxed value at {@code index}
+     */
+    T value(int index);
+
+    /**
+     * @return whether the value at {@code index} is {@code null}
+     */
+    boolean isNull(int index);
+
     PrimitiveType<T> componentType();
 
     <V extends Visitor> V walk(V visitor);
 
+
     interface Visitor {
         void visit(ByteArray byteArray);