Skip to content

ESQL: Check a few toStrings for aggs #132700

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+109 −28
Open

ESQL: Check a few toStrings for aggs #132700

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
db601d7
ESQL: Check a few toStrings for aggs
nik9000 Aug 12, 2025
8fddaff
Merge branch 'main' into esql_agg_tostring_check
nik9000 Aug 18, 2025
348f740
Fixup
nik9000 Aug 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions ...st/java/org/elasticsearch/xpack/esql/expression/function/AbstractAggregationTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,15 @@
import java.util.stream.IntStream;

import static org.elasticsearch.xpack.esql.EsqlTestUtils.unboundLogicalOptimizerContext;
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.lessThan;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.oneOf;
import static org.hamcrest.Matchers.startsWith;

/**
* Base class for aggregation tests.
Expand Down Expand Up @@ -164,6 +166,7 @@ public void testFold() {
private void aggregateSingleMode(Expression expression) {
Object result;
try (var aggregator = aggregator(expression, initialInputChannels(), AggregatorMode.SINGLE)) {
assertAggregatorToString(aggregator);
for (Page inputPage : rows(testCase.getMultiRowFields())) {
try (
BooleanVector noMasking = driverContext().blockFactory().newConstantBooleanVector(true, inputPage.getPositionCount())
Expand All @@ -187,6 +190,7 @@ private void aggregateGroupingSingleMode(Expression expression) {
assumeFalse("Grouping aggregations must receive data to check results", pages.isEmpty());

try (var aggregator = groupingAggregator(expression, initialInputChannels(), AggregatorMode.SINGLE)) {
assertAggregatorToString(aggregator);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move these be their own tests, like in the scalars?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should. I'll do that before merging.

var groupCount = randomIntBetween(1, 1000);
for (Page inputPage : pages) {
processPageGrouping(aggregator, inputPage, groupCount);
Expand Down Expand Up @@ -482,4 +486,43 @@ private void processPageGrouping(GroupingAggregator aggregator, Page inputPage,
}
}
}

private void assertAggregatorToString(Object aggregator) {
if (optIntoToAggregatorToStringChecks() == false) {
return;
}
String expectedStart = switch (aggregator) {
case Aggregator a -> "Aggregator[aggregatorFunction=";
case GroupingAggregator a -> "GroupingAggregator[aggregatorFunction=";
default -> throw new UnsupportedOperationException("can't check toString for [" + aggregator.getClass() + "]");
};
String expectedEnd = switch (aggregator) {
case Aggregator a -> "AggregatorFunction[channels=[0]], mode=SINGLE]";
case GroupingAggregator a -> "GroupingAggregatorFunction[channels=[0]], mode=SINGLE]";
default -> throw new UnsupportedOperationException("can't check toString for [" + aggregator.getClass() + "]");
};

String toString = aggregator.toString();
assertThat(toString, startsWith(expectedStart));
assertThat(toString.substring(expectedStart.length(), toString.length() - expectedEnd.length()), testCase.evaluatorToString());
assertThat(toString, endsWith(expectedEnd));
}

protected boolean optIntoToAggregatorToStringChecks() {
// TODO remove this when everyone has opted in
return false;
}

protected static String standardAggregatorName(String prefix, DataType type) {
String typeName = switch (type) {
case BOOLEAN -> "Boolean";
case KEYWORD, TEXT, VERSION -> "BytesRef";
case DOUBLE -> "Double";
case INTEGER -> "Int";
case IP -> "Ip";
case DATETIME, DATE_NANOS, LONG, UNSIGNED_LONG -> "Long";
default -> throw new UnsupportedOperationException("name for [" + type + "]");
};
return prefix + typeName;
}
}
29 changes: 17 additions & 12 deletions ...ql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/MaxTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.INTEGER),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(200), DataType.INTEGER, "field")),
"Max[field=Attribute[channel=0]]",
standardAggregatorName("Max", DataType.INTEGER),
DataType.INTEGER,
equalTo(200)
)
Expand All @@ -69,7 +69,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.LONG),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(200L), DataType.LONG, "field")),
"Max[field=Attribute[channel=0]]",
standardAggregatorName("Max", DataType.LONG),
DataType.LONG,
equalTo(200L)
)
Expand All @@ -78,7 +78,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.UNSIGNED_LONG),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(new BigInteger("200")), DataType.UNSIGNED_LONG, "field")),
"Max[field=Attribute[channel=0]]",
standardAggregatorName("Max", DataType.UNSIGNED_LONG),
DataType.UNSIGNED_LONG,
equalTo(new BigInteger("200"))
)
Expand All @@ -87,7 +87,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.DOUBLE),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(200.), DataType.DOUBLE, "field")),
"Max[field=Attribute[channel=0]]",
standardAggregatorName("Max", DataType.DOUBLE),
DataType.DOUBLE,
equalTo(200.)
)
Expand All @@ -96,7 +96,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.DATETIME),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(200L), DataType.DATETIME, "field")),
"Max[field=Attribute[channel=0]]",
standardAggregatorName("Max", DataType.DATETIME),
DataType.DATETIME,
equalTo(200L)
)
Expand All @@ -105,7 +105,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.DATE_NANOS),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(200L), DataType.DATE_NANOS, "field")),
"Max[field=Attribute[channel=0]]",
standardAggregatorName("Max", DataType.DATE_NANOS),
DataType.DATE_NANOS,
equalTo(200L)
)
Expand All @@ -114,7 +114,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.BOOLEAN),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(true), DataType.BOOLEAN, "field")),
"Max[field=Attribute[channel=0]]",
standardAggregatorName("Max", DataType.BOOLEAN),
DataType.BOOLEAN,
equalTo(true)
)
Expand All @@ -129,7 +129,7 @@ public static Iterable<Object[]> parameters() {
"field"
)
),
"Max[field=Attribute[channel=0]]",
standardAggregatorName("Max", DataType.IP),
DataType.IP,
equalTo(new BytesRef(InetAddressPoint.encode(InetAddresses.forString("127.0.0.1"))))
)
Expand All @@ -138,7 +138,7 @@ public static Iterable<Object[]> parameters() {
var value = new BytesRef(randomAlphaOfLengthBetween(0, 50));
return new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(value), DataType.KEYWORD, "field")),
"Max[field=Attribute[channel=0]]",
standardAggregatorName("Max", DataType.KEYWORD),
DataType.KEYWORD,
equalTo(value)
);
Expand All @@ -147,7 +147,7 @@ public static Iterable<Object[]> parameters() {
var value = new BytesRef(randomAlphaOfLengthBetween(0, 50));
return new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(value), DataType.TEXT, "field")),
"Max[field=Attribute[channel=0]]",
standardAggregatorName("Max", DataType.TEXT),
DataType.KEYWORD,
equalTo(value)
);
Expand All @@ -159,7 +159,7 @@ public static Iterable<Object[]> parameters() {
.toBytesRef();
return new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(value), DataType.VERSION, "field")),
"Max[field=Attribute[channel=0]]",
standardAggregatorName("Max", DataType.VERSION),
DataType.VERSION,
equalTo(value)
);
Expand Down Expand Up @@ -187,10 +187,15 @@ private static TestCaseSupplier makeSupplier(TestCaseSupplier.TypedDataSupplier

return new TestCaseSupplier.TestCase(
List.of(fieldTypedData),
"Max[field=Attribute[channel=0]]",
standardAggregatorName("Max", fieldSupplier.type()),
fieldSupplier.type(),
equalTo(expected)
);
});
}

@Override
protected boolean optIntoToAggregatorToStringChecks() {
return true;
}
}
13 changes: 9 additions & 4 deletions ...src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/MedianTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.INTEGER),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(200), DataType.INTEGER, "number")),
"Median[field=Attribute[channel=0]]",
standardAggregatorName("Percentile", DataType.INTEGER),
DataType.DOUBLE,
equalTo(200.)
)
Expand All @@ -56,7 +56,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.LONG),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(200L), DataType.LONG, "number")),
"Median[field=Attribute[channel=0]]",
standardAggregatorName("Percentile", DataType.LONG),
DataType.DOUBLE,
equalTo(200.)
)
Expand All @@ -65,7 +65,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.DOUBLE),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(200.), DataType.DOUBLE, "number")),
"Median[field=Attribute[channel=0]]",
standardAggregatorName("Percentile", DataType.DOUBLE),
DataType.DOUBLE,
equalTo(200.)
)
Expand Down Expand Up @@ -94,11 +94,16 @@ private static TestCaseSupplier makeSupplier(TestCaseSupplier.TypedDataSupplier

return new TestCaseSupplier.TestCase(
List.of(fieldTypedData),
"Median[number=Attribute[channel=0]]",
standardAggregatorName("Percentile", fieldSupplier.type()),
DataType.DOUBLE,
equalTo(expected)
);
}
});
}

@Override
protected boolean optIntoToAggregatorToStringChecks() {
return true;
}
}
29 changes: 17 additions & 12 deletions ...ql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/MinTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.INTEGER),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(200), DataType.INTEGER, "field")),
"Min[field=Attribute[channel=0]]",
standardAggregatorName("Min", DataType.INTEGER),
DataType.INTEGER,
equalTo(200)
)
Expand All @@ -69,7 +69,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.LONG),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(200L), DataType.LONG, "field")),
"Min[field=Attribute[channel=0]]",
standardAggregatorName("Min", DataType.LONG),
DataType.LONG,
equalTo(200L)
)
Expand All @@ -78,7 +78,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.UNSIGNED_LONG),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(new BigInteger("200")), DataType.UNSIGNED_LONG, "field")),
"Max[field=Attribute[channel=0]]",
standardAggregatorName("Min", DataType.UNSIGNED_LONG),
DataType.UNSIGNED_LONG,
equalTo(new BigInteger("200"))
)
Expand All @@ -87,7 +87,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.DOUBLE),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(200.), DataType.DOUBLE, "field")),
"Min[field=Attribute[channel=0]]",
standardAggregatorName("Min", DataType.DOUBLE),
DataType.DOUBLE,
equalTo(200.)
)
Expand All @@ -96,7 +96,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.DATETIME),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(200L), DataType.DATETIME, "field")),
"Min[field=Attribute[channel=0]]",
standardAggregatorName("Min", DataType.DATETIME),
DataType.DATETIME,
equalTo(200L)
)
Expand All @@ -105,7 +105,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.DATE_NANOS),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(200L), DataType.DATE_NANOS, "field")),
"Min[field=Attribute[channel=0]]",
standardAggregatorName("Min", DataType.DATE_NANOS),
DataType.DATE_NANOS,
equalTo(200L)
)
Expand All @@ -114,7 +114,7 @@ public static Iterable<Object[]> parameters() {
List.of(DataType.BOOLEAN),
() -> new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(true), DataType.BOOLEAN, "field")),
"Min[field=Attribute[channel=0]]",
standardAggregatorName("Min", DataType.BOOLEAN),
DataType.BOOLEAN,
equalTo(true)
)
Expand All @@ -129,7 +129,7 @@ public static Iterable<Object[]> parameters() {
"field"
)
),
"Min[field=Attribute[channel=0]]",
standardAggregatorName("Min", DataType.IP),
DataType.IP,
equalTo(new BytesRef(InetAddressPoint.encode(InetAddresses.forString("127.0.0.1"))))
)
Expand All @@ -138,7 +138,7 @@ public static Iterable<Object[]> parameters() {
var value = new BytesRef(randomAlphaOfLengthBetween(0, 50));
return new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(value), DataType.KEYWORD, "field")),
"Min[field=Attribute[channel=0]]",
standardAggregatorName("Min", DataType.KEYWORD),
DataType.KEYWORD,
equalTo(value)
);
Expand All @@ -147,7 +147,7 @@ public static Iterable<Object[]> parameters() {
var value = new BytesRef(randomAlphaOfLengthBetween(0, 50));
return new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(value), DataType.TEXT, "field")),
"Min[field=Attribute[channel=0]]",
standardAggregatorName("Min", DataType.TEXT),
DataType.KEYWORD,
equalTo(value)
);
Expand All @@ -159,7 +159,7 @@ public static Iterable<Object[]> parameters() {
.toBytesRef();
return new TestCaseSupplier.TestCase(
List.of(TestCaseSupplier.TypedData.multiRow(List.of(value), DataType.VERSION, "field")),
"Min[field=Attribute[channel=0]]",
standardAggregatorName("Min", DataType.VERSION),
DataType.VERSION,
equalTo(value)
);
Expand Down Expand Up @@ -187,10 +187,15 @@ private static TestCaseSupplier makeSupplier(TestCaseSupplier.TypedDataSupplier

return new TestCaseSupplier.TestCase(
List.of(fieldTypedData),
"Min[field=Attribute[channel=0]]",
standardAggregatorName("Min", fieldSupplier.type()),
fieldSupplier.type(),
equalTo(expected)
);
});
}

@Override
protected boolean optIntoToAggregatorToStringChecks() {
return true;
}
}