From 4749378e4bb1c8d972348bbe3de58049416e3ba4 Mon Sep 17 00:00:00 2001
From: Raunaq Morarka <raunaqmorarka@gmail.com>
Date: Thu, 24 Jul 2025 12:46:43 +0530
Subject: [PATCH] Remove unnecessary usage of SimplePagesHashStrategy in
 PagesIndex

Compiled version of PagesHashStrategy results in same code as SimplePagesHashStrategy
when joinChannels is empty
---
 .../java/io/trino/operator/PagesIndex.java    | 32 ++-----------------
 .../io/trino/sql/gen/TestJoinCompiler.java    | 30 +++++++++++++++++
 2 files changed, 33 insertions(+), 29 deletions(-)

diff --git a/core/trino-main/src/main/java/io/trino/operator/PagesIndex.java b/core/trino-main/src/main/java/io/trino/operator/PagesIndex.java
index 373d031f4a3c..f0986137fbcd 100644
--- a/core/trino-main/src/main/java/io/trino/operator/PagesIndex.java
+++ b/core/trino-main/src/main/java/io/trino/operator/PagesIndex.java
@@ -50,7 +50,6 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.OptionalDouble;
-import java.util.OptionalInt;
 import java.util.function.Supplier;
 import java.util.stream.IntStream;
 import java.util.stream.Stream;
@@ -526,40 +525,15 @@ public LookupSourceSupplier createLookupSourceSupplier(
             HashArraySizeSupplier hashArraySizeSupplier)
     {
         List<ObjectArrayList<Block>> channels = ImmutableList.copyOf(this.channels);
-        if (!joinChannels.isEmpty()) {
-            // todo compiled implementation of lookup join does not support when we are joining with empty join channels.
-            // This code path will trigger only for OUTER joins. To fix that we need to add support for
-            //        OUTER joins into NestedLoopsJoin and remove "type == INNER" condition in LocalExecutionPlanner.visitJoin()
-
-            LookupSourceSupplierFactory lookupSourceFactory = joinCompiler.compileLookupSourceFactory(types, joinChannels, sortChannel, outputChannels);
-            return lookupSourceFactory.createLookupSourceSupplier(
-                    session,
-                    valueAddresses,
-                    channels,
-                    filterFunctionFactory,
-                    sortChannel,
-                    searchFunctionFactories,
-                    hashArraySizeSupplier);
-        }
-
-        PagesHashStrategy hashStrategy = new SimplePagesHashStrategy(
-                types,
-                outputChannels.orElseGet(() -> rangeList(types.size())),
-                channels,
-                joinChannels,
-                sortChannel,
-                blockTypeOperators);
-
-        return new JoinHashSupplier(
+        LookupSourceSupplierFactory lookupSourceFactory = joinCompiler.compileLookupSourceFactory(types, joinChannels, sortChannel, outputChannels);
+        return lookupSourceFactory.createLookupSourceSupplier(
                 session,
-                hashStrategy,
                 valueAddresses,
                 channels,
                 filterFunctionFactory,
                 sortChannel,
                 searchFunctionFactories,
-                hashArraySizeSupplier,
-                OptionalInt.empty());
+                hashArraySizeSupplier);
     }
 
     private static List<Integer> rangeList(int endExclusive)
diff --git a/core/trino-main/src/test/java/io/trino/sql/gen/TestJoinCompiler.java b/core/trino-main/src/test/java/io/trino/sql/gen/TestJoinCompiler.java
index a9c0e57f1fa1..b716d340d01f 100644
--- a/core/trino-main/src/test/java/io/trino/sql/gen/TestJoinCompiler.java
+++ b/core/trino-main/src/test/java/io/trino/sql/gen/TestJoinCompiler.java
@@ -50,6 +50,36 @@ public class TestJoinCompiler
     private final BlockTypeOperators blockTypeOperators = new BlockTypeOperators(typeOperators);
     private final JoinCompiler joinCompiler = new JoinCompiler(typeOperators);
 
+    @Test
+    void testEmptyChannels()
+    {
+        // compile hash strategy with empty join channels
+        PagesHashStrategyFactory pagesHashStrategyFactory = joinCompiler.compilePagesHashStrategyFactory(ImmutableList.of(BIGINT), ImmutableList.of());
+
+        // create hash strategy with a single channel blocks
+        ObjectArrayList<Block> channel = new ObjectArrayList<>();
+        channel.add(BlockAssertions.createStringSequenceBlock(10, 20));
+        channel.add(BlockAssertions.createStringSequenceBlock(20, 30));
+        channel.add(BlockAssertions.createStringSequenceBlock(15, 25));
+
+        List<ObjectArrayList<Block>> channels = ImmutableList.of(channel);
+        PagesHashStrategy hashStrategy = pagesHashStrategyFactory.createPagesHashStrategy(channels);
+
+        assertThat(hashStrategy.getChannelCount()).isEqualTo(1);
+        assertThat(hashStrategy.getSizeInBytes()).isGreaterThan(0L);
+        assertThat(hashStrategy.hashRow(0, new Page(channel.get(0)))).isEqualTo(0L);
+        assertThat(hashStrategy.hashPosition(0, 5)).isEqualTo(0L);
+        assertThat(hashStrategy.positionEqualsPositionIgnoreNulls(0, 5, 0, 5)).isTrue();
+        assertThat(hashStrategy.positionEqualsPosition(0, 5, 0, 5)).isTrue();
+        assertThat(hashStrategy.positionIdenticalToPosition(0, 5, 0, 5)).isTrue();
+        assertThat(hashStrategy.positionEqualsRow(0, 5, 5, new Page(channel.get(0)))).isTrue();
+        assertThat(hashStrategy.positionIdenticalToRow(0, 5, 5, new Page(channel.get(0)))).isTrue();
+        assertThat(hashStrategy.rowEqualsRow(5, new Page(channel.get(0)), 5, new Page(channel.get(0)))).isTrue();
+        assertThat(hashStrategy.rowIdenticalToRow(5, new Page(channel.get(0)), 5, new Page(channel.get(0)))).isTrue();
+        assertThat(hashStrategy.positionEqualsRowIgnoreNulls(0, 5, 5, new Page(channel.get(0)))).isTrue();
+        assertThat(hashStrategy.positionEqualsPositionIgnoreNulls(0, 5, 0, 5)).isTrue();
+    }
+
     @Test
     public void testSingleChannel()
     {