Skip to content

Commit

Permalink
ESQL: Handle add near the end of time (#107968)
Browse files Browse the repository at this point in the history
This updates our tests to handle the case where you are adding a
`TemporalAmount` to a date that is new `Long.MAX_VALUE`. It also adds a
test case that generates dates new that time just so we catch errors out
that way faster.

Closes #107817
  • Loading branch information
nik9000 committed Apr 30, 2024
1 parent 58e199c commit a00345f
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 29 deletions.
Expand Up @@ -427,14 +427,38 @@ public static List<TestCaseSupplier> forBinaryNotCasting(
List<TypedDataSupplier> rhsSuppliers,
List<String> warnings,
boolean symmetric
) {
return forBinaryNotCasting(
name,
lhsName,
rhsName,
expected,
expectedType,
lhsSuppliers,
rhsSuppliers,
(lhs, rhs) -> warnings,
symmetric
);
}

public static List<TestCaseSupplier> forBinaryNotCasting(
String name,
String lhsName,
String rhsName,
BinaryOperator<Object> expected,
DataType expectedType,
List<TypedDataSupplier> lhsSuppliers,
List<TypedDataSupplier> rhsSuppliers,
BiFunction<TypedData, TypedData, List<String>> warnings,
boolean symmetric
) {
List<TestCaseSupplier> suppliers = new ArrayList<>();
casesCrossProduct(
expected,
lhsSuppliers,
rhsSuppliers,
(lhsType, rhsType) -> name + "[" + lhsName + "=Attribute[channel=0], " + rhsName + "=Attribute[channel=1]]",
(lhs, rhs) -> warnings,
warnings,
suppliers,
expectedType,
symmetric
Expand Down Expand Up @@ -970,6 +994,12 @@ public static List<TypedDataSupplier> dateCases() {
// 2286-11-20T17:46:40Z - +292278994-08-17T07:12:55.807Z
() -> ESTestCase.randomLongBetween(10 * (long) 10e11, Long.MAX_VALUE),
DataTypes.DATETIME
),
new TypedDataSupplier(
"<near the end of time>",
// very close to +292278994-08-17T07:12:55.807Z, the maximum supported millis since epoch
() -> ESTestCase.randomLongBetween(Long.MAX_VALUE / 100 * 99, Long.MAX_VALUE),
DataTypes.DATETIME
)
);
}
Expand Down
Expand Up @@ -21,9 +21,12 @@
import java.math.BigInteger;
import java.time.Duration;
import java.time.Period;
import java.time.temporal.TemporalAmount;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.function.BiFunction;
import java.util.function.BinaryOperator;
import java.util.function.Supplier;

import static org.elasticsearch.xpack.esql.expression.predicate.operator.arithmetic.AbstractArithmeticTestCase.arithmeticExceptionOverflowCase;
Expand Down Expand Up @@ -116,29 +119,35 @@ public static Iterable<Object[]> parameters() {
)
);

BinaryOperator<Object> result = (lhs, rhs) -> {
try {
return addDatesAndTemporalAmount(lhs, rhs);
} catch (ArithmeticException e) {
return null;
}
};
BiFunction<TestCaseSupplier.TypedData, TestCaseSupplier.TypedData, List<String>> warnings = (lhs, rhs) -> {
try {
addDatesAndTemporalAmount(lhs.data(), rhs.data());
return List.of();
} catch (ArithmeticException e) {
return List.of(
"Line -1:-1: evaluation of [] failed, treating result as null. Only first 20 failures recorded.",
"Line -1:-1: java.lang.ArithmeticException: long overflow"
);
}
};
suppliers.addAll(
TestCaseSupplier.forBinaryNotCasting(
// TODO: There is an evaluator for Datetime + Period, so it should be tested. Similarly below.
"No evaluator, the tests only trigger the folding code since Period is not representable",
"lhs",
"rhs",
(lhs, rhs) -> {
// this weird casting dance makes the expected value lambda symmetric
Long date;
Period period;
if (lhs instanceof Long) {
date = (Long) lhs;
period = (Period) rhs;
} else {
date = (Long) rhs;
period = (Period) lhs;
}
return asMillis(asDateTime(date).plus(period));
},
result,
DataTypes.DATETIME,
TestCaseSupplier.dateCases(),
TestCaseSupplier.datePeriodCases(),
List.of(),
warnings,
true
)
);
Expand All @@ -148,23 +157,11 @@ public static Iterable<Object[]> parameters() {
"No evaluator, the tests only trigger the folding code since Duration is not representable",
"lhs",
"rhs",
(lhs, rhs) -> {
// this weird casting dance makes the expected value lambda symmetric
Long date;
Duration duration;
if (lhs instanceof Long) {
date = (Long) lhs;
duration = (Duration) rhs;
} else {
date = (Long) rhs;
duration = (Duration) lhs;
}
return asMillis(asDateTime(date).plus(duration));
},
result,
DataTypes.DATETIME,
TestCaseSupplier.dateCases(),
TestCaseSupplier.timeDurationCases(),
List.of(),
warnings,
true
)
);
Expand Down Expand Up @@ -269,6 +266,20 @@ private static String addErrorMessageString(boolean includeOrdinal, List<Set<Dat
}
}

private static Object addDatesAndTemporalAmount(Object lhs, Object rhs) {
// this weird casting dance makes the expected value lambda symmetric
Long date;
TemporalAmount period;
if (lhs instanceof Long) {
date = (Long) lhs;
period = (TemporalAmount) rhs;
} else {
date = (Long) rhs;
period = (TemporalAmount) lhs;
}
return asMillis(asDateTime(date).plus(period));
}

@Override
protected Expression build(Source source, List<Expression> args) {
return new Add(source, args.get(0), args.get(1));
Expand Down

0 comments on commit a00345f

Please sign in to comment.