diff --git a/muted-tests.yml b/muted-tests.yml index 6a57d9a5c3ba6..23916ea2197f5 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -515,9 +515,6 @@ tests: - class: org.elasticsearch.xpack.esql.vector.VectorSimilarityFunctionsIT method: testDifferentDimensions {functionName=v_dot_product similarityFunction=DOT_PRODUCT} issue: https://github.com/elastic/elasticsearch/issues/131845 -- class: org.elasticsearch.compute.aggregation.ValuesBytesRefGroupingAggregatorFunctionTests - method: testSomeFiltered - issue: https://github.com/elastic/elasticsearch/issues/131878 - class: org.elasticsearch.xpack.restart.FullClusterRestartIT method: testWatcherWithApiKey {cluster=UPGRADED} issue: https://github.com/elastic/elasticsearch/issues/131964 diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/aggregation/ValuesBytesRefAggregator.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/aggregation/ValuesBytesRefAggregator.java index 120a77fda92c6..9b5aa1ec9ae59 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/aggregation/ValuesBytesRefAggregator.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/aggregation/ValuesBytesRefAggregator.java @@ -366,12 +366,8 @@ Block buildOrdinalOutputBlock(BlockFactory blockFactory, IntVector selected) { try (var builder = blockFactory.newIntBlockBuilder(estimateSize)) { int nextValuesStart = 0; for (int s = 0; s < selected.getPositionCount(); s++) { - int group = selected.getInt(s); - if (firstValues.size() < group) { - builder.appendNull(); - continue; - } - int firstValue = firstValues.get(group) - 1; + final int group = selected.getInt(s); + final int firstValue = group >= firstValues.size() ? -1 : firstValues.get(group) - 1; if (firstValue < 0) { builder.appendNull(); continue; diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/ValuesBytesRefAggregators.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/ValuesBytesRefAggregators.java index 20ec7b08f9cb7..8448d28015e1c 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/ValuesBytesRefAggregators.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/ValuesBytesRefAggregators.java @@ -160,8 +160,8 @@ static void addOrdinalInputBlock( IntVector hashIds ) { for (int p = 0; p < groupIds.getPositionCount(); p++) { + final int groupId = groupIds.getInt(p); final int valuePosition = p + positionOffset; - final int groupId = groupIds.getInt(valuePosition); final int start = ordinalIds.getFirstValueIndex(valuePosition); final int end = start + ordinalIds.getValueCount(valuePosition); for (int i = start; i < end; i++) { @@ -212,8 +212,8 @@ static void combineIntermediateInputValues( } else { final BytesRef scratch = new BytesRef(); for (int p = 0; p < groupIds.getPositionCount(); p++) { + final int groupId = groupIds.getInt(p); final int valuePosition = p + positionOffset; - final int groupId = groupIds.getInt(valuePosition); final int start = values.getFirstValueIndex(valuePosition); final int end = start + values.getValueCount(valuePosition); for (int i = start; i < end; i++) { diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/X-ValuesAggregator.java.st b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/X-ValuesAggregator.java.st index 017c5540c98f0..3641a17ce6972 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/X-ValuesAggregator.java.st +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/X-ValuesAggregator.java.st @@ -525,12 +525,8 @@ $if(BytesRef)$ try (var builder = blockFactory.newIntBlockBuilder(estimateSize)) { int nextValuesStart = 0; for (int s = 0; s < selected.getPositionCount(); s++) { - int group = selected.getInt(s); - if (firstValues.size() < group) { - builder.appendNull(); - continue; - } - int firstValue = firstValues.get(group) - 1; + final int group = selected.getInt(s); + final int firstValue = group >= firstValues.size() ? -1 : firstValues.get(group) - 1; if (firstValue < 0) { builder.appendNull(); continue; diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/ValuesBytesRefGroupingAggregatorFunctionTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/ValuesBytesRefGroupingAggregatorFunctionTests.java index a1367bee53340..fa274ef05c5fe 100644 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/ValuesBytesRefGroupingAggregatorFunctionTests.java +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/ValuesBytesRefGroupingAggregatorFunctionTests.java @@ -27,6 +27,9 @@ import static org.hamcrest.Matchers.nullValue; public class ValuesBytesRefGroupingAggregatorFunctionTests extends GroupingAggregatorFunctionTestCase { + final boolean ordinals = randomBoolean(); + final boolean withNulls = randomBoolean(); + @Override protected AggregatorFunctionSupplier aggregatorFunction() { return new ValuesBytesRefAggregatorFunctionSupplier(); @@ -39,10 +42,17 @@ protected String expectedDescriptionOfAggregator() { @Override protected SourceOperator simpleInput(BlockFactory blockFactory, int size) { - return new LongBytesRefTupleBlockSourceOperator( - blockFactory, - IntStream.range(0, size).mapToObj(l -> Tuple.tuple(randomLongBetween(0, 4), new BytesRef(randomAlphaOfLengthBetween(0, 100)))) - ); + + return new LongBytesRefTupleBlockSourceOperator(blockFactory, IntStream.range(0, size).mapToObj(l -> { + long groupId = randomLongBetween(0, 100); + if (withNulls && randomBoolean()) { + return Tuple.tuple(groupId, null); + } + if (ordinals && randomBoolean()) { + return Tuple.tuple(groupId, new BytesRef("v" + between(1, 5))); + } + return Tuple.tuple(groupId, new BytesRef(randomAlphaOfLengthBetween(0, 100))); + })); } @Override diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/ValuesDoubleGroupingAggregatorFunctionTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/ValuesDoubleGroupingAggregatorFunctionTests.java index b89612a52c682..d936aa72e868a 100644 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/ValuesDoubleGroupingAggregatorFunctionTests.java +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/ValuesDoubleGroupingAggregatorFunctionTests.java @@ -26,6 +26,8 @@ import static org.hamcrest.Matchers.nullValue; public class ValuesDoubleGroupingAggregatorFunctionTests extends GroupingAggregatorFunctionTestCase { + private final boolean withNulls = randomBoolean(); + @Override protected AggregatorFunctionSupplier aggregatorFunction() { return new ValuesDoubleAggregatorFunctionSupplier(); @@ -40,7 +42,8 @@ protected String expectedDescriptionOfAggregator() { protected SourceOperator simpleInput(BlockFactory blockFactory, int size) { return new LongDoubleTupleBlockSourceOperator( blockFactory, - LongStream.range(0, size).mapToObj(l -> Tuple.tuple(randomLongBetween(0, 4), randomDouble())) + LongStream.range(0, size) + .mapToObj(l -> Tuple.tuple(randomLongBetween(0, 100), withNulls && randomBoolean() ? null : randomDouble())) ); } diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/ValuesFloatGroupingAggregatorFunctionTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/ValuesFloatGroupingAggregatorFunctionTests.java index 7dc550abd4e49..7f6f3345d7cf8 100644 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/ValuesFloatGroupingAggregatorFunctionTests.java +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/ValuesFloatGroupingAggregatorFunctionTests.java @@ -26,6 +26,8 @@ import static org.hamcrest.Matchers.nullValue; public class ValuesFloatGroupingAggregatorFunctionTests extends GroupingAggregatorFunctionTestCase { + private final boolean withNulls = randomBoolean(); + @Override protected AggregatorFunctionSupplier aggregatorFunction() { return new ValuesFloatAggregatorFunctionSupplier(); @@ -40,7 +42,8 @@ protected String expectedDescriptionOfAggregator() { protected SourceOperator simpleInput(BlockFactory blockFactory, int size) { return new LongFloatTupleBlockSourceOperator( blockFactory, - LongStream.range(0, size).mapToObj(l -> Tuple.tuple(randomLongBetween(0, 4), randomFloat())) + LongStream.range(0, size) + .mapToObj(l -> Tuple.tuple(randomLongBetween(0, 100), withNulls && randomBoolean() ? null : randomFloat())) ); } diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/ValuesIntGroupingAggregatorFunctionTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/ValuesIntGroupingAggregatorFunctionTests.java index 7368ed285ddb6..7b902ed5194c8 100644 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/ValuesIntGroupingAggregatorFunctionTests.java +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/ValuesIntGroupingAggregatorFunctionTests.java @@ -40,7 +40,7 @@ protected String expectedDescriptionOfAggregator() { protected SourceOperator simpleInput(BlockFactory blockFactory, int size) { return new LongIntBlockSourceOperator( blockFactory, - LongStream.range(0, size).mapToObj(l -> Tuple.tuple(randomLongBetween(0, 4), randomInt())) + LongStream.range(0, size).mapToObj(l -> Tuple.tuple(randomLongBetween(0, 100), randomInt())) ); }