Ensure top-level java-client filters are preserved

Fixes deephaven#4606
devinrsmith · Oct 5, 2023 · b49428f · b49428f
1 parent 87ee5ec
commit b49428f
Show file tree

Hide file tree

Showing 8 changed files with 124 additions and 22 deletions.
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,5 +1,6 @@
 package io.deephaven.client;
 
+import io.deephaven.api.filter.Filter;
 import io.deephaven.client.impl.TableHandle;
 import io.deephaven.qst.table.TableSpec;
 import io.deephaven.qst.table.TimeTable;
@@ -46,13 +47,42 @@ public void disallowNew() throws InterruptedException {
         disallow(TableSpec.empty(1), "new Object() == 42");
     }
 
-    private void allow(TableSpec parent, String filter) throws InterruptedException, TableHandle.TableHandleException {
+    @Test
+    public void allowTopLevelIn() throws InterruptedException, TableHandle.TableHandleException {
+        allow(TableSpec.empty(1).view("I=ii"), "I in 0, 1", "I > 37");
+    }
+
+    @Test
+    public void disallowNestedIn() throws InterruptedException, TableHandle.TableHandleException {
+        disallow(TableSpec.empty(1).view("I=ii"), Filter.or(Filter.from("I in 0, 1", "I > 37")));
+    }
+
+    @Test
+    public void allowTrue() throws InterruptedException, TableHandle.TableHandleException {
+        allow(TableSpec.empty(1), Filter.ofTrue());
+    }
+
+    @Test
+    public void allowFalse() throws InterruptedException, TableHandle.TableHandleException {
+        allow(TableSpec.empty(1), Filter.ofFalse());
+    }
+
+    private void allow(TableSpec parent, String... filters)
+            throws InterruptedException, TableHandle.TableHandleException {
+        allow(parent, Filter.and(Filter.from(filters)));
+    }
+
+    private void disallow(TableSpec parent, String... filters) throws InterruptedException {
+        disallow(parent, Filter.and(Filter.from(filters)));
+    }
+
+    private void allow(TableSpec parent, Filter filter) throws InterruptedException, TableHandle.TableHandleException {
         try (final TableHandle handle = session.batch().execute(parent.where(filter))) {
             assertThat(handle.isSuccessful()).isTrue();
         }
     }
 
-    private void disallow(TableSpec parent, String filter) throws InterruptedException {
+    private void disallow(TableSpec parent, Filter filter) throws InterruptedException {
         try (final TableHandle handle = session.batch().execute(parent.where(filter))) {
             failBecauseExceptionWasNotThrown(TableHandle.TableHandleException.class);
         } catch (TableHandle.TableHandleException e) {

diff --git a/java-client/session-examples/build.gradle b/java-client/session-examples/build.gradle
@@ -47,6 +47,7 @@ applicationDistribution.into('bin') {
     from(createApplication('subscribe-to-logs', 'io.deephaven.client.examples.SubscribeToLogs'))
     from(createApplication('publish', 'io.deephaven.client.examples.Publish'))
     from(createApplication('message-stream-send-receive', 'io.deephaven.client.examples.MessageStreamSendReceive'))
+    from(createApplication('filter-table', 'io.deephaven.client.examples.FilterTable'))
 
     fileMode = 0755
 }

diff --git a/java-client/session-examples/src/main/java/io/deephaven/client/examples/FilterTable.java b/java-client/session-examples/src/main/java/io/deephaven/client/examples/FilterTable.java
@@ -0,0 +1,50 @@
+/**
+ * Copyright (c) 2016-2022 Deephaven Data Labs and Patent Pending
+ */
+package io.deephaven.client.examples;
+
+import io.deephaven.api.filter.Filter;
+import io.deephaven.client.impl.Session;
+import io.deephaven.client.impl.TableHandle;
+import io.deephaven.qst.table.TableSpec;
+import picocli.CommandLine;
+import picocli.CommandLine.ArgGroup;
+import picocli.CommandLine.Command;
+import picocli.CommandLine.Option;
+import picocli.CommandLine.Parameters;
+
+import java.util.List;
+
+@Command(name = "filter-table", mixinStandardHelpOptions = true,
+        description = "Filter table using raw strings", version = "0.1.0")
+class FilterTable extends SingleSessionExampleBase {
+
+    enum Type {
+        AND, OR
+    }
+
+    @Option(names = {"--filter-type"},
+            description = "The filter type, default: ${DEFAULT-VALUE}, candidates: [ ${COMPLETION-CANDIDATES} ]",
+            defaultValue = "AND")
+    Type type;
+
+    @ArgGroup(exclusive = true, multiplicity = "1")
+    Ticket ticket;
+
+    @Parameters(arity = "1+", paramLabel = "FILTER", description = "Raw-string filters")
+    List<String> filters;
+
+    @Override
+    protected void execute(Session session) throws Exception {
+        final Filter filter = type == Type.AND ? Filter.and(Filter.from(filters)) : Filter.or(Filter.from(filters));
+        final TableSpec filtered = ticket.ticketId().table().where(filter);
+        try (final TableHandle handle = session.execute(filtered)) {
+            session.publish("filter_table_results", handle).get();
+        }
+    }
+
+    public static void main(String[] args) {
+        int execute = new CommandLine(new FilterTable()).execute(args);
+        System.exit(execute);
+    }
+}
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
@@ -281,22 +281,24 @@ public Operation visit(SnapshotWhenTable snapshotWhenTable) {
         }
 
         private Operation createFilterTableRequest(WhereTable whereTable) {
-            FilterTableRequest request = FilterTableRequest.newBuilder()
+            final FilterTableRequest.Builder builder = FilterTableRequest.newBuilder()
                     .setResultId(ticket)
-                    .setSourceId(ref(whereTable.parent()))
-                    .addFilters(FilterAdapter.of(whereTable.filter()))
-                    .build();
-            return op(Builder::setFilter, request);
+                    .setSourceId(ref(whereTable.parent()));
+            for (Filter filter : Filter.extractAnds(whereTable.filter())) {
+                builder.addFilters(FilterAdapter.of(filter));
+            }
+            return op(Builder::setFilter, builder.build());
         }
 
         private Operation createUnstructuredFilterTableRequest(WhereTable whereTable) {
             // TODO(deephaven-core#3740): Remove engine crutch on io.deephaven.api.Strings
-            UnstructuredFilterTableRequest request = UnstructuredFilterTableRequest.newBuilder()
+            final UnstructuredFilterTableRequest.Builder builder = UnstructuredFilterTableRequest.newBuilder()
                     .setResultId(ticket)
-                    .setSourceId(ref(whereTable.parent()))
-                    .addFilters(Strings.of(whereTable.filter()))
-                    .build();
-            return op(Builder::setUnstructuredFilter, request);
+                    .setSourceId(ref(whereTable.parent()));
+            for (Filter filter : Filter.extractAnds(whereTable.filter())) {
+                builder.addFilters(Strings.of(filter));
+            }
+            return op(Builder::setUnstructuredFilter, builder.build());
         }
 
         @Override

diff --git a/sql/src/main/java/io/deephaven/sql/LogicalJoinAdapter.java b/sql/src/main/java/io/deephaven/sql/LogicalJoinAdapter.java
@@ -42,7 +42,7 @@ public static TableSpec indexTable(SqlRootContext rootContext, LogicalJoin join,
         // Ideally, calcite optimizations would do expression pushdown for us (and it _might_ if configured correctly),
         // and hopefully filter pushdown where applicable.
 
-        for (Filter filter : ExtractAnds.of(FilterSimplifier.of(condition))) {
+        for (Filter filter : Filter.extractAnds(FilterSimplifier.of(condition))) {
             if (!(filter instanceof FilterComparison)) {
                 postFilterConditions.add(filter);
                 continue;

diff --git a/...in/java/io/deephaven/sql/ExtractAnds.java → .../io/deephaven/api/filter/ExtractAnds.java b/...in/java/io/deephaven/sql/ExtractAnds.java → .../io/deephaven/api/filter/ExtractAnds.java
@@ -1,17 +1,9 @@
-package io.deephaven.sql;
+package io.deephaven.api.filter;
 
 import io.deephaven.api.RawString;
 import io.deephaven.api.expression.Function;
 import io.deephaven.api.expression.Method;
-import io.deephaven.api.filter.Filter;
 import io.deephaven.api.filter.Filter.Visitor;
-import io.deephaven.api.filter.FilterAnd;
-import io.deephaven.api.filter.FilterComparison;
-import io.deephaven.api.filter.FilterIn;
-import io.deephaven.api.filter.FilterIsNull;
-import io.deephaven.api.filter.FilterNot;
-import io.deephaven.api.filter.FilterOr;
-import io.deephaven.api.filter.FilterPattern;
 import io.deephaven.api.literal.Literal;
 
 import java.util.Collection;

diff --git a/table-api/src/main/java/io/deephaven/api/filter/Filter.java b/table-api/src/main/java/io/deephaven/api/filter/Filter.java
@@ -187,6 +187,18 @@ static Filter and(Collection<? extends Filter> filters) {
         return FilterAnd.of(filters);
     }
 
+    /**
+     * Performs a non-recursive "and-extraction" against {@code filter}. If {@code filter} is a {@link FilterAnd},
+     * {@link FilterAnd#filters()} will be returned. If {@code filter} is {@link Filter#ofTrue()}, an empty list will be
+     * returned. Otherwise, a singleton list of {@code filter} will be returned.
+     *
+     * @param filter the filter
+     * @return the and-extracted filter
+     */
+    static Collection<Filter> extractAnds(Filter filter) {
+        return ExtractAnds.of(filter);
+    }
+
     /**
      * The logical inversion of {@code this}. While logically equivalent to {@code Filter.not(this)}, implementations of
      * this method will return more specifically typed inversions where applicable.

diff --git a/table-api/src/test/java/io/deephaven/api/filter/FilterTest.java b/table-api/src/test/java/io/deephaven/api/filter/FilterTest.java
@@ -13,6 +13,7 @@
 import org.junit.jupiter.api.Test;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 import java.util.regex.Pattern;
 
@@ -173,6 +174,20 @@ void examplesStringsOf() {
         }
     }
 
+    @Test
+    void extractAnds() {
+        for (Filter filter : Examples.of()) {
+            final Collection<Filter> results = ExtractAnds.of(filter);
+            if (filter instanceof FilterAnd) {
+                assertThat(results).isEqualTo(((FilterAnd) filter).filters());
+            } else if (Filter.ofTrue().equals(filter)) {
+                assertThat(results).isEmpty();
+            } else {
+                assertThat(results).containsExactly(filter);
+            }
+        }
+    }
+
     private static void stringsOf(Filter filter, String expected) {
         assertThat(of(filter)).isEqualTo(expected);
         assertThat(filter.walk(FilterSpecificString.INSTANCE)).isEqualTo(expected);