[ESQL] Plumb through ranges and warnings for the casting to double tests #99452
Conversation
elasticsearchmachine
added
the
Team:QL (Deprecated)
|
Pinging @elastic/es-ql (Team:QL) |
|
Pinging @elastic/elasticsearch-esql (:Query Languages/ES|QL) |
| @@ -106,7 +106,7 @@ public static List<TestCaseSupplier> forUnaryCastingToDouble( | |||
| i -> expected.applyAsDouble(i), | |||
| min.intValue(), | |||
| max.intValue(), | |||
| List.of() | |||
| warnings | |||
There was a problem hiding this comment.
question: what are examples where we expect to have a list of warnings that we assert for all tested data types? In this PR, I only see an empty list of warnings everywhere.
There was a problem hiding this comment.
The idea is that we would use the min and max bounds settings to define ranges where the function returns a value, and ranges where it returns null with a warning. I haven't added any new warnings in this PR, because I'm just trying to get the shared code part merged quickly to avoid conflicts, but you can see some in the log10 tests
Short term, meaning over the next few PRs, I'm working on #98698, which will require a lot more warnings from functions, and at the moment that's blocked on these test generators being able to accept warnings and defined ranges.
| for (DataType lhsType : EsqlDataTypes.types()) { | ||
| if (lhsType.isNumeric() == false || EsqlDataTypes.isRepresentable(lhsType) == false) { | ||
| continue; | ||
| } | ||
| for (Map.Entry<String, Supplier<Object>> lhsSupplier : RANDOM_VALUE_SUPPLIERS.get(lhsType)) { | ||
| for (DataType rhsType : EsqlDataTypes.types()) { | ||
| if (rhsType.isNumeric() == false || EsqlDataTypes.isRepresentable(rhsType) == false) { | ||
| continue; | ||
| List<TypedDataSupplier> lhsSuppliers = new ArrayList<>(); | ||
| List<TypedDataSupplier> rhsSuppliers = new ArrayList<>(); | ||
|
|
||
| lhsSuppliers.addAll(intCases(lhsMin.intValue(), lhsMax.intValue())); | ||
| lhsSuppliers.addAll(longCases(lhsMin.longValue(), lhsMax.longValue())); | ||
| lhsSuppliers.addAll(ulongCases(BigInteger.valueOf((long) Math.ceil(lhsMin)), BigInteger.valueOf((long) Math.floor(lhsMax)))); | ||
| lhsSuppliers.addAll(doubleCases(lhsMin, lhsMax)); | ||
|
|
||
| rhsSuppliers.addAll(intCases(rhsMin.intValue(), rhsMax.intValue())); | ||
| rhsSuppliers.addAll(longCases(rhsMin.longValue(), rhsMax.longValue())); | ||
| rhsSuppliers.addAll(ulongCases(BigInteger.valueOf((long) Math.ceil(rhsMin)), BigInteger.valueOf((long) Math.floor(rhsMax)))); | ||
| rhsSuppliers.addAll(doubleCases(rhsMin, rhsMax)); |
There was a problem hiding this comment.
praise: I like the more explicit approach of listing all the test case suppliers, rather than filtering based on some predicates, since this is easier for me to read and quickly understand what is going to be tested.
Add the warnings and range checking parameters to unary and binary casting to double test generators. I also moved the data type to the value supplier, which the binary case needed. That feels more right - that's what I was intending with
TypedDatato begin with, but our abstractions are still messy here.