Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ExactMatch Filter for Non-Convertible Types; Use RangeFilter on QueryScope Vars #5587

Merged
merged 46 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
24f6f65
Fix ExactMatch Filter for Non-Convertible Types
nbauernfeind Jun 7, 2024
1bec5c3
Add LocalDate, LocalTime, LocalDateTime, ZoneDateTime Converters
nbauernfeind Jun 7, 2024
89eee34
Fix #5526 by Sharing Conversion With MatchFilter
nbauernfeind Jun 7, 2024
480bedf
Fix ZonedDateTime and Array Shenanigans
nbauernfeind Jun 7, 2024
45e69bb
cleanup personal review
nbauernfeind Jun 7, 2024
bd53581
spotless
nbauernfeind Jun 7, 2024
0786673
Allow Inverted Versions of Filter Regex
nbauernfeind Jun 7, 2024
b210076
Rename RangeFilter
nbauernfeind Jun 11, 2024
286f51b
f
nbauernfeind Jun 11, 2024
da8bcdf
Add failover to MatchFilter
nbauernfeind Jun 11, 2024
9b3fe23
reduce regex to single pattern
nbauernfeind Jun 11, 2024
7488c97
fix spotless
nbauernfeind Jun 12, 2024
df7e0b1
bugfix
nbauernfeind Jun 12, 2024
18a8ab7
where filter test fixes
nbauernfeind Jun 12, 2024
804d950
where filter factory test
nbauernfeind Jun 12, 2024
46bf831
Update engine/table/src/main/java/io/deephaven/engine/table/impl/sele…
nbauernfeind Jun 18, 2024
136c718
Update engine/table/src/main/java/io/deephaven/engine/table/impl/sele…
nbauernfeind Jun 18, 2024
e0951fe
Update engine/table/src/main/java/io/deephaven/engine/table/impl/sele…
nbauernfeind Jun 18, 2024
2795e17
Update engine/table/src/main/java/io/deephaven/engine/table/impl/sele…
nbauernfeind Jun 18, 2024
d098d21
Update engine/table/src/main/java/io/deephaven/engine/table/impl/sele…
nbauernfeind Jun 18, 2024
6982d74
Ryan's feedback
nbauernfeind Jun 18, 2024
61fb256
rename
nbauernfeind Jun 18, 2024
9d80d20
spotlesS
nbauernfeind Jun 18, 2024
b58f235
hard code mirroring
nbauernfeind Jun 18, 2024
4fc140f
Ryan's Comments Continued
nbauernfeind Jun 18, 2024
4e68b63
Merge remote-tracking branch 'upstream/main' into gh_5584
nbauernfeind Jun 18, 2024
5e5d4cb
other feedback
nbauernfeind Jun 21, 2024
aff8a28
more feedback from rnd1
nbauernfeind Jun 21, 2024
cf07196
spotless
nbauernfeind Jun 21, 2024
628fe02
quick fix
nbauernfeind Jun 21, 2024
73ffe40
whitespace fix
nbauernfeind Jun 21, 2024
15d66d5
Add fallback tests and bugfix
nbauernfeind Jun 21, 2024
1dbab30
Ryan's rnd2 direct suggestions.
nbauernfeind Jun 24, 2024
1befd4d
Ryan's rnd2 feedback
nbauernfeind Jun 24, 2024
1ddde7d
Quick compilation fixes
nbauernfeind Jun 24, 2024
24315c0
Few More Filter Delegation Fixes
nbauernfeind Jun 24, 2024
dd801eb
DateTimeUtil Tests
nbauernfeind Jun 24, 2024
eaf10ee
spotless
nbauernfeind Jun 24, 2024
e92cc09
Ryan's rnd3 feedback
nbauernfeind Jun 24, 2024
4441809
Merge remote-tracking branch 'upstream/main' into gh_5584
nbauernfeind Jun 24, 2024
3d211fc
Make ZonedDateTimeRangeFilter compare ZoneDateTimes
nbauernfeind Jun 24, 2024
c87ba9a
Force column names to take precedence
nbauernfeind Jun 24, 2024
e07f2f0
Fix type coercion from one numeric to another
nbauernfeind Jun 24, 2024
f18e93e
Make actual null work properly w/coercion
nbauernfeind Jun 25, 2024
45c4c35
Also coerce BigDecimal and BigInteger
nbauernfeind Jun 25, 2024
2ebba51
revert commented tests
nbauernfeind Jun 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,7 @@ private PreAndPostFilters applyFilterRenamings(WhereFilter[] filters) {
} else if (filter instanceof MatchFilter) {
final MatchFilter matchFilter = (MatchFilter) filter;
Assert.assertion(myRenames.size() == 1, "Match Filters should only use one column!");
String newName = myRenames.get(matchFilter.getColumnName());
Assert.neqNull(newName, "newName");
final MatchFilter newFilter = matchFilter.renameFilter(newName);
final WhereFilter newFilter = matchFilter.renameFilter(myRenames);
newFilter.init(tableReference.getDefinition(), compilationProcessor);
preViewFilters.add(newFilter);
} else if (filter instanceof ConditionFilter) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import io.deephaven.api.literal.Literal;
import io.deephaven.base.string.cache.CompressedString;
import io.deephaven.base.verify.Assert;
import io.deephaven.engine.liveness.LivenessScopeStack;
import io.deephaven.engine.rowset.RowSet;
import io.deephaven.engine.rowset.WritableRowSet;
Expand Down Expand Up @@ -52,8 +53,8 @@ static MatchFilter ofLiterals(
literals.stream().map(AsObject::of).toArray());
}

/** A fail-over where filter supplier should the match filter initialization fail. */
private final CachingSupplier<WhereFilter> failoverFilter;
/** A fail-over WhereFilter supplier should the match filter initialization fail. */
private final CachingSupplier<ConditionFilter> failoverFilter;

@NotNull
private String columnName;
nbauernfeind marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -84,8 +85,6 @@ public enum CaseSensitivity {
MatchCase, IgnoreCase
}

// TODO NATE NOCOMMIT: USE BUILDER TO CREATE MATCH FILTER??

public MatchFilter(
@NotNull final MatchType matchType,
@NotNull final String columnName,
Expand Down Expand Up @@ -113,7 +112,7 @@ public MatchFilter(
}

public MatchFilter(
rcaudy marked this conversation as resolved.
Show resolved Hide resolved
@Nullable final CachingSupplier<WhereFilter> failoverFilter,
@Nullable final CachingSupplier<ConditionFilter> failoverFilter,
@NotNull final CaseSensitivity sensitivity,
@NotNull final MatchType matchType,
@NotNull final String columnName,
Expand All @@ -122,7 +121,7 @@ public MatchFilter(
}

private MatchFilter(
@Nullable final CachingSupplier<WhereFilter> failoverFilter,
@Nullable final CachingSupplier<ConditionFilter> failoverFilter,
@NotNull final CaseSensitivity sensitivity,
@NotNull final MatchType matchType,
@NotNull final String columnName,
Expand All @@ -136,8 +135,20 @@ private MatchFilter(
this.values = values;
}

public MatchFilter renameFilter(String newName) {
private ConditionFilter getFailoverFilterIfCached() {
return failoverFilter != null ? failoverFilter.getIfCached() : null;
}

public WhereFilter renameFilter(Map<String, String> renames) {
final ConditionFilter failover = getFailoverFilterIfCached();
if (failover != null) {
return failover.renameFilter(renames);
}

final String newName = renames.get(columnName);
Assert.neqNull(newName, "newName");
if (strValues == null) {
// when we're constructed with values then there is no failover filter
return new MatchFilter(getMatchType(), newName, values);
nbauernfeind marked this conversation as resolved.
Show resolved Hide resolved
} else {
return new MatchFilter(
Expand All @@ -146,10 +157,6 @@ public MatchFilter renameFilter(String newName) {
}
}

public String getColumnName() {
return columnName;
}

public Object[] getValues() {
return values;
}
Expand All @@ -164,11 +171,25 @@ public MatchType getMatchType() {

@Override
public List<String> getColumns() {
if (!initialized) {
throw new IllegalStateException("Filter must be initialized to invoke getColumnName");
}
final WhereFilter failover = getFailoverFilterIfCached();
if (failover != null) {
return failover.getColumns();
}
return Collections.singletonList(columnName);
}

@Override
public List<String> getColumnArrays() {
if (!initialized) {
throw new IllegalStateException("Filter must be initialized to invoke getColumnArrays");
}
final WhereFilter failover = getFailoverFilterIfCached();
if (failover != null) {
return failover.getColumnArrays();
}
return Collections.emptyList();
}

Expand Down Expand Up @@ -260,7 +281,7 @@ public Stream<NotificationQueue.Dependency> getDependencyStream() {
@Override
public WritableRowSet filter(
@NotNull RowSet selection, @NotNull RowSet fullSet, @NotNull Table table, boolean usePrev) {
final WhereFilter failover = failoverFilter != null ? failoverFilter.getIfCached() : null;
final WhereFilter failover = getFailoverFilterIfCached();
if (failover != null) {
return failover.filter(selection, fullSet, table, usePrev);
}
Expand All @@ -273,7 +294,7 @@ public WritableRowSet filter(
@Override
public WritableRowSet filterInverse(
@NotNull RowSet selection, @NotNull RowSet fullSet, @NotNull Table table, boolean usePrev) {
final WhereFilter failover = failoverFilter != null ? failoverFilter.getIfCached() : null;
final WhereFilter failover = getFailoverFilterIfCached();
if (failover != null) {
return failover.filterInverse(selection, fullSet, table, usePrev);
}
Expand All @@ -284,7 +305,7 @@ public WritableRowSet filterInverse(

@Override
public boolean isSimpleFilter() {
final WhereFilter failover = failoverFilter != null ? failoverFilter.getIfCached() : null;
final WhereFilter failover = getFailoverFilterIfCached();
if (failover != null) {
return failover.isSimpleFilter();
}
Expand Down Expand Up @@ -717,15 +738,12 @@ public boolean canMemoize() {
public WhereFilter copy() {
final MatchFilter copy;
if (strValues != null) {
final WhereFilter failover = failoverFilter != null ? failoverFilter.getIfCached() : null;
nbauernfeind marked this conversation as resolved.
Show resolved Hide resolved
if (failover != null) {
return failover.copy();
}

copy = new MatchFilter(
nbauernfeind marked this conversation as resolved.
Show resolved Hide resolved
failoverFilter, caseInsensitive ? CaseSensitivity.IgnoreCase : CaseSensitivity.MatchCase,
failoverFilter == null ? null : new CachingSupplier<>(() -> failoverFilter.get().copy()),
caseInsensitive ? CaseSensitivity.IgnoreCase : CaseSensitivity.MatchCase,
getMatchType(), columnName, strValues, null);
} else {
// when we're constructed with values then there is no failover filter
copy = new MatchFilter(getMatchType(), columnName, values);
nbauernfeind marked this conversation as resolved.
Show resolved Hide resolved
}
if (initialized) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import io.deephaven.engine.rowset.WritableRowSet;
import io.deephaven.engine.rowset.RowSet;
import io.deephaven.gui.table.filters.Condition;
import io.deephaven.util.annotations.VisibleForTesting;
import io.deephaven.util.type.TypeUtils;
import org.apache.commons.lang3.mutable.MutableObject;
import org.jetbrains.annotations.NotNull;
Expand All @@ -23,7 +24,6 @@
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.ZonedDateTime;
import java.util.Collections;
import java.util.List;

/**
Expand Down Expand Up @@ -127,12 +127,23 @@ private static Condition conditionFromString(String conditionString) {

@Override
public List<String> getColumns() {
return Collections.singletonList(columnName);
if (filter == null) {
throw new IllegalStateException("Filter must be initialized to invoke getColumnName");
}
return filter.getColumns();
}

@Override
public List<String> getColumnArrays() {
return Collections.emptyList();
if (filter == null) {
throw new IllegalStateException("Filter must be initialized to invoke getColumnArrays");
}
return filter.getColumnArrays();
}

@VisibleForTesting
public WhereFilter getRealFilter() {
return filter;
}

@Override
Expand Down Expand Up @@ -323,10 +334,6 @@ public void setRecomputeListener(RecomputeListener listener) {}

@Override
public WhereFilter copy() {
if (filter != null) {
return filter.copy();
}

return new RangeFilter(columnName, condition, value, expression, filter, parserConfiguration);
nbauernfeind marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,17 @@ interface RecomputeListener {

/**
* Get the columns required by this select filter.
* <p>
* This filter must already be initialized before calling this method.
*
* @return the columns used as input by this select filter.
*/
List<String> getColumns();

/**
* Get the array columns required by this select filter.
* <p>
* This filter must already be initialized before calling this method.
*
* @return the columns used as array input by this select filter.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ public WhereFilter getExpression(String expression, Matcher matcher, Object... a
log.debug().append("WhereFilterFactory creating MatchFilter for expression: ").append(expression)
.endl();
return new MatchFilter(
new CachingSupplier<>(
() -> ConditionFilter.createConditionFilter(expression, parserConfiguration)),
new CachingSupplier<>(() -> (ConditionFilter) ConditionFilter.createConditionFilter(expression,
parserConfiguration)),
CaseSensitivity.MatchCase,
inverted ? MatchType.Inverted : MatchType.Regular,
columnName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,4 @@ public void tearDown() throws Exception {
QueryTable.FORCE_PARALLEL_WHERE = oldParallel;
QueryTable.DISABLE_PARALLEL_WHERE = oldDisable;
}

@Override
public void testRangeFilterFallback() {
super.testRangeFilterFallback();
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -1379,17 +1379,14 @@ public void testMatchFilterFallback() {
final MatchFilter filter = new MatchFilter(
new CachingSupplier<>(() -> {
called.setValue(true);
return ConditionFilter.createConditionFilter("var1 != var2");
return (ConditionFilter) ConditionFilter.createConditionFilter("var1 != var2");
}),
MatchFilter.CaseSensitivity.IgnoreCase, MatchFilter.MatchType.Inverted, "var1", "var2");

final Table result = table.where(filter);
assertTableEquals(table, result);

Assert.eqTrue(called.booleanValue(), "called.booleanValue()");

final WhereFilter copyFilter = filter.copy();
Assert.eqTrue(copyFilter instanceof ConditionFilter, "copyFilter instanceof ConditionFilter");
}

@Test
Expand All @@ -1404,7 +1401,7 @@ public void testRangeFilterFallback() {
final Table result = table.where(filter);
assertTableEquals(table, result);

final WhereFilter copyFilter = filter.copy();
Assert.eqTrue(copyFilter instanceof ConditionFilter, "copyFilter instanceof ConditionFilter");
final WhereFilter realFilter = filter.getRealFilter();
Assert.eqTrue(realFilter instanceof ConditionFilter, "realFilter instanceof ConditionFilter");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5050,11 +5050,10 @@ public static LocalDateTime parseLocalDateTime(@NotNull final String s) {
* Parses the string argument as a {@link LocalDateTime}.
* <p>
* Date time strings are formatted according to the ISO 8601 date time format
* {@code yyyy-MM-ddThh:mm:ss[.SSSSSSSSS] TZ} and others.
* {@code yyyy-MM-ddThh:mm:ss[.SSSSSSSSS]} and others.
*
* @param s date time string
* @return a {@link LocalDateTime} represented by the input string, or {@code null} if the string can not be parsed
* @see DateTimeFormatter#ISO_INSTANT
*/
@ScriptApi
@Nullable
Expand Down
Loading
Loading