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 index 8a6e25393f3..6d96b279bf7 100644 --- 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 index 96fae94f4b5..a15d078fe4f 100644 --- 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 index 040f30ba275..b200b5ab37a 100644 --- 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 index 25c314fb9b0..1199eb1134d 100644 --- 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 index b48b7190602..287f6abb3e5 100644 --- 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()) {