Skip to content

Commit 7380179

Browse files
authored
[ESQL] Shortcircuit grouping aggs on null value (#129708)
Minor aggregators change plus a test to ensure we check that code path. We were doing a `value.isNull()` for each group the value had to be aggregated in, instead of doing it just once at the beginning. The affectation goes as follows: - 1 group, null value: We avoid some extra lightweight calls - N groups, null value: We avoid calling it N times - No groups, or non-null value: No effect
1 parent 9e81541 commit 7380179

File tree

82 files changed

+276
-726
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

82 files changed

+276
-726
lines changed

x-pack/plugin/esql/compute/gen/src/main/java/org/elasticsearch/compute/gen/GroupingAggregatorImplementer.java

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import java.util.Arrays;
2424
import java.util.List;
25+
import java.util.Objects;
2526
import java.util.function.Consumer;
2627
import java.util.function.Function;
2728
import java.util.stream.Collectors;
@@ -411,10 +412,16 @@ private MethodSpec addRawInputLoop(TypeName groupsType, TypeName valuesType) {
411412

412413
builder.beginControlFlow("for (int groupPosition = 0; groupPosition < groups.getPositionCount(); groupPosition++)");
413414
{
414-
if (groupsIsBlock) {
415-
builder.beginControlFlow("if (groups.isNull(groupPosition))");
415+
if (groupsIsBlock || valuesIsBlock) {
416+
String conditions = Stream.of(
417+
groupsIsBlock ? "groups.isNull(groupPosition)" : null,
418+
valuesIsBlock ? "values.isNull(groupPosition + positionOffset)" : null
419+
).filter(Objects::nonNull).collect(Collectors.joining(" || "));
420+
builder.beginControlFlow("if (" + conditions + ")");
416421
builder.addStatement("continue");
417422
builder.endControlFlow();
423+
}
424+
if (groupsIsBlock) {
418425
builder.addStatement("int groupStart = groups.getFirstValueIndex(groupPosition)");
419426
builder.addStatement("int groupEnd = groupStart + groups.getValueCount(groupPosition)");
420427
builder.beginControlFlow("for (int g = groupStart; g < groupEnd; g++)");
@@ -430,9 +437,6 @@ private MethodSpec addRawInputLoop(TypeName groupsType, TypeName valuesType) {
430437
}
431438

432439
if (valuesIsBlock) {
433-
builder.beginControlFlow("if (values.isNull(groupPosition + positionOffset))");
434-
builder.addStatement("continue");
435-
builder.endControlFlow();
436440
builder.addStatement("int valuesStart = values.getFirstValueIndex(groupPosition + positionOffset)");
437441
builder.addStatement("int valuesEnd = valuesStart + values.getValueCount(groupPosition + positionOffset)");
438442
if (aggParam.isArray()) {

x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/CountDistinctBooleanGroupingAggregatorFunction.java

Lines changed: 3 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/CountDistinctBytesRefGroupingAggregatorFunction.java

Lines changed: 3 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/CountDistinctDoubleGroupingAggregatorFunction.java

Lines changed: 3 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/CountDistinctFloatGroupingAggregatorFunction.java

Lines changed: 3 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/CountDistinctIntGroupingAggregatorFunction.java

Lines changed: 3 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

x-pack/plugin/esql/compute/src/main/generated/org/elasticsearch/compute/aggregation/CountDistinctLongGroupingAggregatorFunction.java

Lines changed: 3 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)