Skip to content

Commit feb4cfe

Browse files
raunaqmorarkaraunaqmorarka
committed
Clean up pre-computed hashes related code in tests
1 parent d6891f2 commit feb4cfe

16 files changed

+241
-389
lines changed

‎core/trino-main/src/test/java/io/trino/RowPagesBuilder.java

Lines changed: 7 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@
1717
import com.google.common.collect.Iterables;
1818
import io.trino.spi.Page;
1919
import io.trino.spi.block.Block;
20-
import io.trino.spi.type.BigintType;
2120
import io.trino.spi.type.Type;
22-
import io.trino.type.TypeTestUtils;
2321

2422
import java.util.List;
2523
import java.util.Optional;
@@ -40,31 +38,29 @@ public static RowPagesBuilder rowPagesBuilder(Iterable<Type> types)
4038
return new RowPagesBuilder(types);
4139
}
4240

43-
public static RowPagesBuilder rowPagesBuilder(boolean hashEnabled, List<Integer> hashChannels, Type... types)
41+
public static RowPagesBuilder rowPagesBuilder(List<Integer> hashChannels, Type... types)
4442
{
45-
return rowPagesBuilder(hashEnabled, hashChannels, ImmutableList.copyOf(types));
43+
return rowPagesBuilder(hashChannels, ImmutableList.copyOf(types));
4644
}
4745

48-
public static RowPagesBuilder rowPagesBuilder(boolean hashEnabled, List<Integer> hashChannels, Iterable<Type> types)
46+
public static RowPagesBuilder rowPagesBuilder(List<Integer> hashChannels, Iterable<Type> types)
4947
{
50-
return new RowPagesBuilder(hashEnabled, Optional.of(hashChannels), types);
48+
return new RowPagesBuilder(Optional.of(hashChannels), types);
5149
}
5250

5351
private final ImmutableList.Builder<Page> pages = ImmutableList.builder();
5452
private final List<Type> types;
5553
private RowPageBuilder builder;
56-
private final boolean hashEnabled;
5754
private final Optional<List<Integer>> hashChannels;
5855

5956
RowPagesBuilder(Iterable<Type> types)
6057
{
61-
this(false, Optional.empty(), types);
58+
this(Optional.empty(), types);
6259
}
6360

64-
RowPagesBuilder(boolean hashEnabled, Optional<List<Integer>> hashChannels, Iterable<Type> types)
61+
RowPagesBuilder(Optional<List<Integer>> hashChannels, Iterable<Type> types)
6562
{
6663
this.types = ImmutableList.copyOf(requireNonNull(types, "types is null"));
67-
this.hashEnabled = hashEnabled;
6864
this.hashChannels = hashChannels.map(ImmutableList::copyOf);
6965
builder = rowPageBuilder(types);
7066
}
@@ -118,31 +114,10 @@ public RowPagesBuilder pageBreak()
118114
public List<Page> build()
119115
{
120116
pageBreak();
121-
List<Page> resultPages = pages.build();
122-
if (hashEnabled) {
123-
return pagesWithHash(resultPages);
124-
}
125-
return resultPages;
126-
}
127-
128-
private List<Page> pagesWithHash(List<Page> pages)
129-
{
130-
ImmutableList.Builder<Page> resultPages = ImmutableList.builder();
131-
for (Page page : pages) {
132-
resultPages.add(TypeTestUtils.getHashPage(page, types, hashChannels.get()));
133-
}
134-
return resultPages.build();
117+
return pages.build();
135118
}
136119

137120
public List<Type> getTypes()
138-
{
139-
if (hashEnabled) {
140-
return ImmutableList.copyOf(Iterables.concat(types, ImmutableList.of(BigintType.BIGINT)));
141-
}
142-
return types;
143-
}
144-
145-
public List<Type> getTypesWithoutHash()
146121
{
147122
return types;
148123
}
@@ -151,12 +126,4 @@ public Optional<List<Integer>> getHashChannels()
151126
{
152127
return hashChannels;
153128
}
154-
155-
public Optional<Integer> getHashChannel()
156-
{
157-
if (hashEnabled) {
158-
return Optional.of(types.size());
159-
}
160-
return Optional.empty();
161-
}
162129
}

‎core/trino-main/src/test/java/io/trino/operator/BenchmarkHashAndStreamingAggregationOperators.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,6 @@ public void setup()
141141
}
142142

143143
RowPagesBuilder pagesBuilder = RowPagesBuilder.rowPagesBuilder(
144-
hashAggregation,
145144
hashChannels,
146145
ImmutableList.<Type>builder()
147146
.addAll(hashTypes)

‎core/trino-main/src/test/java/io/trino/operator/BenchmarkWindowOperator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ private List<Page> generateTestData()
172172
private RowPagesBuilder buildPages(int currentPartitionIdentifier, List<Type> typesArray)
173173
{
174174
int groupIdentifier = 100;
175-
RowPagesBuilder rowPagesBuilder = RowPagesBuilder.rowPagesBuilder(false, ImmutableList.of(0), typesArray);
175+
RowPagesBuilder rowPagesBuilder = RowPagesBuilder.rowPagesBuilder(ImmutableList.of(0), typesArray);
176176

177177
for (int i = 0; i < TOTAL_PAGES; i++) {
178178
BlockBuilder firstColumnBlockBuilder = BIGINT.createFixedSizeBlockBuilder(ROWS_PER_PAGE);

‎core/trino-main/src/test/java/io/trino/operator/GroupByHashYieldAssertion.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ public final class GroupByHashYieldAssertion
5555

5656
private GroupByHashYieldAssertion() {}
5757

58-
public static List<Page> createPagesWithDistinctHashKeys(Type type, int pageCount, int positionCountPerPage)
58+
public static List<Page> createPages(Type type, int pageCount, int positionCountPerPage)
5959
{
60-
RowPagesBuilder rowPagesBuilder = rowPagesBuilder(true, ImmutableList.of(0), type);
60+
RowPagesBuilder rowPagesBuilder = rowPagesBuilder(ImmutableList.of(0), type);
6161
for (int i = 0; i < pageCount; i++) {
6262
rowPagesBuilder.addSequencePage(positionCountPerPage, positionCountPerPage * i);
6363
}

‎core/trino-main/src/test/java/io/trino/operator/OperatorAssertion.java

Lines changed: 3 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import java.util.Collection;
3131
import java.util.Iterator;
3232
import java.util.List;
33-
import java.util.Optional;
3433
import java.util.Set;
3534
import java.util.concurrent.ExecutionException;
3635
import java.util.concurrent.TimeUnit;
@@ -229,42 +228,12 @@ public static void assertOperatorEquals(
229228
List<Page> input,
230229
MaterializedResult expected,
231230
boolean revokeMemoryWhenAddingPages)
232-
{
233-
assertOperatorEquals(operatorFactory, driverContext, input, expected, false, ImmutableList.of(), revokeMemoryWhenAddingPages);
234-
}
235-
236-
public static void assertOperatorEquals(
237-
OperatorFactory operatorFactory,
238-
DriverContext driverContext,
239-
List<Page> input,
240-
MaterializedResult expected,
241-
boolean revokeMemoryWhenAddingPages,
242-
boolean closeOperatorFactory)
243-
{
244-
assertOperatorEquals(operatorFactory, driverContext, input, expected, false, ImmutableList.of(), revokeMemoryWhenAddingPages, closeOperatorFactory);
245-
}
246-
247-
public static void assertOperatorEquals(OperatorFactory operatorFactory, DriverContext driverContext, List<Page> input, MaterializedResult expected, boolean hashEnabled, List<Integer> hashChannels)
248-
{
249-
assertOperatorEquals(operatorFactory, driverContext, input, expected, hashEnabled, hashChannels, true);
250-
}
251-
252-
public static void assertOperatorEquals(
253-
OperatorFactory operatorFactory,
254-
DriverContext driverContext,
255-
List<Page> input,
256-
MaterializedResult expected,
257-
boolean hashEnabled,
258-
List<Integer> hashChannels,
259-
boolean revokeMemoryWhenAddingPages)
260231
{
261232
assertOperatorEquals(
262233
operatorFactory,
263234
driverContext,
264235
input,
265236
expected,
266-
hashEnabled,
267-
hashChannels,
268237
revokeMemoryWhenAddingPages,
269238
true);
270239
}
@@ -274,16 +243,10 @@ public static void assertOperatorEquals(
274243
DriverContext driverContext,
275244
List<Page> input,
276245
MaterializedResult expected,
277-
boolean hashEnabled,
278-
List<Integer> hashChannels,
279246
boolean revokeMemoryWhenAddingPages,
280247
boolean closeOperatorFactory)
281248
{
282249
List<Page> pages = toPages(operatorFactory, driverContext, input, revokeMemoryWhenAddingPages, closeOperatorFactory);
283-
if (hashEnabled && !hashChannels.isEmpty()) {
284-
// Drop the hashChannel for all pages
285-
pages = dropChannel(pages, hashChannels);
286-
}
287250
MaterializedResult actual = toMaterializedResult(driverContext.getSession(), expected.getTypes(), pages);
288251
assertThat(actual).containsExactlyElementsOf(expected);
289252
}
@@ -294,58 +257,27 @@ public static void assertOperatorEqualsIgnoreOrder(
294257
List<Page> input,
295258
MaterializedResult expected)
296259
{
297-
assertOperatorEqualsIgnoreOrder(operatorFactory, driverContext, input, expected, false);
298-
}
299-
300-
public static void assertOperatorEqualsIgnoreOrder(
301-
OperatorFactory operatorFactory,
302-
DriverContext driverContext,
303-
List<Page> input,
304-
MaterializedResult expected,
305-
boolean revokeMemoryWhenAddingPages)
306-
{
307-
assertOperatorEqualsIgnoreOrder(operatorFactory, driverContext, input, expected, false, Optional.empty(), revokeMemoryWhenAddingPages);
308-
}
309-
310-
public static void assertOperatorEqualsIgnoreOrder(
311-
OperatorFactory operatorFactory,
312-
DriverContext driverContext,
313-
List<Page> input,
314-
MaterializedResult expected,
315-
boolean hashEnabled,
316-
Optional<Integer> hashChannel)
317-
{
318-
assertOperatorEqualsIgnoreOrder(operatorFactory, driverContext, input, expected, hashEnabled, hashChannel, true);
260+
assertOperatorEqualsIgnoreOrder(operatorFactory, driverContext, input, expected, true);
319261
}
320262

321263
public static void assertOperatorEqualsIgnoreOrder(
322264
OperatorFactory operatorFactory,
323265
DriverContext driverContext,
324266
List<Page> input,
325267
MaterializedResult expected,
326-
boolean hashEnabled,
327-
Optional<Integer> hashChannel,
328268
boolean revokeMemoryWhenAddingPages)
329269
{
330270
assertPagesEqualIgnoreOrder(
331271
driverContext,
332272
toPages(operatorFactory, driverContext, input, revokeMemoryWhenAddingPages),
333-
expected,
334-
hashEnabled,
335-
hashChannel);
273+
expected);
336274
}
337275

338276
public static void assertPagesEqualIgnoreOrder(
339277
DriverContext driverContext,
340278
List<Page> actualPages,
341-
MaterializedResult expected,
342-
boolean hashEnabled,
343-
Optional<Integer> hashChannel)
279+
MaterializedResult expected)
344280
{
345-
if (hashEnabled && hashChannel.isPresent()) {
346-
// Drop the hashChannel for all pages
347-
actualPages = dropChannel(actualPages, ImmutableList.of(hashChannel.get()));
348-
}
349281
MaterializedResult actual = toMaterializedResult(driverContext.getSession(), expected.getTypes(), actualPages);
350282
assertThat(ImmutableMultiset.copyOf(actual.getMaterializedRows())).isEqualTo(ImmutableMultiset.copyOf(expected.getMaterializedRows()));
351283
}

‎core/trino-main/src/test/java/io/trino/operator/TestDistinctLimitOperator.java

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
import static io.airlift.concurrent.Threads.daemonThreadsNamed;
3434
import static io.trino.RowPagesBuilder.rowPagesBuilder;
3535
import static io.trino.SessionTestUtils.TEST_SESSION;
36-
import static io.trino.operator.GroupByHashYieldAssertion.createPagesWithDistinctHashKeys;
36+
import static io.trino.operator.GroupByHashYieldAssertion.createPages;
3737
import static io.trino.operator.GroupByHashYieldAssertion.finishOperatorWithYieldingGroupByHash;
3838
import static io.trino.operator.OperatorAssertion.assertOperatorEquals;
3939
import static io.trino.spi.type.BigintType.BIGINT;
@@ -63,15 +63,9 @@ public void tearDown()
6363

6464
@Test
6565
public void testDistinctLimit()
66-
{
67-
testDistinctLimit(true);
68-
testDistinctLimit(false);
69-
}
70-
71-
public void testDistinctLimit(boolean hashEnabled)
7266
{
7367
DriverContext driverContext = newDriverContext();
74-
RowPagesBuilder rowPagesBuilder = rowPagesBuilder(hashEnabled, Ints.asList(0), BIGINT);
68+
RowPagesBuilder rowPagesBuilder = rowPagesBuilder(Ints.asList(0), BIGINT);
7569
List<Page> input = rowPagesBuilder
7670
.addSequencePage(3, 1)
7771
.addSequencePage(5, 2)
@@ -93,20 +87,14 @@ public void testDistinctLimit(boolean hashEnabled)
9387
.row(5L)
9488
.build();
9589

96-
assertOperatorEquals(operatorFactory, driverContext, input, expected, hashEnabled, ImmutableList.of(1));
90+
assertOperatorEquals(operatorFactory, driverContext, input, expected);
9791
}
9892

9993
@Test
10094
public void testDistinctLimitWithPageAlignment()
101-
{
102-
testDistinctLimitWithPageAlignment(true);
103-
testDistinctLimitWithPageAlignment(false);
104-
}
105-
106-
public void testDistinctLimitWithPageAlignment(boolean hashEnabled)
10795
{
10896
DriverContext driverContext = newDriverContext();
109-
RowPagesBuilder rowPagesBuilder = rowPagesBuilder(hashEnabled, Ints.asList(0), BIGINT);
97+
RowPagesBuilder rowPagesBuilder = rowPagesBuilder(Ints.asList(0), BIGINT);
11098
List<Page> input = rowPagesBuilder
11199
.addSequencePage(3, 1)
112100
.addSequencePage(3, 2)
@@ -126,21 +114,15 @@ public void testDistinctLimitWithPageAlignment(boolean hashEnabled)
126114
.row(3L)
127115
.build();
128116

129-
assertOperatorEquals(operatorFactory, driverContext, input, expected, hashEnabled, ImmutableList.of(1));
117+
assertOperatorEquals(operatorFactory, driverContext, input, expected);
130118
}
131119

132120
@Test
133121
public void testDistinctLimitValuesLessThanLimit()
134-
{
135-
testDistinctLimitValuesLessThanLimit(true);
136-
testDistinctLimitValuesLessThanLimit(false);
137-
}
138-
139-
public void testDistinctLimitValuesLessThanLimit(boolean hashEnabled)
140122
{
141123
DriverContext driverContext = newDriverContext();
142124

143-
RowPagesBuilder rowPagesBuilder = rowPagesBuilder(hashEnabled, Ints.asList(0), BIGINT);
125+
RowPagesBuilder rowPagesBuilder = rowPagesBuilder(Ints.asList(0), BIGINT);
144126
List<Page> input = rowPagesBuilder
145127
.addSequencePage(3, 1)
146128
.addSequencePage(3, 2)
@@ -161,7 +143,7 @@ public void testDistinctLimitValuesLessThanLimit(boolean hashEnabled)
161143
.row(4L)
162144
.build();
163145

164-
assertOperatorEquals(operatorFactory, driverContext, input, expected, hashEnabled, ImmutableList.of(1));
146+
assertOperatorEquals(operatorFactory, driverContext, input, expected);
165147
}
166148

167149
@Test
@@ -173,7 +155,7 @@ public void testMemoryReservationYield()
173155

174156
public void testMemoryReservationYield(Type type)
175157
{
176-
List<Page> input = createPagesWithDistinctHashKeys(type, 6_000, 600);
158+
List<Page> input = createPages(type, 6_000, 600);
177159

178160
OperatorFactory operatorFactory = new DistinctLimitOperator.DistinctLimitOperatorFactory(
179161
0,

‎core/trino-main/src/test/java/io/trino/operator/TestGroupIdOperator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public void tearDown()
7070
@Test
7171
public void testGroupId()
7272
{
73-
RowPagesBuilder rowPagesBuilder = rowPagesBuilder(false, ImmutableList.of(), BIGINT, VARCHAR, BOOLEAN, BIGINT);
73+
RowPagesBuilder rowPagesBuilder = rowPagesBuilder(ImmutableList.of(), BIGINT, VARCHAR, BOOLEAN, BIGINT);
7474
List<Page> input = rowPagesBuilder
7575
.addSequencePage(3, 100, 400, 0, 1000)
7676
.addSequencePage(3, 200, 500, 0, 1100)

0 commit comments

Comments
 (0)