Skip to content

Commit

Permalink
[ESQL] date nanos binary comparisons (elastic#111908)
Browse files Browse the repository at this point in the history
resolves elastic#109992

Nothing fancy here. Nanosecond dates are still longs, and we can just compare them as longs. Please note that, as mentioned in the linked issue, this only supports comparing date nanos to other date nanos, and not comparing to millisecond dates. With the cast functions added in elastic#111850, users can explicitly cast to millisecond dates (or longs) to compare nanos to other things.
  • Loading branch information
not-napoleon authored and davidkyle committed Sep 5, 2024
1 parent 596e78c commit 66a4d12
Show file tree
Hide file tree
Showing 12 changed files with 96 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public class Equals extends EsqlBinaryComparison implements Negatable<EsqlBinary
Map.entry(DataType.LONG, EqualsLongsEvaluator.Factory::new),
Map.entry(DataType.UNSIGNED_LONG, EqualsLongsEvaluator.Factory::new),
Map.entry(DataType.DATETIME, EqualsLongsEvaluator.Factory::new),
Map.entry(DataType.DATE_NANOS, EqualsLongsEvaluator.Factory::new),
Map.entry(DataType.GEO_POINT, EqualsGeometriesEvaluator.Factory::new),
Map.entry(DataType.CARTESIAN_POINT, EqualsGeometriesEvaluator.Factory::new),
Map.entry(DataType.GEO_SHAPE, EqualsGeometriesEvaluator.Factory::new),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public class GreaterThan extends EsqlBinaryComparison implements Negatable<EsqlB
Map.entry(DataType.LONG, GreaterThanLongsEvaluator.Factory::new),
Map.entry(DataType.UNSIGNED_LONG, GreaterThanLongsEvaluator.Factory::new),
Map.entry(DataType.DATETIME, GreaterThanLongsEvaluator.Factory::new),
Map.entry(DataType.DATE_NANOS, GreaterThanLongsEvaluator.Factory::new),
Map.entry(DataType.KEYWORD, GreaterThanKeywordsEvaluator.Factory::new),
Map.entry(DataType.TEXT, GreaterThanKeywordsEvaluator.Factory::new),
Map.entry(DataType.VERSION, GreaterThanKeywordsEvaluator.Factory::new),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public class GreaterThanOrEqual extends EsqlBinaryComparison implements Negatabl
Map.entry(DataType.LONG, GreaterThanOrEqualLongsEvaluator.Factory::new),
Map.entry(DataType.UNSIGNED_LONG, GreaterThanOrEqualLongsEvaluator.Factory::new),
Map.entry(DataType.DATETIME, GreaterThanOrEqualLongsEvaluator.Factory::new),
Map.entry(DataType.DATE_NANOS, GreaterThanOrEqualLongsEvaluator.Factory::new),
Map.entry(DataType.KEYWORD, GreaterThanOrEqualKeywordsEvaluator.Factory::new),
Map.entry(DataType.TEXT, GreaterThanOrEqualKeywordsEvaluator.Factory::new),
Map.entry(DataType.VERSION, GreaterThanOrEqualKeywordsEvaluator.Factory::new),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public class LessThan extends EsqlBinaryComparison implements Negatable<EsqlBina
Map.entry(DataType.LONG, LessThanLongsEvaluator.Factory::new),
Map.entry(DataType.UNSIGNED_LONG, LessThanLongsEvaluator.Factory::new),
Map.entry(DataType.DATETIME, LessThanLongsEvaluator.Factory::new),
Map.entry(DataType.DATE_NANOS, LessThanLongsEvaluator.Factory::new),
Map.entry(DataType.KEYWORD, LessThanKeywordsEvaluator.Factory::new),
Map.entry(DataType.TEXT, LessThanKeywordsEvaluator.Factory::new),
Map.entry(DataType.VERSION, LessThanKeywordsEvaluator.Factory::new),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public class LessThanOrEqual extends EsqlBinaryComparison implements Negatable<E
Map.entry(DataType.LONG, LessThanOrEqualLongsEvaluator.Factory::new),
Map.entry(DataType.UNSIGNED_LONG, LessThanOrEqualLongsEvaluator.Factory::new),
Map.entry(DataType.DATETIME, LessThanOrEqualLongsEvaluator.Factory::new),
Map.entry(DataType.DATE_NANOS, LessThanOrEqualLongsEvaluator.Factory::new),
Map.entry(DataType.KEYWORD, LessThanOrEqualKeywordsEvaluator.Factory::new),
Map.entry(DataType.TEXT, LessThanOrEqualKeywordsEvaluator.Factory::new),
Map.entry(DataType.VERSION, LessThanOrEqualKeywordsEvaluator.Factory::new),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public class NotEquals extends EsqlBinaryComparison implements Negatable<EsqlBin
Map.entry(DataType.LONG, NotEqualsLongsEvaluator.Factory::new),
Map.entry(DataType.UNSIGNED_LONG, NotEqualsLongsEvaluator.Factory::new),
Map.entry(DataType.DATETIME, NotEqualsLongsEvaluator.Factory::new),
Map.entry(DataType.DATE_NANOS, NotEqualsLongsEvaluator.Factory::new),
Map.entry(DataType.GEO_POINT, NotEqualsGeometriesEvaluator.Factory::new),
Map.entry(DataType.CARTESIAN_POINT, NotEqualsGeometriesEvaluator.Factory::new),
Map.entry(DataType.GEO_SHAPE, NotEqualsGeometriesEvaluator.Factory::new),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ public static Iterable<Object[]> parameters() {
)
);
// Datetime
// TODO: I'm surprised this passes. Shouldn't there be a cast from DateTime to Long?
suppliers.addAll(
TestCaseSupplier.forBinaryNotCasting(
"EqualsLongsEvaluator",
Expand All @@ -131,6 +130,20 @@ public static Iterable<Object[]> parameters() {
)
);

suppliers.addAll(
TestCaseSupplier.forBinaryNotCasting(
"EqualsLongsEvaluator",
"lhs",
"rhs",
Object::equals,
DataType.BOOLEAN,
TestCaseSupplier.dateNanosCases(),
TestCaseSupplier.dateNanosCases(),
List.of(),
false
)
);

suppliers.addAll(
TestCaseSupplier.stringCases(
Object::equals,
Expand Down Expand Up @@ -204,7 +217,7 @@ public static Iterable<Object[]> parameters() {
}

private static String typeErrorString =
"boolean, cartesian_point, cartesian_shape, datetime, double, geo_point, geo_shape, integer, ip, keyword, long, text, "
"boolean, cartesian_point, cartesian_shape, datetime, date_nanos, double, geo_point, geo_shape, integer, ip, keyword, long, text, "
+ "unsigned_long or version";

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ public static Iterable<Object[]> parameters() {
)
);
// Datetime
// TODO: I'm surprised this passes. Shouldn't there be a cast from DateTime to Long?
suppliers.addAll(
TestCaseSupplier.forBinaryNotCasting(
"GreaterThanOrEqualLongsEvaluator",
Expand All @@ -121,6 +120,20 @@ public static Iterable<Object[]> parameters() {
)
);

suppliers.addAll(
TestCaseSupplier.forBinaryNotCasting(
"GreaterThanOrEqualLongsEvaluator",
"lhs",
"rhs",
(l, r) -> ((Number) l).longValue() >= ((Number) r).longValue(),
DataType.BOOLEAN,
TestCaseSupplier.dateNanosCases(),
TestCaseSupplier.dateNanosCases(),
List.of(),
false
)
);

suppliers.addAll(
TestCaseSupplier.stringCases(
(l, r) -> ((BytesRef) l).compareTo((BytesRef) r) >= 0,
Expand All @@ -137,7 +150,7 @@ public static Iterable<Object[]> parameters() {
o,
v,
t,
(l, p) -> "datetime, double, integer, ip, keyword, long, text, unsigned_long or version"
(l, p) -> "date_nanos, datetime, double, integer, ip, keyword, long, text, unsigned_long or version"
)
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ public static Iterable<Object[]> parameters() {
)
);
// Datetime
// TODO: I'm surprised this passes. Shouldn't there be a cast from DateTime to Long?
suppliers.addAll(
TestCaseSupplier.forBinaryNotCasting(
"GreaterThanLongsEvaluator",
Expand All @@ -121,6 +120,20 @@ public static Iterable<Object[]> parameters() {
)
);

suppliers.addAll(
TestCaseSupplier.forBinaryNotCasting(
"GreaterThanLongsEvaluator",
"lhs",
"rhs",
(l, r) -> ((Number) l).longValue() > ((Number) r).longValue(),
DataType.BOOLEAN,
TestCaseSupplier.dateNanosCases(),
TestCaseSupplier.dateNanosCases(),
List.of(),
false
)
);

suppliers.addAll(
TestCaseSupplier.stringCases(
(l, r) -> ((BytesRef) l).compareTo((BytesRef) r) > 0,
Expand All @@ -137,7 +150,7 @@ public static Iterable<Object[]> parameters() {
o,
v,
t,
(l, p) -> "datetime, double, integer, ip, keyword, long, text, unsigned_long or version"
(l, p) -> "date_nanos, datetime, double, integer, ip, keyword, long, text, unsigned_long or version"
)
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ public static Iterable<Object[]> parameters() {
)
);
// Datetime
// TODO: I'm surprised this passes. Shouldn't there be a cast from DateTime to Long?
suppliers.addAll(
TestCaseSupplier.forBinaryNotCasting(
"LessThanOrEqualLongsEvaluator",
Expand All @@ -121,6 +120,20 @@ public static Iterable<Object[]> parameters() {
)
);

suppliers.addAll(
TestCaseSupplier.forBinaryNotCasting(
"LessThanOrEqualLongsEvaluator",
"lhs",
"rhs",
(l, r) -> ((Number) l).longValue() <= ((Number) r).longValue(),
DataType.BOOLEAN,
TestCaseSupplier.dateNanosCases(),
TestCaseSupplier.dateNanosCases(),
List.of(),
false
)
);

suppliers.addAll(
TestCaseSupplier.stringCases(
(l, r) -> ((BytesRef) l).compareTo((BytesRef) r) <= 0,
Expand All @@ -137,7 +150,7 @@ public static Iterable<Object[]> parameters() {
o,
v,
t,
(l, p) -> "datetime, double, integer, ip, keyword, long, text, unsigned_long or version"
(l, p) -> "date_nanos, datetime, double, integer, ip, keyword, long, text, unsigned_long or version"
)
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,20 @@ public static Iterable<Object[]> parameters() {
)
);
// Datetime
// TODO: I'm surprised this passes. Shouldn't there be a cast from DateTime to Long?
suppliers.addAll(
TestCaseSupplier.forBinaryNotCasting(
"LessThanLongsEvaluator",
"lhs",
"rhs",
(l, r) -> ((Number) l).longValue() < ((Number) r).longValue(),
DataType.BOOLEAN,
TestCaseSupplier.dateCases(),
TestCaseSupplier.dateCases(),
List.of(),
false
)
);

suppliers.addAll(
TestCaseSupplier.forBinaryNotCasting(
"LessThanLongsEvaluator",
Expand Down Expand Up @@ -137,7 +150,7 @@ public static Iterable<Object[]> parameters() {
o,
v,
t,
(l, p) -> "datetime, double, integer, ip, keyword, long, text, unsigned_long or version"
(l, p) -> "date_nanos, datetime, double, integer, ip, keyword, long, text, unsigned_long or version"
)
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ public static Iterable<Object[]> parameters() {
)
);
// Datetime
// TODO: I'm surprised this passes. Shouldn't there be a cast from DateTime to Long?
suppliers.addAll(
TestCaseSupplier.forBinaryNotCasting(
"NotEqualsLongsEvaluator",
Expand All @@ -129,6 +128,20 @@ public static Iterable<Object[]> parameters() {
false
)
);
// Datetime
suppliers.addAll(
TestCaseSupplier.forBinaryNotCasting(
"NotEqualsLongsEvaluator",
"lhs",
"rhs",
(l, r) -> false == l.equals(r),
DataType.BOOLEAN,
TestCaseSupplier.dateNanosCases(),
TestCaseSupplier.dateNanosCases(),
List.of(),
false
)
);
suppliers.addAll(
TestCaseSupplier.stringCases(
(l, r) -> false == l.equals(r),
Expand Down Expand Up @@ -198,7 +211,7 @@ public static Iterable<Object[]> parameters() {
}

private static String typeErrorString =
"boolean, cartesian_point, cartesian_shape, datetime, double, geo_point, geo_shape, integer, ip, keyword, long, text, "
"boolean, cartesian_point, cartesian_shape, datetime, date_nanos, double, geo_point, geo_shape, integer, ip, keyword, long, text, "
+ "unsigned_long or version";

@Override
Expand Down

0 comments on commit 66a4d12

Please sign in to comment.