Skip to content

Commit 88297b1

Browse files
raunaqmorarkaraunaqmorarka
committed
Remove unused code for pre-computed hashes in join nodes
This code became unused after removal of HashGenerationOptimizer
1 parent 223bcc5 commit 88297b1

File tree

108 files changed

+640
-1838
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

108 files changed

+640
-1838
lines changed

‎core/trino-main/src/main/java/io/trino/operator/ChannelSet.java‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import static io.trino.spi.function.InvocationConvention.InvocationReturnConvention.FAIL_ON_NULL;
2525
import static io.trino.spi.function.InvocationConvention.InvocationReturnConvention.FLAT_RETURN;
2626
import static io.trino.spi.function.InvocationConvention.simpleConvention;
27-
import static io.trino.spi.type.BigintType.BIGINT;
2827
import static java.util.Objects.requireNonNull;
2928

3029
public class ChannelSet
@@ -88,24 +87,14 @@ public ChannelSet build()
8887
return new ChannelSet(set);
8988
}
9089

91-
public void addAll(Block valueBlock, Block hashBlock)
90+
public void addAll(Block valueBlock)
9291
{
9392
if (valueBlock.getPositionCount() == 0) {
9493
return;
9594
}
9695

9796
if (valueBlock instanceof RunLengthEncodedBlock rleBlock) {
98-
if (hashBlock != null) {
99-
set.add(rleBlock.getValue(), 0, BIGINT.getLong(hashBlock, 0));
100-
}
101-
else {
102-
set.add(rleBlock.getValue(), 0);
103-
}
104-
}
105-
else if (hashBlock != null) {
106-
for (int position = 0; position < valueBlock.getPositionCount(); position++) {
107-
set.add(valueBlock, position, BIGINT.getLong(hashBlock, position));
108-
}
97+
set.add(rleBlock.getValue(), 0);
10998
}
11099
else {
111100
for (int position = 0; position < valueBlock.getPositionCount(); position++) {

‎core/trino-main/src/main/java/io/trino/operator/HashSemiJoinOperator.java‎

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import jakarta.annotation.Nullable;
3030

3131
import java.util.List;
32-
import java.util.Optional;
3332

3433
import static com.google.common.base.Preconditions.checkArgument;
3534
import static com.google.common.base.Preconditions.checkState;
@@ -40,7 +39,6 @@
4039
import static io.trino.operator.WorkProcessor.TransformationState.finished;
4140
import static io.trino.operator.WorkProcessor.TransformationState.ofResult;
4241
import static io.trino.operator.WorkProcessorOperatorAdapter.createAdapterOperatorFactory;
43-
import static io.trino.spi.type.BigintType.BIGINT;
4442
import static io.trino.spi.type.BooleanType.BOOLEAN;
4543
import static java.util.Objects.requireNonNull;
4644

@@ -52,10 +50,9 @@ public static OperatorFactory createOperatorFactory(
5250
PlanNodeId planNodeId,
5351
SetSupplier setSupplier,
5452
List<? extends Type> probeTypes,
55-
int probeJoinChannel,
56-
Optional<Integer> probeJoinHashChannel)
53+
int probeJoinChannel)
5754
{
58-
return createAdapterOperatorFactory(new Factory(operatorId, planNodeId, setSupplier, probeTypes, probeJoinChannel, probeJoinHashChannel));
55+
return createAdapterOperatorFactory(new Factory(operatorId, planNodeId, setSupplier, probeTypes, probeJoinChannel));
5956
}
6057

6158
private static class Factory
@@ -66,25 +63,23 @@ private static class Factory
6663
private final SetSupplier setSupplier;
6764
private final List<Type> probeTypes;
6865
private final int probeJoinChannel;
69-
private final Optional<Integer> probeJoinHashChannel;
7066
private boolean closed;
7167

72-
private Factory(int operatorId, PlanNodeId planNodeId, SetSupplier setSupplier, List<? extends Type> probeTypes, int probeJoinChannel, Optional<Integer> probeJoinHashChannel)
68+
private Factory(int operatorId, PlanNodeId planNodeId, SetSupplier setSupplier, List<? extends Type> probeTypes, int probeJoinChannel)
7369
{
7470
this.operatorId = operatorId;
7571
this.planNodeId = requireNonNull(planNodeId, "planNodeId is null");
7672
this.setSupplier = setSupplier;
7773
this.probeTypes = ImmutableList.copyOf(probeTypes);
7874
checkArgument(probeJoinChannel >= 0, "probeJoinChannel is negative");
7975
this.probeJoinChannel = probeJoinChannel;
80-
this.probeJoinHashChannel = probeJoinHashChannel;
8176
}
8277

8378
@Override
8479
public WorkProcessorOperator create(ProcessorContext processorContext, WorkProcessor<Page> sourcePages)
8580
{
8681
checkState(!closed, "Factory is already closed");
87-
return new HashSemiJoinOperator(sourcePages, setSupplier, probeJoinChannel, probeJoinHashChannel, processorContext.getMemoryTrackingContext());
82+
return new HashSemiJoinOperator(sourcePages, setSupplier, probeJoinChannel, processorContext.getMemoryTrackingContext());
8883
}
8984

9085
@Override
@@ -114,7 +109,7 @@ public void close()
114109
@Override
115110
public Factory duplicate()
116111
{
117-
return new Factory(operatorId, planNodeId, setSupplier, probeTypes, probeJoinChannel, probeJoinHashChannel);
112+
return new Factory(operatorId, planNodeId, setSupplier, probeTypes, probeJoinChannel);
118113
}
119114
}
120115

@@ -124,14 +119,12 @@ private HashSemiJoinOperator(
124119
WorkProcessor<Page> sourcePages,
125120
SetSupplier channelSetFuture,
126121
int probeJoinChannel,
127-
Optional<Integer> probeHashChannel,
128122
MemoryTrackingContext memoryTrackingContext)
129123
{
130124
pages = sourcePages
131125
.transform(new SemiJoinPages(
132126
channelSetFuture,
133127
probeJoinChannel,
134-
probeHashChannel,
135128
memoryTrackingContext.aggregateUserMemoryContext()));
136129
}
137130

@@ -144,23 +137,19 @@ public WorkProcessor<Page> getOutputPages()
144137
private static class SemiJoinPages
145138
implements WorkProcessor.Transformation<Page, Page>
146139
{
147-
private static final int NO_PRECOMPUTED_HASH_CHANNEL = -1;
148-
149140
private final int probeJoinChannel;
150-
private final int probeHashChannel; // when >= 0, this is the precomputed hash channel
151141
private final ListenableFuture<ChannelSet> channelSetFuture;
152142
private final LocalMemoryContext localMemoryContext;
153143

154144
@Nullable
155145
private ChannelSet channelSet;
156146

157-
public SemiJoinPages(SetSupplier channelSetFuture, int probeJoinChannel, Optional<Integer> probeHashChannel, AggregatedMemoryContext aggregatedMemoryContext)
147+
public SemiJoinPages(SetSupplier channelSetFuture, int probeJoinChannel, AggregatedMemoryContext aggregatedMemoryContext)
158148
{
159149
checkArgument(probeJoinChannel >= 0, "probeJoinChannel is negative");
160150

161151
this.channelSetFuture = channelSetFuture.getChannelSet();
162152
this.probeJoinChannel = probeJoinChannel;
163-
this.probeHashChannel = probeHashChannel.orElse(NO_PRECOMPUTED_HASH_CHANNEL);
164153
this.localMemoryContext = aggregatedMemoryContext.newLocalMemoryContext(SemiJoinPages.class.getSimpleName());
165154
}
166155

@@ -190,7 +179,6 @@ public TransformationState<Page> process(Page inputPage)
190179

191180
Block probeBlock = inputPage.getBlock(probeJoinChannel).copyRegion(0, inputPage.getPositionCount());
192181
boolean probeMayHaveNull = probeBlock.mayHaveNull();
193-
Block hashBlock = probeHashChannel >= 0 ? inputPage.getBlock(probeHashChannel).copyRegion(0, inputPage.getPositionCount()) : null;
194182

195183
// update hashing strategy to use probe cursor
196184
for (int position = 0; position < inputPage.getPositionCount(); position++) {
@@ -203,14 +191,7 @@ public TransformationState<Page> process(Page inputPage)
203191
}
204192
}
205193
else {
206-
boolean contains;
207-
if (hashBlock != null) {
208-
long rawHash = BIGINT.getLong(hashBlock, position);
209-
contains = channelSet.contains(probeBlock, position, rawHash);
210-
}
211-
else {
212-
contains = channelSet.contains(probeBlock, position);
213-
}
194+
boolean contains = channelSet.contains(probeBlock, position);
214195
if (!contains && channelSet.containsNull()) {
215196
blockBuilder.appendNull();
216197
}

‎core/trino-main/src/main/java/io/trino/operator/OperatorFactories.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ public static OperatorFactory join(
4444
boolean hasFilter,
4545
List<Type> probeTypes,
4646
List<Integer> probeJoinChannel,
47-
OptionalInt probeHashChannel,
4847
Optional<List<Integer>> probeOutputChannelsOptional)
4948
{
5049
List<Integer> probeOutputChannels = probeOutputChannelsOptional.orElseGet(() -> rangeList(probeTypes.size()));
@@ -60,7 +59,7 @@ public static OperatorFactory join(
6059
probeOutputChannelTypes,
6160
lookupSourceFactory.getBuildOutputTypes(),
6261
joinType,
63-
new JoinProbe.JoinProbeFactory(probeOutputChannels, probeJoinChannel, probeHashChannel, hasFilter)));
62+
new JoinProbe.JoinProbeFactory(probeOutputChannels, probeJoinChannel, hasFilter)));
6463
}
6564

6665
public static OperatorFactory spillingJoin(
@@ -70,7 +69,6 @@ public static OperatorFactory spillingJoin(
7069
JoinBridgeManager<? extends LookupSourceFactory> lookupSourceFactory,
7170
List<Type> probeTypes,
7271
List<Integer> probeJoinChannel,
73-
OptionalInt probeHashChannel,
7472
Optional<List<Integer>> probeOutputChannelsOptional,
7573
OptionalInt totalOperatorsCount,
7674
PartitioningSpillerFactory partitioningSpillerFactory,
@@ -89,11 +87,10 @@ public static OperatorFactory spillingJoin(
8987
probeOutputChannelTypes,
9088
lookupSourceFactory.getBuildOutputTypes(),
9189
joinType,
92-
new JoinProbeFactory(probeOutputChannels.stream().mapToInt(i -> i).toArray(), probeJoinChannel, probeHashChannel),
90+
new JoinProbeFactory(probeOutputChannels.stream().mapToInt(i -> i).toArray(), probeJoinChannel),
9391
typeOperators,
9492
totalOperatorsCount,
9593
probeJoinChannel,
96-
probeHashChannel,
9794
partitioningSpillerFactory));
9895
}
9996

‎core/trino-main/src/main/java/io/trino/operator/PagesIndex.java‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -456,19 +456,19 @@ private PagesIndexOrdering createPagesIndexComparator(List<Integer> sortChannels
456456

457457
public Supplier<LookupSource> createLookupSourceSupplier(Session session, List<Integer> joinChannels)
458458
{
459-
return createLookupSourceSupplier(session, joinChannels, OptionalInt.empty(), Optional.empty(), Optional.empty(), ImmutableList.of());
459+
return createLookupSourceSupplier(session, joinChannels, Optional.empty(), Optional.empty(), ImmutableList.of());
460460
}
461461

462-
public PagesHashStrategy createPagesHashStrategy(List<Integer> joinChannels, OptionalInt hashChannel)
462+
public PagesHashStrategy createPagesHashStrategy(List<Integer> joinChannels)
463463
{
464-
return createPagesHashStrategy(joinChannels, hashChannel, Optional.empty());
464+
return createPagesHashStrategy(joinChannels, Optional.empty());
465465
}
466466

467-
private PagesHashStrategy createPagesHashStrategy(List<Integer> joinChannels, OptionalInt hashChannel, Optional<List<Integer>> outputChannels)
467+
private PagesHashStrategy createPagesHashStrategy(List<Integer> joinChannels, Optional<List<Integer>> outputChannels)
468468
{
469469
try {
470470
return joinCompiler.compilePagesHashStrategyFactory(types, joinChannels, outputChannels)
471-
.createPagesHashStrategy(ImmutableList.copyOf(channels), hashChannel);
471+
.createPagesHashStrategy(ImmutableList.copyOf(channels));
472472
}
473473
catch (Exception e) {
474474
log.error(e, "Lookup source compile failed for types=%s error=%s", types, e);
@@ -480,7 +480,6 @@ private PagesHashStrategy createPagesHashStrategy(List<Integer> joinChannels, Op
480480
outputChannels.orElseGet(() -> rangeList(types.size())),
481481
ImmutableList.copyOf(channels),
482482
joinChannels,
483-
hashChannel,
484483
Optional.empty(),
485484
blockTypeOperators);
486485
}
@@ -494,12 +493,11 @@ public PagesIndexComparator createChannelComparator(int leftChannel, int rightCh
494493
public LookupSourceSupplier createLookupSourceSupplier(
495494
Session session,
496495
List<Integer> joinChannels,
497-
OptionalInt hashChannel,
498496
Optional<JoinFilterFunctionFactory> filterFunctionFactory,
499497
Optional<Integer> sortChannel,
500498
List<JoinFilterFunctionFactory> searchFunctionFactories)
501499
{
502-
return createLookupSourceSupplier(session, joinChannels, hashChannel, filterFunctionFactory, sortChannel, searchFunctionFactories, Optional.empty(), defaultHashArraySizeSupplier());
500+
return createLookupSourceSupplier(session, joinChannels, filterFunctionFactory, sortChannel, searchFunctionFactories, Optional.empty(), defaultHashArraySizeSupplier());
503501
}
504502

505503
public PagesSpatialIndexSupplier createPagesSpatialIndex(
@@ -521,7 +519,6 @@ public PagesSpatialIndexSupplier createPagesSpatialIndex(
521519
public LookupSourceSupplier createLookupSourceSupplier(
522520
Session session,
523521
List<Integer> joinChannels,
524-
OptionalInt hashChannel,
525522
Optional<JoinFilterFunctionFactory> filterFunctionFactory,
526523
Optional<Integer> sortChannel,
527524
List<JoinFilterFunctionFactory> searchFunctionFactories,
@@ -539,7 +536,6 @@ public LookupSourceSupplier createLookupSourceSupplier(
539536
session,
540537
valueAddresses,
541538
channels,
542-
hashChannel,
543539
filterFunctionFactory,
544540
sortChannel,
545541
searchFunctionFactories,
@@ -551,7 +547,6 @@ public LookupSourceSupplier createLookupSourceSupplier(
551547
outputChannels.orElseGet(() -> rangeList(types.size())),
552548
channels,
553549
joinChannels,
554-
hashChannel,
555550
sortChannel,
556551
blockTypeOperators);
557552

‎core/trino-main/src/main/java/io/trino/operator/SetBuilderOperator.java‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323
import io.trino.sql.gen.JoinCompiler;
2424
import io.trino.sql.planner.plan.PlanNodeId;
2525

26-
import java.util.Optional;
27-
2826
import static com.google.common.base.Preconditions.checkArgument;
2927
import static com.google.common.base.Preconditions.checkState;
3028
import static java.util.Objects.requireNonNull;
@@ -65,7 +63,6 @@ public static class SetBuilderOperatorFactory
6563
{
6664
private final int operatorId;
6765
private final PlanNodeId planNodeId;
68-
private final Optional<Integer> hashChannel;
6966
private final SetSupplier setProvider;
7067
private final int setChannel;
7168
private final int expectedPositions;
@@ -78,7 +75,6 @@ public SetBuilderOperatorFactory(
7875
PlanNodeId planNodeId,
7976
Type type,
8077
int setChannel,
81-
Optional<Integer> hashChannel,
8278
int expectedPositions,
8379
JoinCompiler joinCompiler,
8480
TypeOperators typeOperators)
@@ -88,7 +84,6 @@ public SetBuilderOperatorFactory(
8884
checkArgument(setChannel >= 0, "setChannel is negative");
8985
this.setProvider = new SetSupplier(requireNonNull(type, "type is null"));
9086
this.setChannel = setChannel;
91-
this.hashChannel = requireNonNull(hashChannel, "hashChannel is null");
9287
this.expectedPositions = expectedPositions;
9388
this.joinCompiler = requireNonNull(joinCompiler, "joinCompiler is null");
9489
this.typeOperators = requireNonNull(typeOperators, "blockTypeOperators is null");
@@ -104,7 +99,7 @@ public Operator createOperator(DriverContext driverContext)
10499
{
105100
checkState(!closed, "Factory is already closed");
106101
OperatorContext operatorContext = driverContext.addOperatorContext(operatorId, planNodeId, SetBuilderOperator.class.getSimpleName());
107-
return new SetBuilderOperator(operatorContext, setProvider, setChannel, hashChannel, expectedPositions, joinCompiler, typeOperators);
102+
return new SetBuilderOperator(operatorContext, setProvider, setChannel, expectedPositions, joinCompiler, typeOperators);
108103
}
109104

110105
@Override
@@ -116,14 +111,13 @@ public void noMoreOperators()
116111
@Override
117112
public OperatorFactory duplicate()
118113
{
119-
return new SetBuilderOperatorFactory(operatorId, planNodeId, setProvider.getType(), setChannel, hashChannel, expectedPositions, joinCompiler, typeOperators);
114+
return new SetBuilderOperatorFactory(operatorId, planNodeId, setProvider.getType(), setChannel, expectedPositions, joinCompiler, typeOperators);
120115
}
121116
}
122117

123118
private final OperatorContext operatorContext;
124119
private final SetSupplier setSupplier;
125120
private final int setChannel;
126-
private final int hashChannel;
127121

128122
private final ChannelSetBuilder channelSetBuilder;
129123

@@ -133,7 +127,6 @@ public SetBuilderOperator(
133127
OperatorContext operatorContext,
134128
SetSupplier setSupplier,
135129
int setChannel,
136-
Optional<Integer> hashChannel,
137130
int expectedPositions,
138131
JoinCompiler joinCompiler,
139132
TypeOperators typeOperators)
@@ -142,7 +135,6 @@ public SetBuilderOperator(
142135
this.setSupplier = requireNonNull(setSupplier, "setSupplier is null");
143136

144137
this.setChannel = setChannel;
145-
this.hashChannel = hashChannel.orElse(-1);
146138

147139
// Set builder has a single channel which goes in channel 0, if hash is present, add a hashBlock to channel 1
148140
this.channelSetBuilder = new ChannelSetBuilder(
@@ -189,7 +181,7 @@ public void addInput(Page page)
189181
requireNonNull(page, "page is null");
190182
checkState(!isFinished(), "Operator is already finished");
191183

192-
channelSetBuilder.addAll(page.getBlock(setChannel), hashChannel == -1 ? null : page.getBlock(hashChannel));
184+
channelSetBuilder.addAll(page.getBlock(setChannel));
193185
}
194186

195187
@Override

0 commit comments

Comments
 (0)