Skip to content

Commit 979ae58

Browse files
committed
ESQL: Add names to aggregator tests
Opts all remaining aggs into the name tests. Adds a couple more serialization tests while I'm there.
1 parent abcffc4 commit 979ae58

27 files changed

+255
-61
lines changed

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/First.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@ public First withFilter(Expression filter) {
9191
return new First(source(), field(), filter, sort);
9292
}
9393

94+
public Expression sort() {
95+
return sort;
96+
}
97+
9498
@Override
9599
public DataType dataType() {
96100
return field().dataType();

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Last.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@ public Last withFilter(Expression filter) {
9191
return new Last(source(), field(), filter, sort);
9292
}
9393

94+
public Expression sort() {
95+
return sort;
96+
}
97+
9498
@Override
9599
public DataType dataType() {
96100
return field().dataType();

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractAggregationTestCase.java

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
package org.elasticsearch.xpack.esql.expression.function;
99

10+
import joptsimple.internal.Strings;
11+
1012
import org.elasticsearch.compute.aggregation.Aggregator;
1113
import org.elasticsearch.compute.aggregation.AggregatorFunctionSupplier;
1214
import org.elasticsearch.compute.aggregation.AggregatorMode;
@@ -506,41 +508,46 @@ private void processPageGrouping(GroupingAggregator aggregator, Page inputPage,
506508
}
507509

508510
private void assertAggregatorToString(Object aggregator) {
509-
if (optIntoToAggregatorToStringChecks() == false) {
510-
return;
511-
}
512511
String expectedStart = switch (aggregator) {
513512
case Aggregator a -> "Aggregator[aggregatorFunction=";
514513
case GroupingAggregator a -> "GroupingAggregator[aggregatorFunction=";
515514
default -> throw new UnsupportedOperationException("can't check toString for [" + aggregator.getClass() + "]");
516515
};
516+
String channels = initialInputChannels().stream().map(Object::toString).collect(Collectors.joining(", "));
517517
String expectedEnd = switch (aggregator) {
518-
case Aggregator a -> "AggregatorFunction[channels=[0]], mode=SINGLE]";
519-
case GroupingAggregator a -> "GroupingAggregatorFunction[channels=[0]], mode=SINGLE]";
518+
case Aggregator a -> "AggregatorFunction[channels=[" + channels + "]], mode=SINGLE]";
519+
case GroupingAggregator a -> "GroupingAggregatorFunction[channels=[" + channels + "]], mode=SINGLE]";
520520
default -> throw new UnsupportedOperationException("can't check toString for [" + aggregator.getClass() + "]");
521521
};
522522

523523
String toString = aggregator.toString();
524524
assertThat(toString, startsWith(expectedStart));
525-
assertThat(toString.substring(expectedStart.length(), toString.length() - expectedEnd.length()), testCase.evaluatorToString());
526525
assertThat(toString, endsWith(expectedEnd));
527-
}
528-
529-
protected boolean optIntoToAggregatorToStringChecks() {
530-
// TODO remove this when everyone has opted in
531-
return false;
526+
assertThat(toString.substring(expectedStart.length(), toString.length() - expectedEnd.length()), testCase.evaluatorToString());
532527
}
533528

534529
protected static String standardAggregatorName(String prefix, DataType type) {
535530
String typeName = switch (type) {
536531
case BOOLEAN -> "Boolean";
532+
case CARTESIAN_POINT -> "CartesianPoint";
533+
case CARTESIAN_SHAPE -> "CartesianShape";
534+
case GEO_POINT -> "GeoPoint";
535+
case GEO_SHAPE -> "GeoShape";
537536
case KEYWORD, TEXT, VERSION -> "BytesRef";
538-
case DOUBLE -> "Double";
539-
case INTEGER -> "Int";
537+
case DOUBLE, COUNTER_DOUBLE -> "Double";
538+
case INTEGER, COUNTER_INTEGER -> "Int";
540539
case IP -> "Ip";
541-
case DATETIME, DATE_NANOS, LONG, UNSIGNED_LONG -> "Long";
540+
case DATETIME, DATE_NANOS, LONG, COUNTER_LONG, UNSIGNED_LONG -> "Long";
541+
case NULL -> "Null";
542542
default -> throw new UnsupportedOperationException("name for [" + type + "]");
543543
};
544544
return prefix + typeName;
545545
}
546+
547+
protected static String standardAggregatorNameAllBytesTheSame(String prefix, DataType type) {
548+
return standardAggregatorName(prefix, switch (type) {
549+
case CARTESIAN_POINT, CARTESIAN_SHAPE, GEO_POINT, GEO_SHAPE, IP -> DataType.KEYWORD;
550+
default -> type;
551+
});
552+
}
546553
}

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/CountDistinctTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ private static TestCaseSupplier makeSupplier(
127127

128128
return new TestCaseSupplier.TestCase(
129129
List.of(fieldTypedData, precisionTypedData),
130-
"CountDistinct[field=Attribute[channel=0],precision=Attribute[channel=1]]",
130+
standardAggregatorNameAllBytesTheSame("CountDistinct", fieldTypedData.type()),
131131
DataType.LONG,
132132
equalTo(result)
133133
);
@@ -149,7 +149,7 @@ private static TestCaseSupplier makeSupplier(TestCaseSupplier.TypedDataSupplier
149149

150150
return new TestCaseSupplier.TestCase(
151151
List.of(fieldTypedData),
152-
"CountDistinct[field=Attribute[channel=0]]",
152+
standardAggregatorNameAllBytesTheSame("CountDistinct", fieldTypedData.type()),
153153
DataType.LONG,
154154
equalTo(result)
155155
);

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/CountTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public static Iterable<Object[]> parameters() {
7878
List.of(dataType),
7979
() -> new TestCaseSupplier.TestCase(
8080
List.of(TestCaseSupplier.TypedData.multiRow(List.of(), dataType, "field")),
81-
"Count[field=Attribute[channel=0]]",
81+
"Count",
8282
DataType.LONG,
8383
equalTo(0L)
8484
)
@@ -102,7 +102,7 @@ private static TestCaseSupplier makeSupplier(TestCaseSupplier.TypedDataSupplier
102102

103103
return new TestCaseSupplier.TestCase(
104104
List.of(fieldTypedData),
105-
"Count[field=Attribute[channel=0]]",
105+
"Count",
106106
DataType.LONG,
107107
equalTo(rowCount)
108108
);

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/FirstOverTimeTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ private static TestCaseSupplier makeSupplier(TestCaseSupplier.TypedDataSupplier
8585
}
8686
return new TestCaseSupplier.TestCase(
8787
List.of(fieldTypedData, timestampsField),
88-
"FirstOverTime[field=Attribute[channel=0],timestamp=Attribute[channel=1]]",
88+
standardAggregatorName("First", fieldSupplier.type()) + "ByTimestamp",
8989
type,
9090
equalTo(expected)
9191
);
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.esql.expression.function.aggregate;
9+
10+
import org.elasticsearch.xpack.esql.core.expression.Expression;
11+
import org.elasticsearch.xpack.esql.expression.AbstractExpressionSerializationTests;
12+
13+
import java.io.IOException;
14+
15+
public class FirstSerializationTests extends AbstractExpressionSerializationTests<First> {
16+
@Override
17+
protected First createTestInstance() {
18+
return new First(randomSource(), randomChild(), randomChild());
19+
}
20+
21+
@Override
22+
protected First mutateInstance(First instance) throws IOException {
23+
Expression field = instance.field();
24+
Expression sort = instance.sort();
25+
if (randomBoolean()) {
26+
field = randomValueOtherThan(field, AbstractExpressionSerializationTests::randomChild);
27+
} else {
28+
sort = randomValueOtherThan(sort, AbstractExpressionSerializationTests::randomChild);
29+
}
30+
return new First(instance.source(), field, sort);
31+
}
32+
}

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/FirstTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ static TestCaseSupplier makeSupplier(
8181
}
8282
return new TestCaseSupplier.TestCase(
8383
List.of(values, sorts),
84-
"unused",
84+
standardAggregatorName(first ? "First" : "Last", values.type()) + "ByTimestamp",
8585
values.type(),
8686
anyOf(() -> Iterators.map(expected.iterator(), Matchers::equalTo))
8787
);

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/LastOverTimeTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ private static TestCaseSupplier makeSupplier(TestCaseSupplier.TypedDataSupplier
8585
}
8686
return new TestCaseSupplier.TestCase(
8787
List.of(fieldTypedData, timestampsField),
88-
"LastOverTime[field=Attribute[channel=0],timestamp=Attribute[channel=1]]",
88+
standardAggregatorName("Last", fieldSupplier.type()) + "ByTimestamp",
8989
type,
9090
equalTo(expected)
9191
);
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.esql.expression.function.aggregate;
9+
10+
import org.elasticsearch.xpack.esql.core.expression.Expression;
11+
import org.elasticsearch.xpack.esql.expression.AbstractExpressionSerializationTests;
12+
13+
import java.io.IOException;
14+
15+
public class LastSerializationTests extends AbstractExpressionSerializationTests<Last> {
16+
@Override
17+
protected Last createTestInstance() {
18+
return new Last(randomSource(), randomChild(), randomChild());
19+
}
20+
21+
@Override
22+
protected Last mutateInstance(Last instance) throws IOException {
23+
Expression field = instance.field();
24+
Expression sort = instance.sort();
25+
if (randomBoolean()) {
26+
field = randomValueOtherThan(field, AbstractExpressionSerializationTests::randomChild);
27+
} else {
28+
sort = randomValueOtherThan(sort, AbstractExpressionSerializationTests::randomChild);
29+
}
30+
return new Last(instance.source(), field, sort);
31+
}
32+
}

0 commit comments

Comments
 (0)