Add FilterIn support via java-client (#4748)

deephaven · Oct 31, 2023 · d5a47b3 · d5a47b3
1 parent bc26812
commit d5a47b3
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 5 deletions.
diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/WhereFilterAdapter.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/WhereFilterAdapter.java
@@ -100,8 +100,7 @@ public static WhereFilter of(FilterComparison comparison, boolean inverted) {
     public static WhereFilter of(FilterIn in, boolean inverted) {
         if (in.values().size() == 1) {
             // Simplified case, handles transpositions of LHS / RHS most optimally
-            final FilterComparison eq = FilterComparison.eq(in.expression(), in.values().get(0));
-            return of(eq, inverted);
+            return of(in.asEquals(), inverted);
         }
         if (in.expression() instanceof ColumnName) {
             // In the case where LHS is a column name, we want to be as efficient as possible and only read that column

diff --git a/java-client/session-dagger/src/test/java/io/deephaven/client/WhereSessionTest.java b/java-client/session-dagger/src/test/java/io/deephaven/client/WhereSessionTest.java
@@ -1,7 +1,12 @@
 package io.deephaven.client;
 
+import io.deephaven.api.ColumnName;
 import io.deephaven.api.filter.Filter;
+import io.deephaven.api.filter.FilterComparison;
+import io.deephaven.api.filter.FilterIn;
+import io.deephaven.api.literal.Literal;
 import io.deephaven.client.impl.TableHandle;
+import io.deephaven.client.impl.TableHandle.TableHandleException;
 import io.deephaven.qst.table.TableSpec;
 import io.deephaven.qst.table.TimeTable;
 import org.junit.Test;
@@ -67,6 +72,54 @@ public void allowFalse() throws InterruptedException, TableHandle.TableHandleExc
         allow(TableSpec.empty(1), Filter.ofFalse());
     }
 
+    @Test
+    public void allowInSimple() throws TableHandleException, InterruptedException {
+        final FilterIn filter = FilterIn.of(ColumnName.of("Foo"), Literal.of("Foo"));
+        allow(TableSpec.empty(1).view("Foo=`Foo`", "Bar=`Bar`").where(filter));
+    }
+
+    @Test
+    public void allowIn() throws TableHandleException, InterruptedException {
+        final FilterIn filter = FilterIn.of(ColumnName.of("Foo"), Literal.of("Foo"), Literal.of("FooBar"));
+        allow(TableSpec.empty(1).view("Foo=`Foo`", "Bar=`Bar`").where(filter));
+    }
+
+    @Test
+    public void allowNotInSimple() throws TableHandleException, InterruptedException {
+        final FilterIn filter = FilterIn.of(ColumnName.of("Foo"), Literal.of("Foo"));
+        allow(TableSpec.empty(1).view("Foo=`Foo`", "Bar=`Bar`").where(Filter.not(filter)));
+    }
+
+    @Test
+    public void allowNotIn() throws TableHandleException, InterruptedException {
+        final FilterIn filter = FilterIn.of(ColumnName.of("Foo"), Literal.of("Foo"), Literal.of("FooBar"));
+        allow(TableSpec.empty(1).view("Foo=`Foo`", "Bar=`Bar`").where(Filter.not(filter)));
+    }
+
+    @Test
+    public void allowEqComparison() throws TableHandleException, InterruptedException {
+        final FilterComparison filter = FilterComparison.eq(ColumnName.of("Foo"), Literal.of("Foo"));
+        allow(TableSpec.empty(1).view("Foo=`Foo`", "Bar=`Bar`").where(filter));
+    }
+
+    @Test
+    public void allowNeqComparison() throws TableHandleException, InterruptedException {
+        final FilterComparison filter = FilterComparison.neq(ColumnName.of("Foo"), Literal.of("Foo"));
+        allow(TableSpec.empty(1).view("Foo=`Foo`", "Bar=`Bar`").where(filter));
+    }
+
+    @Test
+    public void allowNotEqComparison() throws TableHandleException, InterruptedException {
+        final FilterComparison filter = FilterComparison.eq(ColumnName.of("Foo"), Literal.of("Foo"));
+        allow(TableSpec.empty(1).view("Foo=`Foo`", "Bar=`Bar`").where(Filter.not(filter)));
+    }
+
+    @Test
+    public void allowNotNeqComparison() throws TableHandleException, InterruptedException {
+        final FilterComparison filter = FilterComparison.neq(ColumnName.of("Foo"), Literal.of("Foo"));
+        allow(TableSpec.empty(1).view("Foo=`Foo`", "Bar=`Bar`").where(Filter.not(filter)));
+    }
+
     private void allow(TableSpec parent, String... filters)
             throws InterruptedException, TableHandle.TableHandleException {
         allow(parent, Filter.and(Filter.from(filters)));

diff --git a/java-client/session/src/main/java/io/deephaven/client/impl/BatchTableRequestBuilder.java b/java-client/session/src/main/java/io/deephaven/client/impl/BatchTableRequestBuilder.java
@@ -49,6 +49,7 @@
 import io.deephaven.proto.backplane.grpc.FetchTableRequest;
 import io.deephaven.proto.backplane.grpc.FilterTableRequest;
 import io.deephaven.proto.backplane.grpc.HeadOrTailRequest;
+import io.deephaven.proto.backplane.grpc.InCondition;
 import io.deephaven.proto.backplane.grpc.IsNullCondition;
 import io.deephaven.proto.backplane.grpc.MergeTablesRequest;
 import io.deephaven.proto.backplane.grpc.NaturalJoinTablesRequest;
@@ -807,9 +808,19 @@ public Condition visit(FilterIsNull isNull) {
         @Override
         public Condition visit(FilterComparison comparison) {
             FilterComparison preferred = comparison.maybeTranspose();
+            Operator operator = preferred.operator();
+            // Processing as single FilterIn is currently the more efficient server impl.
+            // See FilterTableGrpcImpl
+            // See io.deephaven.server.table.ops.filter.FilterFactory
+            switch (operator) {
+                case EQUALS:
+                    return visit(FilterIn.of(preferred.lhs(), preferred.rhs()));
+                case NOT_EQUALS:
+                    return visit(Filter.not(FilterIn.of(preferred.lhs(), preferred.rhs())));
+            }
             return Condition.newBuilder()
                     .setCompare(CompareCondition.newBuilder()
-                            .setOperation(adapt(preferred.operator()))
+                            .setOperation(adapt(operator))
                             .setLhs(ExpressionAdapter.adapt(preferred.lhs()))
                             .setRhs(ExpressionAdapter.adapt(preferred.rhs()))
                             .build())
@@ -818,8 +829,12 @@ public Condition visit(FilterComparison comparison) {
 
         @Override
         public Condition visit(FilterIn in) {
-            // TODO(deephaven-core#3609): Update gRPC expression / filter / literal structures
-            throw new UnsupportedOperationException("Can't build Condition with FilterIn");
+            final InCondition.Builder builder = InCondition.newBuilder()
+                    .setTarget(ExpressionAdapter.adapt(in.expression()));
+            for (Expression value : in.values()) {
+                builder.addCandidates(ExpressionAdapter.adapt(value));
+            }
+            return Condition.newBuilder().setIn(builder).build();
         }
 
         @Override

diff --git a/server/src/test/java/io/deephaven/server/table/ops/filter/NormalizeNotsTest.java b/server/src/test/java/io/deephaven/server/table/ops/filter/NormalizeNotsTest.java
@@ -90,6 +90,14 @@ public void testNormalizeNots() {
                                 CaseSensitivity.MATCH_CASE, MatchType.INVERTED)));
     }
 
+    @Test
+    public void testNormalizeNotsInCondition() {
+        final Condition aInB = in(reference("ColumnA"), reference("ColumnB"));
+        final Condition aNotInB = notIn(reference("ColumnA"), reference("ColumnB"));
+        assertFilterEquals("not(REGULAR) == INVERTED", not(aInB), aNotInB);
+        assertFilterEquals("not(INVERTED) == REGULAR", not(aNotInB), aInB);
+    }
+
     @Override
     protected Condition execute(Condition f) {
         return NormalizeNots.exec(f);

diff --git a/table-api/src/main/java/io/deephaven/api/filter/FilterIn.java b/table-api/src/main/java/io/deephaven/api/filter/FilterIn.java
@@ -61,6 +61,20 @@ public final Filter asComparisons() {
         return Filter.or(values().stream().map(this::expEq).collect(Collectors.toList()));
     }
 
+    /**
+     * Creates the logical equivalent of {@code this} as a single {@link FilterComparison#eq(Expression, Expression)}
+     * between {@link #expression()} and the only value from {@link #values()}.
+     *
+     * @return the equals-filter
+     * @throws IllegalArgumentException if {@code values().size() != 1}
+     */
+    public final FilterComparison asEquals() {
+        if (values().size() != 1) {
+            throw new IllegalArgumentException("Must only call this when values().size() == 1");
+        }
+        return expEq(values().get(0));
+    }
+
     @Check
     final void checkNotEmpty() {
         if (values().isEmpty()) {