Skip to content

Commit d662314

Browse files
authored
Fix silly rate bug (elastic#134826)
This change fixes a bug in the rate function where rows with values were incorrectly excluded instead of rows without values. Most of the metrics in our tests are dense, so this issue was not detected.
1 parent 2a08e5d commit d662314

File tree

5 files changed

+17
-5
lines changed

5 files changed

+17
-5
lines changed

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

Lines changed: 1 addition & 1 deletion
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-src/org/elasticsearch/compute/aggregation/RateIntGroupingAggregatorFunction.java

Lines changed: 1 addition & 1 deletion
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-src/org/elasticsearch/compute/aggregation/RateLongGroupingAggregatorFunction.java

Lines changed: 1 addition & 1 deletion
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/java/org/elasticsearch/compute/aggregation/X-RateGroupingAggregatorFunction.java.st

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ public final class Rate$Type$GroupingAggregatorFunction implements GroupingAggre
207207
Buffer buffer = null;
208208
for (int p = 0; p < groups.getPositionCount(); p++) {
209209
int valuePosition = positionOffset + p;
210-
if (valueBlock.isNull(valuePosition) == false) {
210+
if (valueBlock.isNull(valuePosition)) {
211211
continue;
212212
}
213213
assert valueBlock.getValueCount(valuePosition) == 1 : "expected single-valued block " + valueBlock;

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import java.util.ArrayList;
2424
import java.util.List;
25+
import java.util.Objects;
2526
import java.util.function.Supplier;
2627

2728
import static org.hamcrest.Matchers.equalTo;
@@ -85,6 +86,17 @@ private static TestCaseSupplier makeSupplier(TestCaseSupplier.TypedDataSupplier
8586
return new TestCaseSupplier(fieldSupplier.name(), List.of(type, DataType.DATETIME, DataType.INTEGER, DataType.LONG), () -> {
8687
TestCaseSupplier.TypedData fieldTypedData = fieldSupplier.get();
8788
List<Object> dataRows = fieldTypedData.multiRowData();
89+
if (randomBoolean()) {
90+
List<Object> withNulls = new ArrayList<>(dataRows);
91+
for (Object dataRow : dataRows) {
92+
if (randomBoolean()) {
93+
withNulls.add(null);
94+
} else {
95+
withNulls.add(dataRow);
96+
}
97+
}
98+
dataRows = withNulls;
99+
}
88100
fieldTypedData = TestCaseSupplier.TypedData.multiRow(dataRows, type, fieldTypedData.name());
89101
List<Long> timestamps = new ArrayList<>();
90102
List<Integer> slices = new ArrayList<>();
@@ -107,7 +119,7 @@ private static TestCaseSupplier makeSupplier(TestCaseSupplier.TypedDataSupplier
107119
DataType.LONG,
108120
"_max_timestamp"
109121
);
110-
122+
dataRows = dataRows.stream().filter(Objects::nonNull).toList();
111123
final Matcher<?> matcher;
112124
if (dataRows.size() < 2) {
113125
matcher = Matchers.nullValue();

0 commit comments

Comments
 (0)