Skip to content

Commit

Permalink
Ensure top-level java-client filters are preserved (#4607)
Browse files Browse the repository at this point in the history
Fixes #4606
  • Loading branch information
devinrsmith committed Oct 6, 2023
1 parent 03da796 commit 16cb2f4
Show file tree
Hide file tree
Showing 8 changed files with 124 additions and 22 deletions.
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions java-client/session-examples/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion sql/src/main/java/io/deephaven/sql/LogicalJoinAdapter.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
12 changes: 12 additions & 0 deletions table-api/src/main/java/io/deephaven/api/filter/Filter.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
15 changes: 15 additions & 0 deletions table-api/src/test/java/io/deephaven/api/filter/FilterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 16cb2f4

Please sign in to comment.