Fix block hash test and revert mistaken changes

ChrisHegarty · Sep 22, 2023 · 089690e · 089690e
1 parent 49410d3
commit 089690e
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 6 deletions.
diff --git a/.../src/main/java/org/elasticsearch/compute/aggregation/blockhash/BytesRefLongBlockHash.java b/.../src/main/java/org/elasticsearch/compute/aggregation/blockhash/BytesRefLongBlockHash.java
@@ -74,7 +74,6 @@ public void add(Page page, GroupingAggregatorFunction.AddInput addInput) {
         } else {
             new AddBlock(block1, block2, addInput).add();
         }
-        Releasables.closeExpectNoException(block1, block2);
     }
 
     public IntVector add(BytesRefVector vector1, LongVector vector2) {

diff --git a/...l/compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/IntBlockHash.java b/...l/compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/IntBlockHash.java
@@ -20,7 +20,6 @@
 import org.elasticsearch.compute.data.Page;
 import org.elasticsearch.compute.operator.MultivalueDedupe;
 import org.elasticsearch.compute.operator.MultivalueDedupeInt;
-import org.elasticsearch.core.Releasables;
 
 import java.util.BitSet;
 
@@ -53,7 +52,6 @@ public void add(Page page, GroupingAggregatorFunction.AddInput addInput) {
         } else {
             addInput.add(0, add(vector));
         }
-        Releasables.closeExpectNoException(block);
     }
 
     private IntVector add(IntVector vector) {

diff --git a/.../compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/LongBlockHash.java b/.../compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/LongBlockHash.java
@@ -13,6 +13,7 @@
 import org.elasticsearch.compute.aggregation.GroupingAggregatorFunction;
 import org.elasticsearch.compute.aggregation.SeenGroupIds;
 import org.elasticsearch.compute.data.Block;
+import org.elasticsearch.compute.data.IntArrayVector;
 import org.elasticsearch.compute.data.IntBlock;
 import org.elasticsearch.compute.data.IntVector;
 import org.elasticsearch.compute.data.LongArrayBlock;
@@ -62,7 +63,7 @@ private IntVector add(LongVector vector) {
         for (int i = 0; i < vector.getPositionCount(); i++) {
             groups[i] = Math.toIntExact(hashOrdToGroupNullReserved(longHash.add(vector.getLong(i))));
         }
-        return vector.blockFactory().newIntArrayVector(groups, groups.length);
+        return new IntArrayVector(groups, groups.length);
     }
 
     private IntBlock add(LongBlock block) {

diff --git a/...compute/src/test/java/org/elasticsearch/compute/aggregation/blockhash/BlockHashTests.java b/...compute/src/test/java/org/elasticsearch/compute/aggregation/blockhash/BlockHashTests.java
@@ -28,6 +28,7 @@
 import org.elasticsearch.compute.data.Page;
 import org.elasticsearch.compute.operator.HashAggregationOperator;
 import org.elasticsearch.core.Releasables;
+import org.elasticsearch.indices.breaker.CircuitBreakerService;
 import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
 import org.elasticsearch.test.ESTestCase;
 import org.junit.After;
@@ -46,11 +47,14 @@
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.startsWith;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class BlockHashTests extends ESTestCase {
 
-    static final CircuitBreaker breaker = new MockBigArrays.LimitedBreaker("esql-test-breaker", ByteSizeValue.ofGb(1));
-    static final BlockFactory blockFactory = BlockFactory.getInstance(breaker, BigArrays.NON_RECYCLING_INSTANCE);
+    final CircuitBreaker breaker = new MockBigArrays.LimitedBreaker("esql-test-breaker", ByteSizeValue.ofGb(1));
+    final BigArrays bigArrays = new MockBigArrays(PageCacheRecycler.NON_RECYCLING_INSTANCE, mockBreakerService(breaker));
+    final BlockFactory blockFactory = BlockFactory.getInstance(breaker, bigArrays);
 
     @ParametersFactory
     public static List<Object[]> params() {
@@ -1189,4 +1193,11 @@ private void assertKeys(Block[] actualKeys, Object[][] expectedKeys) {
             }
         }
     }
+
+    // A breaker service that always returns the given breaker for getBreaker(CircuitBreaker.REQUEST)
+    static CircuitBreakerService mockBreakerService(CircuitBreaker breaker) {
+        CircuitBreakerService breakerService = mock(CircuitBreakerService.class);
+        when(breakerService.getBreaker(CircuitBreaker.REQUEST)).thenReturn(breaker);
+        return breakerService;
+    }
 }