Browse Source

Fix silly rate bug (#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.
Nhat Nguyen 1 month ago
parent
commit
d662314b37

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

@@ -207,7 +207,7 @@ public final class RateDoubleGroupingAggregatorFunction implements GroupingAggre
             Buffer buffer = null;
             for (int p = 0; p < groups.getPositionCount(); p++) {
                 int valuePosition = positionOffset + p;
-                if (valueBlock.isNull(valuePosition) == false) {
+                if (valueBlock.isNull(valuePosition)) {
                     continue;
                 }
                 assert valueBlock.getValueCount(valuePosition) == 1 : "expected single-valued block " + valueBlock;

+ 1 - 1
x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/aggregation/RateIntGroupingAggregatorFunction.java

@@ -207,7 +207,7 @@ public final class RateIntGroupingAggregatorFunction implements GroupingAggregat
             Buffer buffer = null;
             for (int p = 0; p < groups.getPositionCount(); p++) {
                 int valuePosition = positionOffset + p;
-                if (valueBlock.isNull(valuePosition) == false) {
+                if (valueBlock.isNull(valuePosition)) {
                     continue;
                 }
                 assert valueBlock.getValueCount(valuePosition) == 1 : "expected single-valued block " + valueBlock;

+ 1 - 1
x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/aggregation/RateLongGroupingAggregatorFunction.java

@@ -207,7 +207,7 @@ public final class RateLongGroupingAggregatorFunction implements GroupingAggrega
             Buffer buffer = null;
             for (int p = 0; p < groups.getPositionCount(); p++) {
                 int valuePosition = positionOffset + p;
-                if (valueBlock.isNull(valuePosition) == false) {
+                if (valueBlock.isNull(valuePosition)) {
                     continue;
                 }
                 assert valueBlock.getValueCount(valuePosition) == 1 : "expected single-valued block " + valueBlock;

+ 1 - 1
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/X-RateGroupingAggregatorFunction.java.st

@@ -207,7 +207,7 @@ public final class Rate$Type$GroupingAggregatorFunction implements GroupingAggre
             Buffer buffer = null;
             for (int p = 0; p < groups.getPositionCount(); p++) {
                 int valuePosition = positionOffset + p;
-                if (valueBlock.isNull(valuePosition) == false) {
+                if (valueBlock.isNull(valuePosition)) {
                     continue;
                 }
                 assert valueBlock.getValueCount(valuePosition) == 1 : "expected single-valued block " + valueBlock;

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

@@ -22,6 +22,7 @@ import org.hamcrest.Matchers;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
 import java.util.function.Supplier;
 
 import static org.hamcrest.Matchers.equalTo;
@@ -85,6 +86,17 @@ public class RateTests extends AbstractAggregationTestCase {
         return new TestCaseSupplier(fieldSupplier.name(), List.of(type, DataType.DATETIME, DataType.INTEGER, DataType.LONG), () -> {
             TestCaseSupplier.TypedData fieldTypedData = fieldSupplier.get();
             List<Object> dataRows = fieldTypedData.multiRowData();
+            if (randomBoolean()) {
+                List<Object> withNulls = new ArrayList<>(dataRows);
+                for (Object dataRow : dataRows) {
+                    if (randomBoolean()) {
+                        withNulls.add(null);
+                    } else {
+                        withNulls.add(dataRow);
+                    }
+                }
+                dataRows = withNulls;
+            }
             fieldTypedData = TestCaseSupplier.TypedData.multiRow(dataRows, type, fieldTypedData.name());
             List<Long> timestamps = new ArrayList<>();
             List<Integer> slices = new ArrayList<>();
@@ -107,7 +119,7 @@ public class RateTests extends AbstractAggregationTestCase {
                 DataType.LONG,
                 "_max_timestamp"
             );
-
+            dataRows = dataRows.stream().filter(Objects::nonNull).toList();
             final Matcher<?> matcher;
             if (dataRows.size() < 2) {
                 matcher = Matchers.nullValue();