From 96a3c00754223626ff1df3799522277a086ec0a1 Mon Sep 17 00:00:00 2001
From: Soumyava <93540295+somu-imply@users.noreply.github.com>
Date: Mon, 15 May 2023 20:10:36 -0700
Subject: [PATCH] =?UTF-8?q?Fixing=20an=20issue=20with=20filtering=20on=20a?=
 =?UTF-8?q?=20single=20dimension=20by=20converting=20In=E2=80=A6=20(#14277?=
 =?UTF-8?q?)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Fixing an issue with filtering on a single dimension by converting In filter to a selector filter as needed with Filters.toFilter

* Adding a test so that any future refactoring does not break this behavior

* Made comment a bit more meaningful
---
 .../segment/join/JoinableFactoryWrapper.java  |  6 ++-
 .../join/JoinableFactoryWrapperTest.java      | 43 +++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/processing/src/main/java/org/apache/druid/segment/join/JoinableFactoryWrapper.java b/processing/src/main/java/org/apache/druid/segment/join/JoinableFactoryWrapper.java
index 94c98f8ae2b..df4d14cf621 100644
--- a/processing/src/main/java/org/apache/druid/segment/join/JoinableFactoryWrapper.java
+++ b/processing/src/main/java/org/apache/druid/segment/join/JoinableFactoryWrapper.java
@@ -30,6 +30,7 @@ import org.apache.druid.java.util.common.Pair;
 import org.apache.druid.query.filter.Filter;
 import org.apache.druid.query.filter.InDimFilter;
 import org.apache.druid.segment.filter.FalseFilter;
+import org.apache.druid.segment.filter.Filters;
 import org.apache.druid.utils.CollectionUtils;
 
 import javax.annotation.Nullable;
@@ -177,7 +178,10 @@ public class JoinableFactoryWrapper
         }
         return new JoinClauseToFilterConversion(null, false);
       }
-      final Filter onlyFilter = new InDimFilter(leftColumn, columnValuesWithUniqueFlag.getColumnValues());
+      final Filter onlyFilter = Filters.toFilter(new InDimFilter(
+          leftColumn,
+          columnValuesWithUniqueFlag.getColumnValues()
+      ));
       if (!columnValuesWithUniqueFlag.isAllUnique()) {
         joinClauseFullyConverted = false;
       }
diff --git a/processing/src/test/java/org/apache/druid/segment/join/JoinableFactoryWrapperTest.java b/processing/src/test/java/org/apache/druid/segment/join/JoinableFactoryWrapperTest.java
index 8ff6d8461f8..f4ee1676b59 100644
--- a/processing/src/test/java/org/apache/druid/segment/join/JoinableFactoryWrapperTest.java
+++ b/processing/src/test/java/org/apache/druid/segment/join/JoinableFactoryWrapperTest.java
@@ -36,6 +36,7 @@ import org.apache.druid.query.filter.InDimFilter;
 import org.apache.druid.segment.column.ColumnType;
 import org.apache.druid.segment.column.RowSignature;
 import org.apache.druid.segment.filter.FalseFilter;
+import org.apache.druid.segment.filter.SelectorFilter;
 import org.apache.druid.segment.join.lookup.LookupJoinable;
 import org.apache.druid.segment.join.table.IndexedTable;
 import org.apache.druid.segment.join.table.IndexedTableJoinable;
@@ -100,6 +101,13 @@ public class JoinableFactoryWrapperTest extends NullHandlingTest
                   .build()
   );
 
+  private static final InlineDataSource INDEXED_TABLE_DS_ONE_ROW = InlineDataSource.fromIterable(
+      ImmutableList.of(
+          new Object[]{"Mexico"}
+      ),
+      RowSignature.builder().add("country", ColumnType.STRING).build()
+  );
+
   private static final InlineDataSource NULL_INDEXED_TABLE_DS = InlineDataSource.fromIterable(
       ImmutableList.of(
           new Object[]{null}
@@ -123,6 +131,14 @@ public class JoinableFactoryWrapperTest extends NullHandlingTest
       DateTimes.nowUtc().toString()
   );
 
+  private static final IndexedTable TEST_INDEXED_TABLE_ONE_ROW = new RowBasedIndexedTable<>(
+      INDEXED_TABLE_DS_ONE_ROW.getRowsAsList(),
+      INDEXED_TABLE_DS_ONE_ROW.rowAdapter(),
+      INDEXED_TABLE_DS_ONE_ROW.getRowSignature(),
+      ImmutableSet.of("country"),
+      DateTimes.nowUtc().toString()
+  );
+
   private static final IndexedTable TEST_NULL_INDEXED_TABLE = new RowBasedIndexedTable<>(
       NULL_INDEXED_TABLE_DS.getRowsAsList(),
       NULL_INDEXED_TABLE_DS.rowAdapter(),
@@ -240,6 +256,33 @@ public class JoinableFactoryWrapperTest extends NullHandlingTest
     );
   }
 
+  @Test
+  public void test_convertJoinsToPartialFiltersWithSelectorFiltersInsteadOfInsForSingleValue()
+  {
+    JoinableClause joinableClause = new JoinableClause(
+        "j.",
+        new IndexedTableJoinable(TEST_INDEXED_TABLE_ONE_ROW),
+        JoinType.INNER,
+        JoinConditionAnalysis.forExpression("x == \"j.country\"", "j.", ExprMacroTable.nil())
+    );
+    final Pair<List<Filter>, List<JoinableClause>> conversion = JoinableFactoryWrapper.convertJoinsToFilters(
+        ImmutableList.of(joinableClause),
+        ImmutableSet.of("x"),
+        Integer.MAX_VALUE
+    );
+
+    // Although the filter created was an In Filter in equijoin (here inFilter = IN (Mexico))
+    // We should receive a SelectorFilter for Filters.toFilter(inFilter) call
+    // and should receive a SelectorFilter with x = Mexico
+    Assert.assertEquals(
+        Pair.of(
+            ImmutableList.of(new SelectorFilter("x", "Mexico", null)),
+            ImmutableList.of()
+        ),
+        conversion
+    );
+  }
+
   @Test
   public void test_convertJoinsToFilters_convertTwoInnerJoins()
   {