Skip to content

Commit 9422b5a

Browse files
authored
Avoid O(N^2) in VALUES with ordinals grouping (#130576) (#130773)
Using the VALUES aggregator with ordinals grouping led to accidental quadratic complexity. Queries like FROM .. | STATS ... VALUES(field) ... BY keyword-field are affected by this performance issue. This change caches a sorted structure - previously used to fix a similar O(N^2) problem when emitting the output block - during the merging phase of the OrdinalGroupingOperator.
1 parent e26dbc2 commit 9422b5a

File tree

14 files changed

+678
-313
lines changed

14 files changed

+678
-313
lines changed

benchmarks/src/main/java/org/elasticsearch/benchmark/compute/operator/ValuesAggregatorBenchmark.java

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,8 @@ public class ValuesAggregatorBenchmark {
8585
try {
8686
for (String groups : ValuesAggregatorBenchmark.class.getField("groups").getAnnotationsByType(Param.class)[0].value()) {
8787
for (String dataType : ValuesAggregatorBenchmark.class.getField("dataType").getAnnotationsByType(Param.class)[0].value()) {
88-
run(Integer.parseInt(groups), dataType, 10);
88+
run(Integer.parseInt(groups), dataType, 10, 0);
89+
run(Integer.parseInt(groups), dataType, 10, 1);
8990
}
9091
}
9192
} catch (NoSuchFieldException e) {
@@ -103,7 +104,10 @@ public class ValuesAggregatorBenchmark {
103104
@Param({ BYTES_REF, INT, LONG })
104105
public String dataType;
105106

106-
private static Operator operator(DriverContext driverContext, int groups, String dataType) {
107+
@Param({ "0", "1" })
108+
public int numOrdinalMerges;
109+
110+
private static Operator operator(DriverContext driverContext, int groups, String dataType, int numOrdinalMerges) {
107111
if (groups == 1) {
108112
return new AggregationOperator(
109113
List.of(supplier(dataType).aggregatorFactory(AggregatorMode.SINGLE, List.of(0)).apply(driverContext)),
@@ -115,7 +119,24 @@ private static Operator operator(DriverContext driverContext, int groups, String
115119
List.of(supplier(dataType).groupingAggregatorFactory(AggregatorMode.SINGLE, List.of(1))),
116120
() -> BlockHash.build(groupSpec, driverContext.blockFactory(), 16 * 1024, false),
117121
driverContext
118-
);
122+
) {
123+
@Override
124+
public Page getOutput() {
125+
mergeOrdinal();
126+
return super.getOutput();
127+
}
128+
129+
// simulate OrdinalsGroupingOperator
130+
void mergeOrdinal() {
131+
var merged = supplier(dataType).groupingAggregatorFactory(AggregatorMode.SINGLE, List.of(1)).apply(driverContext);
132+
for (int i = 0; i < numOrdinalMerges; i++) {
133+
for (int p = 0; p < groups; p++) {
134+
merged.addIntermediateRow(p, aggregators.getFirst(), p);
135+
}
136+
}
137+
aggregators.set(0, merged);
138+
}
139+
};
119140
}
120141

121142
private static AggregatorFunctionSupplier supplier(String dataType) {
@@ -314,12 +335,12 @@ private static Block groupingBlock(int groups) {
314335

315336
@Benchmark
316337
public void run() {
317-
run(groups, dataType, OP_COUNT);
338+
run(groups, dataType, OP_COUNT, numOrdinalMerges);
318339
}
319340

320-
private static void run(int groups, String dataType, int opCount) {
341+
private static void run(int groups, String dataType, int opCount, int numOrdinalMerges) {
321342
DriverContext driverContext = driverContext();
322-
try (Operator operator = operator(driverContext, groups, dataType)) {
343+
try (Operator operator = operator(driverContext, groups, dataType, numOrdinalMerges)) {
323344
Page page = page(groups, dataType);
324345
for (int i = 0; i < opCount; i++) {
325346
operator.addInput(page.shallowCopy());

docs/changelog/130576.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 130576
2+
summary: Avoid O(N^2) in VALUES with ordinals grouping
3+
area: ES|QL
4+
type: bug
5+
issues: []

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

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

0 commit comments

Comments
 (0)