From 16cb2f4d1aff6849600a43d08e540e43693edb7f Mon Sep 17 00:00:00 2001 From: Devin Smith Date: Fri, 6 Oct 2023 13:56:58 -0700 Subject: [PATCH] Ensure top-level java-client filters are preserved (#4607) Fixes #4606 --- .../io/deephaven/client/WhereSessionTest.java | 34 ++++++++++++- java-client/session-examples/build.gradle | 1 + .../client/examples/FilterTable.java | 50 +++++++++++++++++++ .../client/impl/BatchTableRequestBuilder.java | 22 ++++---- .../io/deephaven/sql/LogicalJoinAdapter.java | 2 +- .../io/deephaven/api/filter}/ExtractAnds.java | 10 +--- .../java/io/deephaven/api/filter/Filter.java | 12 +++++ .../io/deephaven/api/filter/FilterTest.java | 15 ++++++ 8 files changed, 124 insertions(+), 22 deletions(-) create mode 100644 java-client/session-examples/src/main/java/io/deephaven/client/examples/FilterTable.java rename {sql/src/main/java/io/deephaven/sql => table-api/src/main/java/io/deephaven/api/filter}/ExtractAnds.java (83%) 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 407bbef355d..96fae94f4b5 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,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 index 305cbad1f0e..d4bc6184f24 100644 --- 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 new file mode 100644 index 00000000000..985551e3057 --- /dev/null +++ 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 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 index f52f3f812f0..040f30ba275 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 @@ -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 index a3c1e617d02..0c4bb281fca 100644 --- 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/sql/src/main/java/io/deephaven/sql/ExtractAnds.java b/table-api/src/main/java/io/deephaven/api/filter/ExtractAnds.java similarity index 83% rename from sql/src/main/java/io/deephaven/sql/ExtractAnds.java rename to table-api/src/main/java/io/deephaven/api/filter/ExtractAnds.java index fcacbd9bc55..dc7f61cdacb 100644 --- a/sql/src/main/java/io/deephaven/sql/ExtractAnds.java +++ b/table-api/src/main/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 index 9e0b967987d..205c4a3f3ad 100644 --- 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 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 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 index 299243c1157..43f57b05f8e 100644 --- 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 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);