Skip to content

Commit ed18e7c

Browse files
committed
Make operator tests not depend directly on the BlockHash implementation
1 parent 9d8f749 commit ed18e7c

File tree

2 files changed

+13
-11
lines changed

2 files changed

+13
-11
lines changed

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/blockhash/LongTopNBlockHash.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@
3131
/**
3232
* Maps a {@link LongBlock} column to group ids, keeping only the top N values.
3333
*/
34-
// TODO: package-private instead of public?
35-
public final class LongTopNBlockHash extends BlockHash {
34+
final class LongTopNBlockHash extends BlockHash {
3635
private final int channel;
3736
private final boolean asc;
3837
private final boolean nullsFirst;

x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/operator/HashAggregationOperatorTests.java

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import org.elasticsearch.compute.aggregation.SumLongAggregatorFunctionSupplier;
1616
import org.elasticsearch.compute.aggregation.SumLongGroupingAggregatorFunctionTests;
1717
import org.elasticsearch.compute.aggregation.blockhash.BlockHash;
18-
import org.elasticsearch.compute.aggregation.blockhash.LongTopNBlockHash;
1918
import org.elasticsearch.compute.data.Block;
2019
import org.elasticsearch.compute.data.BlockFactory;
2120
import org.elasticsearch.compute.data.BlockUtils;
@@ -113,14 +112,16 @@ public void testTopNNullsLast() {
113112
var aggregatorChannels = List.of(1);
114113

115114
try (
116-
var operator = new HashAggregationOperator(
115+
var operator = new HashAggregationOperator.HashAggregationOperatorFactory(
116+
List.of(new BlockHash.GroupSpec(groupChannel, ElementType.LONG, false, new BlockHash.TopNDef(0, ascOrder, false, 3))),
117+
mode,
117118
List.of(
118119
new SumLongAggregatorFunctionSupplier().groupingAggregatorFactory(mode, aggregatorChannels),
119120
new MaxLongAggregatorFunctionSupplier().groupingAggregatorFactory(mode, aggregatorChannels)
120121
),
121-
() -> new LongTopNBlockHash(groupChannel, ascOrder, false, 3, blockFactory()),
122-
driverContext()
123-
)
122+
randomPageSize(),
123+
null
124+
).get(driverContext())
124125
) {
125126
var page = new Page(
126127
BlockUtils.fromList(
@@ -188,14 +189,16 @@ public void testTopNNullsFirst() {
188189
var aggregatorChannels = List.of(1);
189190

190191
try (
191-
var operator = new HashAggregationOperator(
192+
var operator = new HashAggregationOperator.HashAggregationOperatorFactory(
193+
List.of(new BlockHash.GroupSpec(groupChannel, ElementType.LONG, false, new BlockHash.TopNDef(0, ascOrder, true, 3))),
194+
mode,
192195
List.of(
193196
new SumLongAggregatorFunctionSupplier().groupingAggregatorFactory(mode, aggregatorChannels),
194197
new MaxLongAggregatorFunctionSupplier().groupingAggregatorFactory(mode, aggregatorChannels)
195198
),
196-
() -> new LongTopNBlockHash(groupChannel, ascOrder, true, 3, blockFactory()),
197-
driverContext()
198-
)
199+
randomPageSize(),
200+
null
201+
).get(driverContext())
199202
) {
200203
var page = new Page(
201204
BlockUtils.fromList(

0 commit comments

Comments
 (0)