Skip to content

Commit

Permalink
ESQL: Better tests to AUTO_BUCKET (#107228)
Browse files Browse the repository at this point in the history 
This improves the tests for AUTO_BUCKET marginally, specifically so that
it tests all valid combinations of arguments and generates a correct
types table. This'll combine nicely with #106782 to generate the
signatures that kibana needs for it's editor.
  • Loading branch information
@nik9000
nik9000 committed Apr 9, 2024
1 parent 31c05e9 commit aba7566
Show file tree
Hide file tree
Showing 6 changed files with 180 additions and 100 deletions.
37 changes: 36 additions & 1 deletion docs/reference/esql/functions/types/auto_bucket.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,40 @@
[%header.monospaced.styled,format=dsv,separator=|]
|===
field | buckets | from | to | result

datetime | integer | datetime | datetime | datetime
datetime | integer | datetime | keyword | datetime
datetime | integer | datetime | text | datetime
datetime | integer | keyword | datetime | datetime
datetime | integer | keyword | keyword | datetime
datetime | integer | keyword | text | datetime
datetime | integer | text | datetime | datetime
datetime | integer | text | keyword | datetime
datetime | integer | text | text | datetime
double | integer | double | double | double
double | integer | double | integer | double
double | integer | double | long | double
double | integer | integer | double | double
double | integer | integer | integer | double
double | integer | integer | long | double
double | integer | long | double | double
double | integer | long | integer | double
double | integer | long | long | double
integer | integer | double | double | double
integer | integer | double | integer | double
integer | integer | double | long | double
integer | integer | integer | double | double
integer | integer | integer | integer | double
integer | integer | integer | long | double
integer | integer | long | double | double
integer | integer | long | integer | double
integer | integer | long | long | double
long | integer | double | double | double
long | integer | double | integer | double
long | integer | double | long | double
long | integer | integer | double | double
long | integer | integer | integer | double
long | integer | integer | long | double
long | integer | long | double | double
long | integer | long | integer | double
long | integer | long | long | double
|===
4 changes: 2 additions & 2 deletions x-pack/plugin/esql/qa/testFixtures/src/main/resources/meta.csv-spec
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ synopsis:keyword
"double asin(number:double|integer|long|unsigned_long)"
"double atan(number:double|integer|long|unsigned_long)"
"double atan2(y_coordinate:double|integer|long|unsigned_long, x_coordinate:double|integer|long|unsigned_long)"
"double|date auto_bucket(field:integer|long|double|date, buckets:integer, from:integer|long|double|date|string, to:integer|long|double|date|string)"
"double|date auto_bucket(field:integer|long|double|date, buckets:integer, from:integer|long|double|date|keyword|text, to:integer|long|double|date|keyword|text)"
"double avg(number:double|integer|long)"
"boolean|cartesian_point|date|double|geo_point|integer|ip|keyword|long|text|unsigned_long|version case(condition:boolean, trueValue...:boolean|cartesian_point|date|double|geo_point|integer|ip|keyword|long|text|unsigned_long|version)"
"double|integer|long|unsigned_long ceil(number:double|integer|long|unsigned_long)"
Expand Down Expand Up @@ -117,7 +117,7 @@ acos |number |"double|integer|long|unsigne
asin |number |"double|integer|long|unsigned_long" |Number between -1 and 1. If `null`, the function returns `null`.
atan |number |"double|integer|long|unsigned_long" |Numeric expression. If `null`, the function returns `null`.
atan2 |[y_coordinate, x_coordinate] |["double|integer|long|unsigned_long", "double|integer|long|unsigned_long"] |[y coordinate. If `null`\, the function returns `null`., x coordinate. If `null`\, the function returns `null`.]
auto_bucket |[field, buckets, from, to] |["integer|long|double|date", integer, "integer|long|double|date|string", "integer|long|double|date|string"] |["", "", "", ""]
auto_bucket |[field, buckets, from, to] |["integer|long|double|date", integer, "integer|long|double|date|keyword|text", "integer|long|double|date|keyword|text"] |["", "", "", ""]
avg |number |"double|integer|long" |[""]
case |[condition, trueValue] |[boolean, "boolean|cartesian_point|date|double|geo_point|integer|ip|keyword|long|text|unsigned_long|version"] |["", ""]
ceil |number |"double|integer|long|unsigned_long" |Numeric expression. If `null`, the function returns `null`.
Expand Down
4 changes: 2 additions & 2 deletions ...rc/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/AutoBucket.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ public AutoBucket(
Source source,
@Param(name = "field", type = { "integer", "long", "double", "date" }) Expression field,
@Param(name = "buckets", type = { "integer" }) Expression buckets,
@Param(name = "from", type = { "integer", "long", "double", "date", "string" }) Expression from,
@Param(name = "to", type = { "integer", "long", "double", "date", "string" }) Expression to
@Param(name = "from", type = { "integer", "long", "double", "date", "keyword", "text" }) Expression from,
@Param(name = "to", type = { "integer", "long", "double", "date", "keyword", "text" }) Expression to
) {
super(source, List.of(field, buckets, from, to));
this.field = field;
Expand Down
48 changes: 36 additions & 12 deletions .../test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,10 @@ public static ExpressionEvaluator.Factory evaluator(Expression e) {
}
Layout.Builder builder = new Layout.Builder();
buildLayout(builder, e);
assertTrue(e.resolved());
Expression.TypeResolution resolution = e.typeResolved();
if (resolution.unresolved()) {
throw new AssertionError("expected resolved " + resolution.message());
}
return EvalMapper.toEvaluator(e, builder.build());
}

Expand Down Expand Up @@ -256,7 +259,10 @@ public final void testEvaluate() {
}
return;
}
assertFalse("expected resolved", expression.typeResolved().unresolved());
Expression.TypeResolution resolution = expression.typeResolved();
if (resolution.unresolved()) {
throw new AssertionError("expected resolved " + resolution.message());
}
expression = new FoldNull().rule(expression);
assertThat(expression.dataType(), equalTo(testCase.expectedType()));
logger.info("Result type: " + expression.dataType());
Expand Down Expand Up @@ -596,6 +602,28 @@ private static String signatureType(DataType type) {
* on input types like {@link Greatest} or {@link Coalesce}.
*/
protected static List<TestCaseSupplier> anyNullIsNull(boolean entirelyNullPreservesType, List<TestCaseSupplier> testCaseSuppliers) {
return anyNullIsNull(
testCaseSuppliers,
(nullPosition, nullValueDataType, original) -> entirelyNullPreservesType == false
&& nullValueDataType == DataTypes.NULL
&& original.getData().size() == 1 ? DataTypes.NULL : original.expectedType(),
(nullPosition, original) -> original
);
}

public interface ExpectedType {
DataType expectedType(int nullPosition, DataType nullValueDataType, TestCaseSupplier.TestCase original);
}

public interface ExpectedEvaluatorToString {
Matcher<String> evaluatorToString(int nullPosition, Matcher<String> original);
}

protected static List<TestCaseSupplier> anyNullIsNull(
List<TestCaseSupplier> testCaseSuppliers,
ExpectedType expectedType,
ExpectedEvaluatorToString evaluatorToString
) {
typesRequired(testCaseSuppliers);
List<TestCaseSupplier> suppliers = new ArrayList<>(testCaseSuppliers.size());
suppliers.addAll(testCaseSuppliers);
Expand All @@ -618,15 +646,12 @@ protected static List<TestCaseSupplier> anyNullIsNull(boolean entirelyNullPreser
TestCaseSupplier.TestCase oc = original.get();
List<TestCaseSupplier.TypedData> data = IntStream.range(0, oc.getData().size()).mapToObj(i -> {
TestCaseSupplier.TypedData od = oc.getData().get(i);
if (i == finalNullPosition) {
return new TestCaseSupplier.TypedData(null, od.type(), od.name());
}
return od;
return i == finalNullPosition ? od.forceValueToNull() : od;
}).toList();
return new TestCaseSupplier.TestCase(
data,
oc.evaluatorToString(),
oc.expectedType(),
evaluatorToString.evaluatorToString(finalNullPosition, oc.evaluatorToString()),
expectedType.expectedType(finalNullPosition, oc.getData().get(finalNullPosition).type(), oc),
nullValue(),
null,
oc.getExpectedTypeError(),
Expand All @@ -649,7 +674,7 @@ protected static List<TestCaseSupplier> anyNullIsNull(boolean entirelyNullPreser
return new TestCaseSupplier.TestCase(
data,
equalTo("LiteralsEvaluator[lit=null]"),
entirelyNullPreservesType == false && oc.getData().size() == 1 ? DataTypes.NULL : oc.expectedType(),
expectedType.expectedType(finalNullPosition, DataTypes.NULL, oc),
nullValue(),
null,
oc.getExpectedTypeError(),
Expand Down Expand Up @@ -755,9 +780,8 @@ private static Stream<List<DataType>> allPermutations(int argumentCount) {
if (argumentCount == 0) {
return Stream.of(List.of());
}
if (argumentCount > 4) {
// TODO check for a limit 4. is arbitrary.
throw new IllegalArgumentException("would generate too many types");
if (argumentCount > 3) {
throw new IllegalArgumentException("would generate too many combinations");
}
Stream<List<DataType>> stream = representable().map(t -> List.of(t));
for (int i = 1; i < argumentCount; i++) {
Expand Down
8 changes: 8 additions & 0 deletions ...esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/TestCaseSupplier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1325,6 +1325,14 @@ public TypedData forceLiteral() {
return new TypedData(data, type, name, true);
}

/**
* Return a {@link TypedData} that always returns {@code null} for it's
* value without modifying anything else in the supplier.
*/
public TypedData forceValueToNull() {
return new TypedData(null, type, name, forceLiteral);
}

@Override
public String toString() {
if (type == DataTypes.UNSIGNED_LONG && data instanceof Long longData) {
Expand Down

0 comments on commit aba7566

Please sign in to comment.