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 17 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 @@ -49,4 +49,14 @@ public OUTPUT_TYPE get() {
}
return cachedResult;
}

public OUTPUT_TYPE getIfCached() {
if (hasCachedResult) { // force a volatile read
if (errorResult != null) {
throw errorResult;
}
return cachedResult;
}
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ static WhereFilter makeByteRangeFilter(String columnName, Condition condition, b
case GREATER_THAN_OR_EQUAL:
return geq(columnName, value);
default:
throw new IllegalArgumentException("RangeConditionFilter does not support condition " + condition);
throw new IllegalArgumentException("RangeFilter does not support condition " + condition);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ static WhereFilter makeCharRangeFilter(String columnName, Condition condition, c
case GREATER_THAN_OR_EQUAL:
return geq(columnName, value);
default:
throw new IllegalArgumentException("RangeConditionFilter does not support condition " + condition);
throw new IllegalArgumentException("RangeFilter does not support condition " + condition);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ static WhereFilter makeDoubleRangeFilter(String columnName, Condition condition,
case GREATER_THAN_OR_EQUAL:
return geq(columnName, value);
default:
throw new IllegalArgumentException("RangeConditionFilter does not support condition " + condition);
throw new IllegalArgumentException("RangeFilter does not support condition " + condition);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ static WhereFilter makeFloatRangeFilter(String columnName, Condition condition,
case GREATER_THAN_OR_EQUAL:
return geq(columnName, value);
default:
throw new IllegalArgumentException("RangeConditionFilter does not support condition " + condition);
throw new IllegalArgumentException("RangeFilter does not support condition " + condition);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ static WhereFilter makeIntRangeFilter(String columnName, Condition condition, in
case GREATER_THAN_OR_EQUAL:
return geq(columnName, value);
default:
throw new IllegalArgumentException("RangeConditionFilter does not support condition " + condition);
throw new IllegalArgumentException("RangeFilter does not support condition " + condition);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ static WhereFilter makeLongRangeFilter(String columnName, Condition condition, l
case GREATER_THAN_OR_EQUAL:
return geq(columnName, value);
default:
throw new IllegalArgumentException("RangeConditionFilter does not support condition " + condition);
throw new IllegalArgumentException("RangeFilter does not support condition " + condition);
}
}

Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@
*/
public class RangeFilter extends WhereFilterImpl {

private final String columnName;
private final Condition condition;
private final String value;
private String columnName;
private String value;
private Condition condition;

// The expression prior to being parsed
private final String expression;
Expand All @@ -48,7 +48,7 @@ public class RangeFilter extends WhereFilterImpl {
private final FormulaParserConfiguration parserConfiguration;

/**
* Creates a RangeConditionFilter.
* Creates a RangeFilter.
*
* @param columnName the column to filter
* @param condition the condition for filtering
Expand All @@ -59,7 +59,7 @@ public RangeFilter(String columnName, Condition condition, String value) {
}

/**
* Creates a RangeConditionFilter.
* Creates a RangeFilter.
*
* @param columnName the column to filter
* @param condition the condition for filtering
Expand All @@ -73,13 +73,13 @@ public RangeFilter(String columnName, Condition condition, String value, String
}

/**
* Creates a RangeConditionFilter.
* Creates a RangeFilter.
*
* @param columnName the column to filter
* @param conditionString the String representation of a condition for filtering
* @param value a String representation of the numeric filter value
* @param expression the original expression prior to being parsed
* @param parserConfiguration the parser configuration to useyy
* @param parserConfiguration the parser configuration to use
*/
public RangeFilter(String columnName, String conditionString, String value, String expression,
FormulaParserConfiguration parserConfiguration) {
Expand All @@ -89,7 +89,7 @@ public RangeFilter(String columnName, String conditionString, String value, Stri
// Used for copy method
private RangeFilter(String columnName, Condition condition, String value, String expression,
WhereFilter filter, FormulaParserConfiguration parserConfiguration) {
Assert.eqTrue(conditionSupported(condition), condition + " is not supported by RangeConditionFilter");
Assert.eqTrue(conditionSupported(condition), condition + " is not supported by RangeFilter");
this.columnName = columnName;
this.condition = condition;
this.value = value;
Expand Down Expand Up @@ -121,7 +121,7 @@ private static Condition conditionFromString(String conditionString) {
case ">=":
return Condition.GREATER_THAN_OR_EQUAL;
default:
throw new IllegalArgumentException(conditionString + " is not supported by RangeConditionFilter");
throw new IllegalArgumentException(conditionString + " is not supported by RangeFilter");
}
}

Expand All @@ -148,50 +148,72 @@ public void init(
return;
}

final ColumnDefinition<?> def = tableDefinition.getColumn(columnName);
RuntimeException conversionError = null;
ColumnDefinition<?> def = tableDefinition.getColumn(columnName);
if (def == null) {
throw new RuntimeException("Column \"" + columnName + "\" doesn't exist in this table, available columns: "
+ tableDefinition.getColumnNames());
if ((def = tableDefinition.getColumn(value)) != null) {
// fix up for the case where column name and variable name were swapped
String tmp = columnName;
columnName = value;
value = tmp;
condition = condition.mirror();
} else {
conversionError = new RuntimeException("Column \"" + columnName
+ "\" doesn't exist in this table, available columns: " + tableDefinition.getColumnNames());
}
}

final Class<?> colClass = def.getDataType();
final Class<?> colClass = def == null ? null : def.getDataType();
final MutableObject<Object> realValue = new MutableObject<>();

final MatchFilter.ColumnTypeConvertor convertor =
MatchFilter.ColumnTypeConvertorFactory.getConvertor(def.getDataType());
if (def != null) {
final MatchFilter.ColumnTypeConvertor convertor =
MatchFilter.ColumnTypeConvertorFactory.getConvertor(def.getDataType());

RuntimeException conversionError = null;
final MutableObject<Object> realValue = new MutableObject<>();
try {
boolean wasAnArrayType = convertor.convertValue(
def, value, compilationProcessor.getQueryScopeVariables(), realValue::setValue);
if (wasAnArrayType) {
throw new IllegalArgumentException("RangeConditionFilter does not support array types for column "
+ columnName + " with value <" + value + ">");
try {
boolean wasAnArrayType = convertor.convertValue(
def, value, compilationProcessor.getQueryScopeVariables(), realValue::setValue);
if (wasAnArrayType) {
conversionError =
new IllegalArgumentException("RangeFilter does not support array types for column "
+ columnName + " with value <" + value + ">");
}
} catch (final RuntimeException err) {
conversionError = err;
}
} catch (final RuntimeException err) {
conversionError = err;
}

if (conversionError != null) {
if (expression != null) {
filter = ConditionFilter.createConditionFilter(expression, parserConfiguration);
try {
filter = ConditionFilter.createConditionFilter(expression, parserConfiguration);
} catch (final RuntimeException ignored) {
throw conversionError;
}
} else {
throw conversionError;
}
} else if (colClass == double.class || colClass == Double.class) {
filter = DoubleRangeFilter.makeDoubleRangeFilter(columnName, condition, (double) realValue.getValue());
filter = DoubleRangeFilter.makeDoubleRangeFilter(columnName, condition,
TypeUtils.unbox((Double) realValue.getValue()));
} else if (colClass == float.class || colClass == Float.class) {
filter = FloatRangeFilter.makeFloatRangeFilter(columnName, condition, (float) realValue.getValue());
filter = FloatRangeFilter.makeFloatRangeFilter(columnName, condition,
TypeUtils.unbox((Float) realValue.getValue()));
} else if (colClass == char.class || colClass == Character.class) {
filter = CharRangeFilter.makeCharRangeFilter(columnName, condition, (char) realValue.getValue());
filter = CharRangeFilter.makeCharRangeFilter(columnName, condition,
TypeUtils.unbox((Character) realValue.getValue()));
} else if (colClass == byte.class || colClass == Byte.class) {
filter = ByteRangeFilter.makeByteRangeFilter(columnName, condition, (byte) realValue.getValue());
filter = ByteRangeFilter.makeByteRangeFilter(columnName, condition,
TypeUtils.unbox((Byte) realValue.getValue()));
} else if (colClass == short.class || colClass == Short.class) {
filter = ShortRangeFilter.makeShortRangeFilter(columnName, condition, (short) realValue.getValue());
filter = ShortRangeFilter.makeShortRangeFilter(columnName, condition,
TypeUtils.unbox((Short) realValue.getValue()));
} else if (colClass == int.class || colClass == Integer.class) {
filter = IntRangeFilter.makeIntRangeFilter(columnName, condition, (int) realValue.getValue());
filter = IntRangeFilter.makeIntRangeFilter(columnName, condition,
TypeUtils.unbox((Integer) realValue.getValue()));
} else if (colClass == long.class || colClass == Long.class) {
filter = LongRangeFilter.makeLongRangeFilter(columnName, condition, (long) realValue.getValue());
filter = LongRangeFilter.makeLongRangeFilter(columnName, condition,
TypeUtils.unbox((Long) realValue.getValue()));
} else if (colClass == Instant.class) {
filter = makeInstantRangeFilter(columnName, condition,
DateTimeUtils.epochNanos((Instant) realValue.getValue()));
Expand All @@ -216,9 +238,14 @@ public void init(
// The expression looks like a comparison of number, string, or boolean
// but the type does not match (or the column type is misconfigured)
if (expression != null) {
filter = ConditionFilter.createConditionFilter(expression, parserConfiguration);
try {
filter = ConditionFilter.createConditionFilter(expression, parserConfiguration);
} catch (final RuntimeException ignored) {
throw new IllegalArgumentException("RangeFilter does not support type "
+ colClass.getSimpleName() + " for column " + columnName);
}
} else {
throw new IllegalArgumentException("RangeConditionFilter does not support type "
throw new IllegalArgumentException("RangeFilter does not support type "
+ colClass.getSimpleName() + " for column " + columnName);
}
}
Expand All @@ -237,7 +264,7 @@ private static LongRangeFilter makeInstantRangeFilter(String columnName, Conditi
case GREATER_THAN_OR_EQUAL:
return new InstantRangeFilter(columnName, value, Long.MAX_VALUE, true, true);
default:
throw new IllegalArgumentException("RangeConditionFilter does not support condition " + condition);
throw new IllegalArgumentException("RangeFilter does not support condition " + condition);
}
}

Expand All @@ -252,7 +279,7 @@ private static LongRangeFilter makeZonedDateTimeRangeFilter(String columnName, C
case GREATER_THAN_OR_EQUAL:
return new ZonedDateTimeRangeFilter(columnName, value, Long.MAX_VALUE, true, true);
default:
throw new IllegalArgumentException("RangeConditionFilter does not support condition " + condition);
throw new IllegalArgumentException("RangeFilter does not support condition " + condition);
}
}

Expand All @@ -268,7 +295,7 @@ private static SingleSidedComparableRangeFilter makeComparableRangeFilter(String
case GREATER_THAN_OR_EQUAL:
return new SingleSidedComparableRangeFilter(columnName, comparable, true, true);
default:
throw new IllegalArgumentException("RangeConditionFilter does not support condition " + condition);
throw new IllegalArgumentException("RangeFilter does not support condition " + condition);
}
}

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

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

nbauernfeind marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -62,7 +62,7 @@ static WhereFilter makeShortRangeFilter(String columnName, Condition condition,
case GREATER_THAN_OR_EQUAL:
return geq(columnName, value);
default:
throw new IllegalArgumentException("RangeConditionFilter does not support condition " + condition);
throw new IllegalArgumentException("RangeFilter does not support condition " + condition);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@ public PreferredLhsColumnRhsVisitor(ColumnName lhs) {
this.lhs = Objects.requireNonNull(lhs);
}

// The String vs non-String cases are separated out, as it's necessary in the RangeConditionFilter case to
// wrap String literals with quotes (as that's what RangeConditionFilter expects wrt parsing). MatchFilter
// The String vs non-String cases are separated out, as it's necessary in the RangeFilter case to
// wrap String literals with quotes (as that's what RangeFilter expects wrt parsing). MatchFilter
// allows us to pass in the already parsed Object (otherwise, if we were passing strValues we would need to
// wrap them)

Expand Down
Loading
Loading