clintropolis commented on code in PR #14587:
URL: https://github.com/apache/druid/pull/14587#discussion_r1286465236


##########
processing/src/test/java/org/apache/druid/segment/UnnestStorageAdapterTest.java:
##########
@@ -300,7 +306,175 @@ public void 
test_pushdown_or_filters_unnested_and_original_dimension_with_unnest
       return null;
     });
   }
+  @Test
+  public void 
test_nested_filters_unnested_and_original_dimension_with_unnest_adapters()
+  {
+    final UnnestStorageAdapter unnestStorageAdapter = new UnnestStorageAdapter(
+        new TestStorageAdapter(INCREMENTAL_INDEX),
+        new ExpressionVirtualColumn(OUTPUT_COLUMN_NAME, "\"" + COLUMNNAME + 
"\"", null, ExprMacroTable.nil()),
+        null
+    );
+
+    final VirtualColumn vc = unnestStorageAdapter.getUnnestColumn();
+
+    final String inputColumn = 
unnestStorageAdapter.getUnnestInputIfDirectAccess(vc);
+
+    final OrFilter baseFilter = new OrFilter(ImmutableList.of(
+        selector(OUTPUT_COLUMN_NAME, "1"),
+        new AndFilter(ImmutableList.of(
+            selector(inputColumn, "2"),
+            selector(OUTPUT_COLUMN_NAME, "10")
+        ))
+    ));
+
+    final OrFilter expectedPushDownFilter = new OrFilter(ImmutableList.of(
+        selector(inputColumn, "1"),
+        new AndFilter(ImmutableList.of(
+            selector(inputColumn, "2"),
+            selector(inputColumn, "10")
+        ))
+    ));
+
+    final Sequence<Cursor> cursorSequence = unnestStorageAdapter.makeCursors(
+        baseFilter,
+        unnestStorageAdapter.getInterval(),
+        VirtualColumns.EMPTY,
+        Granularities.ALL,
+        false,
+        null
+    );
+
+    final TestStorageAdapter base = (TestStorageAdapter) 
unnestStorageAdapter.getBaseAdapter();
+    final Filter pushDownFilter = base.getPushDownFilter();
+
+    Assert.assertEquals(expectedPushDownFilter, pushDownFilter);
+    cursorSequence.accumulate(null, (accumulated, cursor) -> {
+      Assert.assertEquals(cursor.getClass(), PostJoinCursor.class);
+      final Filter postFilter = ((PostJoinCursor) cursor).getPostJoinFilter();
+      // OR-case so base filter should match the postJoinFilter
+      Assert.assertEquals(baseFilter, postFilter);
+      return null;
+    });
+  }
+  @Test
+  public void test_nested_filters_unnested_and_topLevel1And3filtersInOR()
+  {
+    final Filter testQueryFilter = and(ImmutableList.of(
+        selector(OUTPUT_COLUMN_NAME, "3"),
+        or(ImmutableList.of(
+            selector("newcol", "2"),
+            selector(COLUMNNAME, "2"),
+            selector(OUTPUT_COLUMN_NAME, "1")
+        ))
+    ));
+    testComputeBaseAndPostUnnestFilters(
+        testQueryFilter,
+        "multi-string1 = 3",
+        "(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,
+        "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,
+        "(multi-string1 = 3 || newcol = 2 || multi-string1 = 2 || (newcol = 3 
&& multi-string1 = 7 && newcol_1 = 10) || multi-string1 = 1)",
+        "(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(
+        selector(OUTPUT_COLUMN_NAME, "3"),
+        and(ImmutableList.of(
+            selector("newcol", "2"),
+            selector(COLUMNNAME, "2"),
+            selector(OUTPUT_COLUMN_NAME, "1")
+        ))
+    ));
+    testComputeBaseAndPostUnnestFilters(
+        testQueryFilter,
+        "(multi-string1 = 3 || (newcol = 2 && multi-string1 = 2 && 
multi-string1 = 1))",
+        "(unnested-multi-string1 = 3 || (newcol = 2 && multi-string1 = 2 && 
unnested-multi-string1 = 1))"
+    );
+  }
 
+  @Test
+  public void 
test_nested_filters_unnested_and_topLevelAND3filtersInORWithNestedOrs()
+  {
+    final Filter testQueryFilter = and(ImmutableList.of(
+        selector(OUTPUT_COLUMN_NAME, "3"),
+        or(ImmutableList.of(
+            selector("newcol", "2"),
+            selector(COLUMNNAME, "2")
+        )),
+        or(ImmutableList.of(
+            selector("newcol", "4"),
+            selector(COLUMNNAME, "8"),
+            selector(OUTPUT_COLUMN_NAME, "6")
+        ))
+    ));
+    testComputeBaseAndPostUnnestFilters(
+        testQueryFilter,
+        "multi-string1 = 3",
+        "(unnested-multi-string1 = 3 && (newcol = 2 || multi-string1 = 2) && 
(newcol = 4 || multi-string1 = 8 || unnested-multi-string1 = 6))"
+    );
+  }
+
+  @Test
+  public void test_nested_filters_unnested_and_topLevelAND2sdf()
+  {
+    final Filter testQueryFilter = and(ImmutableList.of(
+        selector(OUTPUT_COLUMN_NAME, "3"),
+        selector(COLUMNNAME, "2")
+    ));
+    testComputeBaseAndPostUnnestFilters(
+        testQueryFilter,
+        "multi-string1 = 3",

Review Comment:
   why is it that only the filter on the unnest column pushed down instead of 
the whole and? same question for other test cases involving an AND? I think I 
would expect that in the case of AND especially, everything which can be pushed 
down is pushed down. This is what the comments in 
`computeBaseAndPostUnnestFilters` indicates should happen..
   
   In the case of an AND filter, I think that everything can be pushed down 
except for the case where the unnest output column is not a simple direct 
access to a MVD. Since it is an AND though, I think all of the other clauses 
could still be pushed down in that case, leaving only the unnest filter which 
could not be rewritten as a post filter.
   
   But in the case of OR filters, I think its either all or nothing can be 
pushed down. So if there is a filter on the unnest output column where the 
virtual column is not a simple direct column access to a multi-value string, 
then nothing can be pushed down, else if the unnest column filters can be 
rewritten because they are simple mvd direct access, everything can be pushed 
down.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to