Skip to content

Commit 980fb1b

Browse files
nik9000dnhatn
nik9000
and
dnhatn
authored
Add tests for nulls in aggs (ESQL-575)
This addes tests for `null`s in grouping and non-grouping aggs and randomizes inputs for those tests. We have an assertion that we don't access null values in blocks, but this adds an extra layer of paranoia. The randomized tests will sometimes fail, even without the assertion. Finally, I renamed the test classes to line up with the names of the generated classes - they have the operation name first, then the type, then the `GroupingAggregatorFunction` or `AggregatorFunction` part. Costin likes that ordering better and I like consistency better. Co-authored-by: Nhat Nguyen <[email protected]>
1 parent b926d41 commit 980fb1b

File tree

56 files changed

+977
-388
lines changed

Some content is hidden

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

56 files changed

+977
-388
lines changed

‎benchmarks/src/main/java/org/elasticsearch/benchmark/compute/operation/AggregatorBenchmark.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ private static void checkGrouped(String prefix, String op, Page page) {
124124
DoubleBlock dValues = (DoubleBlock) values;
125125
for (int g = 0; g < GROUPS; g++) {
126126
long group = g;
127-
double sum = LongStream.range(0, BLOCK_LENGTH).filter(l -> l % GROUPS == group).mapToDouble(l -> (double) l).sum();
127+
long sum = LongStream.range(0, BLOCK_LENGTH).filter(l -> l % GROUPS == group).sum();
128128
long count = LongStream.range(0, BLOCK_LENGTH).filter(l -> l % GROUPS == group).count();
129129
double expected = sum / count;
130130
if (dValues.getDouble(g) != expected) {

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

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -187,9 +187,10 @@ private MethodSpec addRawInput() {
187187
private MethodSpec addRawVector() {
188188
MethodSpec.Builder builder = MethodSpec.methodBuilder("addRawVector");
189189
builder.addModifiers(Modifier.PRIVATE).addParameter(LONG_VECTOR, "groupIdVector").addParameter(valueVectorType(), "vector");
190-
builder.beginControlFlow("for (int i = 0; i < vector.getPositionCount(); i++)");
190+
builder.beginControlFlow("for (int position = 0; position < vector.getPositionCount(); position++)");
191191
{
192-
combineRawInput(builder, "vector");
192+
builder.addStatement("int groupId = Math.toIntExact(groupIdVector.getLong(position))");
193+
combineRawInput(builder, "vector", "position");
193194
}
194195
builder.endControlFlow();
195196
return builder.build();
@@ -198,18 +199,21 @@ private MethodSpec addRawVector() {
198199
private MethodSpec addRawBlock() {
199200
MethodSpec.Builder builder = MethodSpec.methodBuilder("addRawBlock");
200201
builder.addModifiers(Modifier.PRIVATE).addParameter(LONG_VECTOR, "groupIdVector").addParameter(valueBlockType(), "block");
201-
builder.beginControlFlow("for (int i = 0; i < block.getTotalValueCount(); i++)");
202+
203+
builder.beginControlFlow("for (int offset = 0; offset < block.getTotalValueCount(); offset++)");
202204
{
203-
builder.beginControlFlow("if (block.isNull(i) == false)");
204-
combineRawInput(builder, "block");
205+
builder.beginControlFlow("if (block.isNull(offset) == false)");
206+
{
207+
builder.addStatement("int groupId = Math.toIntExact(groupIdVector.getLong(offset))");
208+
combineRawInput(builder, "block", "offset");
209+
}
205210
builder.endControlFlow();
206211
}
207212
builder.endControlFlow();
208213
return builder.build();
209214
}
210215

211-
private void combineRawInput(MethodSpec.Builder builder, String blockVariable) {
212-
builder.addStatement("int groupId = Math.toIntExact(groupIdVector.getLong(i))");
216+
private void combineRawInput(MethodSpec.Builder builder, String blockVariable, String offsetVariable) {
213217
TypeName valueType = TypeName.get(combine.getParameters().get(combine.getParameters().size() - 1).asType());
214218
if (valueType.isPrimitive() == false) {
215219
throw new IllegalArgumentException("second parameter to combine must be a primitive");
@@ -219,27 +223,44 @@ private void combineRawInput(MethodSpec.Builder builder, String blockVariable) {
219223
+ valueType.toString().substring(1);
220224
TypeName returnType = TypeName.get(combine.getReturnType());
221225
if (returnType.isPrimitive()) {
222-
combineRawInputForPrimitive(builder, secondParameterGetter, blockVariable);
226+
combineRawInputForPrimitive(builder, secondParameterGetter, blockVariable, offsetVariable);
223227
return;
224228
}
225229
if (returnType == TypeName.VOID) {
226-
combineRawInputForVoid(builder, secondParameterGetter, blockVariable);
230+
combineRawInputForVoid(builder, secondParameterGetter, blockVariable, offsetVariable);
227231
return;
228232
}
229233
throw new IllegalArgumentException("combine must return void or a primitive");
230234
}
231235

232-
private void combineRawInputForPrimitive(MethodSpec.Builder builder, String secondParameterGetter, String blockVariable) {
236+
private void combineRawInputForPrimitive(
237+
MethodSpec.Builder builder,
238+
String secondParameterGetter,
239+
String blockVariable,
240+
String offsetVariable
241+
) {
233242
builder.addStatement(
234-
"state.set($T.combine(state.getOrDefault(groupId), $L.$L(i)), groupId)",
243+
"state.set($T.combine(state.getOrDefault(groupId), $L.$L($L)), groupId)",
235244
declarationType,
236245
blockVariable,
237-
secondParameterGetter
246+
secondParameterGetter,
247+
offsetVariable
238248
);
239249
}
240250

241-
private void combineRawInputForVoid(MethodSpec.Builder builder, String secondParameterGetter, String blockVariable) {
242-
builder.addStatement("$T.combine(state, groupId, $L.$L(i))", declarationType, blockVariable, secondParameterGetter);
251+
private void combineRawInputForVoid(
252+
MethodSpec.Builder builder,
253+
String secondParameterGetter,
254+
String blockVariable,
255+
String offsetVariable
256+
) {
257+
builder.addStatement(
258+
"$T.combine(state, groupId, $L.$L($L))",
259+
declarationType,
260+
blockVariable,
261+
secondParameterGetter,
262+
offsetVariable
263+
);
243264
}
244265

245266
private MethodSpec addIntermediateInput() {

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,17 +44,17 @@ public void addRawInput(LongVector groupIdVector, Page page) {
4444
}
4545

4646
private void addRawVector(LongVector groupIdVector, DoubleVector vector) {
47-
for (int i = 0; i < vector.getPositionCount(); i++) {
48-
int groupId = Math.toIntExact(groupIdVector.getLong(i));
49-
AvgDoubleAggregator.combine(state, groupId, vector.getDouble(i));
47+
for (int position = 0; position < vector.getPositionCount(); position++) {
48+
int groupId = Math.toIntExact(groupIdVector.getLong(position));
49+
AvgDoubleAggregator.combine(state, groupId, vector.getDouble(position));
5050
}
5151
}
5252

5353
private void addRawBlock(LongVector groupIdVector, DoubleBlock block) {
54-
for (int i = 0; i < block.getTotalValueCount(); i++) {
55-
if (block.isNull(i) == false) {
56-
int groupId = Math.toIntExact(groupIdVector.getLong(i));
57-
AvgDoubleAggregator.combine(state, groupId, block.getDouble(i));
54+
for (int offset = 0; offset < block.getTotalValueCount(); offset++) {
55+
if (block.isNull(offset) == false) {
56+
int groupId = Math.toIntExact(groupIdVector.getLong(offset));
57+
AvgDoubleAggregator.combine(state, groupId, block.getDouble(offset));
5858
}
5959
}
6060
}

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,17 @@ public void addRawInput(LongVector groupIdVector, Page page) {
4242
}
4343

4444
private void addRawVector(LongVector groupIdVector, LongVector vector) {
45-
for (int i = 0; i < vector.getPositionCount(); i++) {
46-
int groupId = Math.toIntExact(groupIdVector.getLong(i));
47-
AvgLongAggregator.combine(state, groupId, vector.getLong(i));
45+
for (int position = 0; position < vector.getPositionCount(); position++) {
46+
int groupId = Math.toIntExact(groupIdVector.getLong(position));
47+
AvgLongAggregator.combine(state, groupId, vector.getLong(position));
4848
}
4949
}
5050

5151
private void addRawBlock(LongVector groupIdVector, LongBlock block) {
52-
for (int i = 0; i < block.getTotalValueCount(); i++) {
53-
if (block.isNull(i) == false) {
54-
int groupId = Math.toIntExact(groupIdVector.getLong(i));
55-
AvgLongAggregator.combine(state, groupId, block.getLong(i));
52+
for (int offset = 0; offset < block.getTotalValueCount(); offset++) {
53+
if (block.isNull(offset) == false) {
54+
int groupId = Math.toIntExact(groupIdVector.getLong(offset));
55+
AvgLongAggregator.combine(state, groupId, block.getLong(offset));
5656
}
5757
}
5858
}

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,17 +44,17 @@ public void addRawInput(LongVector groupIdVector, Page page) {
4444
}
4545

4646
private void addRawVector(LongVector groupIdVector, DoubleVector vector) {
47-
for (int i = 0; i < vector.getPositionCount(); i++) {
48-
int groupId = Math.toIntExact(groupIdVector.getLong(i));
49-
state.set(MaxDoubleAggregator.combine(state.getOrDefault(groupId), vector.getDouble(i)), groupId);
47+
for (int position = 0; position < vector.getPositionCount(); position++) {
48+
int groupId = Math.toIntExact(groupIdVector.getLong(position));
49+
state.set(MaxDoubleAggregator.combine(state.getOrDefault(groupId), vector.getDouble(position)), groupId);
5050
}
5151
}
5252

5353
private void addRawBlock(LongVector groupIdVector, DoubleBlock block) {
54-
for (int i = 0; i < block.getTotalValueCount(); i++) {
55-
if (block.isNull(i) == false) {
56-
int groupId = Math.toIntExact(groupIdVector.getLong(i));
57-
state.set(MaxDoubleAggregator.combine(state.getOrDefault(groupId), block.getDouble(i)), groupId);
54+
for (int offset = 0; offset < block.getTotalValueCount(); offset++) {
55+
if (block.isNull(offset) == false) {
56+
int groupId = Math.toIntExact(groupIdVector.getLong(offset));
57+
state.set(MaxDoubleAggregator.combine(state.getOrDefault(groupId), block.getDouble(offset)), groupId);
5858
}
5959
}
6060
}

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,17 @@ public void addRawInput(LongVector groupIdVector, Page page) {
4343
}
4444

4545
private void addRawVector(LongVector groupIdVector, LongVector vector) {
46-
for (int i = 0; i < vector.getPositionCount(); i++) {
47-
int groupId = Math.toIntExact(groupIdVector.getLong(i));
48-
state.set(MaxLongAggregator.combine(state.getOrDefault(groupId), vector.getLong(i)), groupId);
46+
for (int position = 0; position < vector.getPositionCount(); position++) {
47+
int groupId = Math.toIntExact(groupIdVector.getLong(position));
48+
state.set(MaxLongAggregator.combine(state.getOrDefault(groupId), vector.getLong(position)), groupId);
4949
}
5050
}
5151

5252
private void addRawBlock(LongVector groupIdVector, LongBlock block) {
53-
for (int i = 0; i < block.getTotalValueCount(); i++) {
54-
if (block.isNull(i) == false) {
55-
int groupId = Math.toIntExact(groupIdVector.getLong(i));
56-
state.set(MaxLongAggregator.combine(state.getOrDefault(groupId), block.getLong(i)), groupId);
53+
for (int offset = 0; offset < block.getTotalValueCount(); offset++) {
54+
if (block.isNull(offset) == false) {
55+
int groupId = Math.toIntExact(groupIdVector.getLong(offset));
56+
state.set(MaxLongAggregator.combine(state.getOrDefault(groupId), block.getLong(offset)), groupId);
5757
}
5858
}
5959
}

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,17 +45,17 @@ public void addRawInput(LongVector groupIdVector, Page page) {
4545
}
4646

4747
private void addRawVector(LongVector groupIdVector, DoubleVector vector) {
48-
for (int i = 0; i < vector.getPositionCount(); i++) {
49-
int groupId = Math.toIntExact(groupIdVector.getLong(i));
50-
MedianAbsoluteDeviationDoubleAggregator.combine(state, groupId, vector.getDouble(i));
48+
for (int position = 0; position < vector.getPositionCount(); position++) {
49+
int groupId = Math.toIntExact(groupIdVector.getLong(position));
50+
MedianAbsoluteDeviationDoubleAggregator.combine(state, groupId, vector.getDouble(position));
5151
}
5252
}
5353

5454
private void addRawBlock(LongVector groupIdVector, DoubleBlock block) {
55-
for (int i = 0; i < block.getTotalValueCount(); i++) {
56-
if (block.isNull(i) == false) {
57-
int groupId = Math.toIntExact(groupIdVector.getLong(i));
58-
MedianAbsoluteDeviationDoubleAggregator.combine(state, groupId, block.getDouble(i));
55+
for (int offset = 0; offset < block.getTotalValueCount(); offset++) {
56+
if (block.isNull(offset) == false) {
57+
int groupId = Math.toIntExact(groupIdVector.getLong(offset));
58+
MedianAbsoluteDeviationDoubleAggregator.combine(state, groupId, block.getDouble(offset));
5959
}
6060
}
6161
}

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,17 +44,17 @@ public void addRawInput(LongVector groupIdVector, Page page) {
4444
}
4545

4646
private void addRawVector(LongVector groupIdVector, LongVector vector) {
47-
for (int i = 0; i < vector.getPositionCount(); i++) {
48-
int groupId = Math.toIntExact(groupIdVector.getLong(i));
49-
MedianAbsoluteDeviationLongAggregator.combine(state, groupId, vector.getLong(i));
47+
for (int position = 0; position < vector.getPositionCount(); position++) {
48+
int groupId = Math.toIntExact(groupIdVector.getLong(position));
49+
MedianAbsoluteDeviationLongAggregator.combine(state, groupId, vector.getLong(position));
5050
}
5151
}
5252

5353
private void addRawBlock(LongVector groupIdVector, LongBlock block) {
54-
for (int i = 0; i < block.getTotalValueCount(); i++) {
55-
if (block.isNull(i) == false) {
56-
int groupId = Math.toIntExact(groupIdVector.getLong(i));
57-
MedianAbsoluteDeviationLongAggregator.combine(state, groupId, block.getLong(i));
54+
for (int offset = 0; offset < block.getTotalValueCount(); offset++) {
55+
if (block.isNull(offset) == false) {
56+
int groupId = Math.toIntExact(groupIdVector.getLong(offset));
57+
MedianAbsoluteDeviationLongAggregator.combine(state, groupId, block.getLong(offset));
5858
}
5959
}
6060
}

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,17 @@ public void addRawInput(LongVector groupIdVector, Page page) {
4343
}
4444

4545
private void addRawVector(LongVector groupIdVector, DoubleVector vector) {
46-
for (int i = 0; i < vector.getPositionCount(); i++) {
47-
int groupId = Math.toIntExact(groupIdVector.getLong(i));
48-
MedianDoubleAggregator.combine(state, groupId, vector.getDouble(i));
46+
for (int position = 0; position < vector.getPositionCount(); position++) {
47+
int groupId = Math.toIntExact(groupIdVector.getLong(position));
48+
MedianDoubleAggregator.combine(state, groupId, vector.getDouble(position));
4949
}
5050
}
5151

5252
private void addRawBlock(LongVector groupIdVector, DoubleBlock block) {
53-
for (int i = 0; i < block.getTotalValueCount(); i++) {
54-
if (block.isNull(i) == false) {
55-
int groupId = Math.toIntExact(groupIdVector.getLong(i));
56-
MedianDoubleAggregator.combine(state, groupId, block.getDouble(i));
53+
for (int offset = 0; offset < block.getTotalValueCount(); offset++) {
54+
if (block.isNull(offset) == false) {
55+
int groupId = Math.toIntExact(groupIdVector.getLong(offset));
56+
MedianDoubleAggregator.combine(state, groupId, block.getDouble(offset));
5757
}
5858
}
5959
}

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,17 @@ public void addRawInput(LongVector groupIdVector, Page page) {
4242
}
4343

4444
private void addRawVector(LongVector groupIdVector, LongVector vector) {
45-
for (int i = 0; i < vector.getPositionCount(); i++) {
46-
int groupId = Math.toIntExact(groupIdVector.getLong(i));
47-
MedianLongAggregator.combine(state, groupId, vector.getLong(i));
45+
for (int position = 0; position < vector.getPositionCount(); position++) {
46+
int groupId = Math.toIntExact(groupIdVector.getLong(position));
47+
MedianLongAggregator.combine(state, groupId, vector.getLong(position));
4848
}
4949
}
5050

5151
private void addRawBlock(LongVector groupIdVector, LongBlock block) {
52-
for (int i = 0; i < block.getTotalValueCount(); i++) {
53-
if (block.isNull(i) == false) {
54-
int groupId = Math.toIntExact(groupIdVector.getLong(i));
55-
MedianLongAggregator.combine(state, groupId, block.getLong(i));
52+
for (int offset = 0; offset < block.getTotalValueCount(); offset++) {
53+
if (block.isNull(offset) == false) {
54+
int groupId = Math.toIntExact(groupIdVector.getLong(offset));
55+
MedianLongAggregator.combine(state, groupId, block.getLong(offset));
5656
}
5757
}
5858
}

0 commit comments

Comments
 (0)