addressing comments

apache · Aug 2, 2023 · 0508ec4 · 0508ec4
1 parent 27981e5
commit 0508ec4
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 55 deletions.
diff --git a/processing/src/main/java/org/apache/druid/segment/UnnestStorageAdapter.java b/processing/src/main/java/org/apache/druid/segment/UnnestStorageAdapter.java
@@ -364,7 +364,8 @@ class FilterSplitter
 
     public FilterSplitter(
         String inputColumn,
-        ColumnCapabilities inputColumnCapabilites, VirtualColumns queryVirtualColumns
+        ColumnCapabilities inputColumnCapabilites,
+        VirtualColumns queryVirtualColumns
     )
     {
       this.inputColumn = inputColumn;

diff --git a/processing/src/test/java/org/apache/druid/segment/UnnestStorageAdapterTest.java b/processing/src/test/java/org/apache/druid/segment/UnnestStorageAdapterTest.java
@@ -30,6 +30,7 @@
 import org.apache.druid.query.QueryMetrics;
 import org.apache.druid.query.dimension.DefaultDimensionSpec;
 import org.apache.druid.query.filter.Filter;
+import org.apache.druid.query.filter.SelectorDimFilter;
 import org.apache.druid.segment.column.ColumnCapabilities;
 import org.apache.druid.segment.column.ValueType;
 import org.apache.druid.segment.filter.AndFilter;
@@ -57,8 +58,7 @@
 import java.util.Arrays;
 import java.util.List;
 
-import static org.apache.druid.segment.filter.FilterTestUtils.sdf;
-import static org.apache.druid.segment.filter.FilterTestUtils.sdfd;
+import static org.apache.druid.segment.filter.FilterTestUtils.selector;
 import static org.apache.druid.segment.filter.Filters.and;
 import static org.apache.druid.segment.filter.Filters.or;
 
@@ -276,13 +276,13 @@ public void test_pushdown_or_filters_unnested_and_original_dimension_with_unnest
     final String inputColumn = unnestStorageAdapter.getUnnestInputIfDirectAccess(vc);
 
     final OrFilter baseFilter = new OrFilter(ImmutableList.of(
-        sdf(OUTPUT_COLUMN_NAME, "1", null),
-        sdf(inputColumn, "2", null)
+        selector(OUTPUT_COLUMN_NAME, "1"),
+        selector(inputColumn, "2")
     ));
 
     final OrFilter expectedPushDownFilter = new OrFilter(ImmutableList.of(
-        sdf(inputColumn, "1", null),
-        sdf(inputColumn, "2", null)
+        selector(inputColumn, "1"),
+        selector(inputColumn, "2")
     ));
 
     final Sequence<Cursor> cursorSequence = unnestStorageAdapter.makeCursors(
@@ -320,18 +320,18 @@ public void test_nested_filters_unnested_and_original_dimension_with_unnest_adap
     final String inputColumn = unnestStorageAdapter.getUnnestInputIfDirectAccess(vc);
 
     final OrFilter baseFilter = new OrFilter(ImmutableList.of(
-        sdf(OUTPUT_COLUMN_NAME, "1", null),
+        selector(OUTPUT_COLUMN_NAME, "1"),
         new AndFilter(ImmutableList.of(
-            sdf(inputColumn, "2", null),
-            sdf(OUTPUT_COLUMN_NAME, "10", null)
+            selector(inputColumn, "2"),
+            selector(OUTPUT_COLUMN_NAME, "10")
         ))
     ));
 
     final OrFilter expectedPushDownFilter = new OrFilter(ImmutableList.of(
-        sdf(inputColumn, "1", null),
+        selector(inputColumn, "1"),
         new AndFilter(ImmutableList.of(
-            sdf(inputColumn, "2", null),
-            sdf(inputColumn, "10", null)
+            selector(inputColumn, "2"),
+            selector(inputColumn, "10")
         ))
     ));
 
@@ -360,11 +360,11 @@ public void test_nested_filters_unnested_and_original_dimension_with_unnest_adap
   public void test_nested_filters_unnested_and_topLevel1And3filtersInOR()
   {
     final Filter testQueryFilter = and(ImmutableList.of(
-        sdf(OUTPUT_COLUMN_NAME, "3", null),
+        selector(OUTPUT_COLUMN_NAME, "3"),
         or(ImmutableList.of(
-            sdf("newcol", "2", null),
-            sdf(COLUMNNAME, "2", null),
-            sdf(OUTPUT_COLUMN_NAME, "1", null)
+            selector("newcol", "2"),
+            selector(COLUMNNAME, "2"),
+            selector(OUTPUT_COLUMN_NAME, "1")
         ))
     ));
     testComputeBaseAndPostUnnestFilters(
@@ -373,16 +373,64 @@ public void test_nested_filters_unnested_and_topLevel1And3filtersInOR()
         "(unnested-multi-string1 = 3 && (newcol = 2 || multi-string1 = 2 || unnested-multi-string1 = 1))"
     );
   }
-
+  @Test
+  public void test_nested_multiLevel_filters_unnested()
+  {
+    final Filter testQueryFilter = and(ImmutableList.of(
+        selector(OUTPUT_COLUMN_NAME, "3"),
+        or(ImmutableList.of(
+            or(ImmutableList.of(
+                selector("newcol", "2"),
+                selector(COLUMNNAME, "2"),
+                and(ImmutableList.of(
+                    selector("newcol", "3"),
+                    selector(COLUMNNAME, "7")
+                ))
+            )),
+            selector(OUTPUT_COLUMN_NAME, "1")
+        ))
+    ));
+    testComputeBaseAndPostUnnestFilters(
+        testQueryFilter,
+        "unnested-multi-string1 = 3",
+        "(unnested-multi-string1 = 3 && (newcol = 2 || multi-string1 = 2 || (newcol = 3 && multi-string1 = 7) || unnested-multi-string1 = 1))"
+    );
+  }
+  @Test
+  public void test_nested_multiLevel_filters_unnested5Level()
+  {
+    final Filter testQueryFilter = or(ImmutableList.of(
+        selector(OUTPUT_COLUMN_NAME, "3"),
+        or(ImmutableList.of(
+            or(ImmutableList.of(
+                selector("newcol", "2"),
+                selector(COLUMNNAME, "2"),
+                and(ImmutableList.of(
+                    selector("newcol", "3"),
+                    and(ImmutableList.of(
+                        selector(COLUMNNAME, "7"),
+                        selector("newcol_1", "10")
+                    ))
+                ))
+            )),
+            selector(OUTPUT_COLUMN_NAME, "1")
+        ))
+    ));
+    testComputeBaseAndPostUnnestFilters(
+        testQueryFilter,
+        "",
+        "(unnested-multi-string1 = 3 || newcol = 2 || multi-string1 = 2 || (newcol = 3 && multi-string1 = 7 && newcol_1 = 10) || unnested-multi-string1 = 1)"
+    );
+  }
   @Test
   public void test_nested_filters_unnested_and_topLevelORAnd3filtersInOR()
   {
     final Filter testQueryFilter = or(ImmutableList.of(
-        sdf(OUTPUT_COLUMN_NAME, "3", null),
+        selector(OUTPUT_COLUMN_NAME, "3"),
         and(ImmutableList.of(
-            sdf("newcol", "2", null),
-            sdf(COLUMNNAME, "2", null),
-            sdf(OUTPUT_COLUMN_NAME, "1", null)
+            selector("newcol", "2"),
+            selector(COLUMNNAME, "2"),
+            selector(OUTPUT_COLUMN_NAME, "1")
         ))
     ));
     testComputeBaseAndPostUnnestFilters(
@@ -396,15 +444,15 @@ public void test_nested_filters_unnested_and_topLevelORAnd3filtersInOR()
   public void test_nested_filters_unnested_and_topLevelAND3filtersInORWithNestedOrs()
   {
     final Filter testQueryFilter = and(ImmutableList.of(
-        sdf(OUTPUT_COLUMN_NAME, "3", null),
+        selector(OUTPUT_COLUMN_NAME, "3"),
         or(ImmutableList.of(
-            sdf("newcol", "2", null),
-            sdf(COLUMNNAME, "2", null)
+            selector("newcol", "2"),
+            selector(COLUMNNAME, "2")
         )),
         or(ImmutableList.of(
-            sdf("newcol", "4", null),
-            sdf(COLUMNNAME, "8", null),
-            sdf(OUTPUT_COLUMN_NAME, "6", null)
+            selector("newcol", "4"),
+            selector(COLUMNNAME, "8"),
+            selector(OUTPUT_COLUMN_NAME, "6")
         ))
     ));
     testComputeBaseAndPostUnnestFilters(
@@ -418,8 +466,8 @@ public void test_nested_filters_unnested_and_topLevelAND3filtersInORWithNestedOr
   public void test_nested_filters_unnested_and_topLevelAND2sdf()
   {
     final Filter testQueryFilter = and(ImmutableList.of(
-        sdf(OUTPUT_COLUMN_NAME, "3", null),
-        sdf(COLUMNNAME, "2", null)
+        selector(OUTPUT_COLUMN_NAME, "3"),
+        selector(COLUMNNAME, "2")
     ));
     testComputeBaseAndPostUnnestFilters(
         testQueryFilter,
@@ -433,15 +481,15 @@ public void test_pushdown_filters_unnested_dimension_with_unnest_adapters()
     final UnnestStorageAdapter unnestStorageAdapter = new UnnestStorageAdapter(
         new TestStorageAdapter(INCREMENTAL_INDEX),
         new ExpressionVirtualColumn(OUTPUT_COLUMN_NAME, "\"" + COLUMNNAME + "\"", null, ExprMacroTable.nil()),
-         sdfd(OUTPUT_COLUMN_NAME, "1", null)
+        new SelectorDimFilter(OUTPUT_COLUMN_NAME, "1", null)
     );
 
     final VirtualColumn vc = unnestStorageAdapter.getUnnestColumn();
 
     final String inputColumn = unnestStorageAdapter.getUnnestInputIfDirectAccess(vc);
 
     final Filter expectedPushDownFilter =
-        sdf(inputColumn, "1", null);
+        selector(inputColumn, "1");
 
 
     final Sequence<Cursor> cursorSequence = unnestStorageAdapter.makeCursors(
@@ -486,10 +534,9 @@ public void test_pushdown_filters_unnested_dimension_outside()
 
     final String inputColumn = unnestStorageAdapter.getUnnestInputIfDirectAccess(vc);
 
-    final Filter expectedPushDownFilter =
-        sdf(inputColumn, "1", null);
+    final Filter expectedPushDownFilter = selector(inputColumn, "1");
 
-    final Filter queryFilter = sdf(OUTPUT_COLUMN_NAME, "1", null);
+    final Filter queryFilter = selector(OUTPUT_COLUMN_NAME, "1");
     final Sequence<Cursor> cursorSequence = unnestStorageAdapter.makeCursors(
         queryFilter,
         unnestStorageAdapter.getInterval(),

diff --git a/processing/src/test/java/org/apache/druid/segment/filter/FilterTestUtils.java b/processing/src/test/java/org/apache/druid/segment/filter/FilterTestUtils.java
@@ -19,10 +19,7 @@
 
 package org.apache.druid.segment.filter;
 
-import org.apache.druid.query.extraction.ExtractionFn;
-import org.apache.druid.query.filter.DimFilter;
 import org.apache.druid.query.filter.Filter;
-import org.apache.druid.query.filter.SelectorDimFilter;
 
 import java.util.Arrays;
 
@@ -47,19 +44,4 @@ public static SelectorFilter selector(final String fieldName, final String value
   {
     return new SelectorFilter(fieldName, value, null);
   }
-
-  public static Filter sdf(String dimension, String value)
-  {
-    return new SelectorDimFilter(dimension, value, null).toFilter();
-  }
-
-  public static Filter sdf(String dimension, String value, ExtractionFn extractionFn)
-  {
-    return new SelectorDimFilter(dimension, value, extractionFn).toFilter();
-  }
-
-  public static DimFilter sdfd(String dimension, String value, ExtractionFn extractionFn)
-  {
-    return new SelectorDimFilter(dimension, value, extractionFn);
-  }
 }
diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java
@@ -2933,7 +2933,7 @@ public void testUnnestThriceWithFiltersOnDimAndUnnestCol()
                   .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST)
                   .filters(and(
                       NullHandling.sqlCompatible()
-                      ? equality("dimZipf", "27", ColumnType.LONG)
+                      ? numericEquality("dimZipf", "27", ColumnType.LONG)
                       : bound("dimZipf", "27", "27", false, false, null, StringComparators.NUMERIC),
                       equality("j0.unnest", "Baz", ColumnType.STRING)
                   ))
@@ -3023,7 +3023,7 @@ public void testUnnestThriceWithFiltersOnDimAndAllUnnestColumns()
                   .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST)
                   .filters(and(
                       NullHandling.sqlCompatible()
-                      ? equality("dimZipf", "27", ColumnType.LONG)
+                      ? numericEquality("dimZipf", "27", ColumnType.LONG)
                       : bound("dimZipf", "27", "27", false, false, null, StringComparators.NUMERIC),
                       equality("j0.unnest", "Baz", ColumnType.STRING)
                   ))