This is an automated email from the ASF dual-hosted git repository.

cwylie pushed a commit to branch 32.0.0
in repository https://gitbox.apache.org/repos/asf/druid.git


The following commit(s) were added to refs/heads/32.0.0 by this push:
     new 3da3ff0901e fix issue with OR filter vector value matcher for proper 
3VL behavior when inverted (#17655) (#17656)
3da3ff0901e is described below

commit 3da3ff0901ea387445cd748aaabb3341e4f09dce
Author: Clint Wylie <[email protected]>
AuthorDate: Thu Jan 23 11:48:50 2025 -0800

    fix issue with OR filter vector value matcher for proper 3VL behavior when 
inverted (#17655) (#17656)
    
    Fixes a bug with the OR filters vectorized value matcher that causes vector 
engines processing filter an OR filter under a NOT filter (ex. of the form 
NOT(x OR y)) to produce incorrect results for null values matched.
    
    This bug is due to incorrectly hard coding the includeUnknown parameter as 
false for OR filter child vector matchers after the initial filter clause 
instead of passing it through the function parameter to the underlying matchers.
---
 .../org/apache/druid/segment/filter/OrFilter.java  |  2 +-
 .../apache/druid/segment/filter/AndFilterTest.java | 46 +++++++++--
 .../apache/druid/segment/filter/OrFilterTest.java  | 96 ++++++++++++++++++++--
 3 files changed, 129 insertions(+), 15 deletions(-)

diff --git 
a/processing/src/main/java/org/apache/druid/segment/filter/OrFilter.java 
b/processing/src/main/java/org/apache/druid/segment/filter/OrFilter.java
index e8bdce85c9b..8363fd29c9f 100644
--- a/processing/src/main/java/org/apache/druid/segment/filter/OrFilter.java
+++ b/processing/src/main/java/org/apache/druid/segment/filter/OrFilter.java
@@ -144,7 +144,7 @@ public class OrFilter implements BooleanFilter
           }
 
           currentMask.removeAll(currentMatch);
-          currentMatch = baseMatchers[i].match(currentMask, false);
+          currentMatch = baseMatchers[i].match(currentMask, includeUnknown);
           retVal.addAll(currentMatch, scratch);
 
           if (currentMatch == currentMask) {
diff --git 
a/processing/src/test/java/org/apache/druid/segment/filter/AndFilterTest.java 
b/processing/src/test/java/org/apache/druid/segment/filter/AndFilterTest.java
index 35185967f66..b60fa4dbe55 100644
--- 
a/processing/src/test/java/org/apache/druid/segment/filter/AndFilterTest.java
+++ 
b/processing/src/test/java/org/apache/druid/segment/filter/AndFilterTest.java
@@ -21,7 +21,6 @@ package org.apache.druid.segment.filter;
 
 import com.google.common.base.Function;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import nl.jqno.equalsverifier.EqualsVerifier;
 import org.apache.druid.data.input.InputRow;
 import org.apache.druid.data.input.impl.DimensionsSpec;
@@ -34,8 +33,11 @@ import org.apache.druid.java.util.common.Pair;
 import org.apache.druid.query.filter.AndDimFilter;
 import org.apache.druid.query.filter.NotDimFilter;
 import org.apache.druid.query.filter.SelectorDimFilter;
+import org.apache.druid.query.filter.TrueDimFilter;
 import org.apache.druid.segment.CursorFactory;
 import org.apache.druid.segment.IndexBuilder;
+import org.apache.druid.segment.column.ColumnType;
+import org.apache.druid.segment.column.RowSignature;
 import org.junit.AfterClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -57,13 +59,19 @@ public class AndFilterTest extends BaseFilterTest
       )
   );
 
+  private static final RowSignature ROW_SIGNATURE = RowSignature.builder()
+                                                                .add("dim0", 
ColumnType.STRING)
+                                                                .add("dim1", 
ColumnType.STRING)
+                                                                .add("dim2", 
ColumnType.STRING)
+                                                                .build();
+
   private static final List<InputRow> ROWS = ImmutableList.of(
-      PARSER.parseBatch(ImmutableMap.of("dim0", "0", "dim1", "0")).get(0),
-      PARSER.parseBatch(ImmutableMap.of("dim0", "1", "dim1", "0")).get(0),
-      PARSER.parseBatch(ImmutableMap.of("dim0", "2", "dim1", "0")).get(0),
-      PARSER.parseBatch(ImmutableMap.of("dim0", "3", "dim1", "0")).get(0),
-      PARSER.parseBatch(ImmutableMap.of("dim0", "4", "dim1", "0")).get(0),
-      PARSER.parseBatch(ImmutableMap.of("dim0", "5", "dim1", "0")).get(0)
+      makeSchemaRow(PARSER, ROW_SIGNATURE, "0", "0", "a"),
+      makeSchemaRow(PARSER, ROW_SIGNATURE, "1", "0", null),
+      makeSchemaRow(PARSER, ROW_SIGNATURE, "2", "0", "b"),
+      makeSchemaRow(PARSER, ROW_SIGNATURE, "3", "0", null),
+      makeSchemaRow(PARSER, ROW_SIGNATURE, "4", "0", "c"),
+      makeSchemaRow(PARSER, ROW_SIGNATURE, "5", "0", null)
   );
 
   public AndFilterTest(
@@ -177,6 +185,30 @@ public class AndFilterTest extends BaseFilterTest
     );
   }
 
+  @Test
+  public void testNotAndWithNulls()
+  {
+    assertFilterMatches(
+        new AndDimFilter(
+            ImmutableList.of(
+                TrueDimFilter.instance(),
+                new SelectorDimFilter("dim2", "c", null)
+            )
+        ),
+        ImmutableList.of("4")
+    );
+    assertFilterMatches(
+        new NotDimFilter(
+            new AndDimFilter(ImmutableList.of(
+                TrueDimFilter.instance(),
+                new SelectorDimFilter("dim2", "c", null)
+            )
+            )
+        ),
+        ImmutableList.of("0", "2")
+    );
+  }
+
   @Test
   public void test_equals()
   {
diff --git 
a/processing/src/test/java/org/apache/druid/segment/filter/OrFilterTest.java 
b/processing/src/test/java/org/apache/druid/segment/filter/OrFilterTest.java
index 54689d30d9d..60da4f0c12a 100644
--- a/processing/src/test/java/org/apache/druid/segment/filter/OrFilterTest.java
+++ b/processing/src/test/java/org/apache/druid/segment/filter/OrFilterTest.java
@@ -21,7 +21,6 @@ package org.apache.druid.segment.filter;
 
 import com.google.common.base.Function;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import nl.jqno.equalsverifier.EqualsVerifier;
 import org.apache.druid.data.input.InputRow;
@@ -40,6 +39,8 @@ import org.apache.druid.query.filter.SelectorDimFilter;
 import org.apache.druid.query.filter.TrueDimFilter;
 import org.apache.druid.segment.CursorFactory;
 import org.apache.druid.segment.IndexBuilder;
+import org.apache.druid.segment.column.ColumnType;
+import org.apache.druid.segment.column.RowSignature;
 import org.junit.AfterClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -60,14 +61,19 @@ public class OrFilterTest extends BaseFilterTest
           DimensionsSpec.EMPTY
       )
   );
+  private static final RowSignature ROW_SIGNATURE = RowSignature.builder()
+                                                                .add("dim0", 
ColumnType.STRING)
+                                                                .add("dim1", 
ColumnType.STRING)
+                                                                .add("dim2", 
ColumnType.STRING)
+                                                                .build();
 
   private static final List<InputRow> ROWS = ImmutableList.of(
-      PARSER.parseBatch(ImmutableMap.of("dim0", "0", "dim1", "0")).get(0),
-      PARSER.parseBatch(ImmutableMap.of("dim0", "1", "dim1", "0")).get(0),
-      PARSER.parseBatch(ImmutableMap.of("dim0", "2", "dim1", "0")).get(0),
-      PARSER.parseBatch(ImmutableMap.of("dim0", "3", "dim1", "0")).get(0),
-      PARSER.parseBatch(ImmutableMap.of("dim0", "4", "dim1", "0")).get(0),
-      PARSER.parseBatch(ImmutableMap.of("dim0", "5", "dim1", "0")).get(0)
+      makeSchemaRow(PARSER, ROW_SIGNATURE, "0", "0", "a"),
+      makeSchemaRow(PARSER, ROW_SIGNATURE, "1", "0", null),
+      makeSchemaRow(PARSER, ROW_SIGNATURE, "2", "0", "b"),
+      makeSchemaRow(PARSER, ROW_SIGNATURE, "3", "0", null),
+      makeSchemaRow(PARSER, ROW_SIGNATURE, "4", "0", "c"),
+      makeSchemaRow(PARSER, ROW_SIGNATURE, "5", "0", null)
   );
 
   public OrFilterTest(
@@ -152,6 +158,17 @@ public class OrFilterTest extends BaseFilterTest
         ),
         ImmutableList.of("0", "1", "2", "3", "4", "5")
     );
+    assertFilterMatches(
+        NotDimFilter.of(
+          new OrDimFilter(
+              ImmutableList.of(
+                  new SelectorDimFilter("dim0", "7", null),
+                  new SelectorDimFilter("dim1", "0", null)
+              )
+          )
+        ),
+        ImmutableList.of()
+    );
   }
 
   @Test
@@ -208,6 +225,17 @@ public class OrFilterTest extends BaseFilterTest
         ),
         ImmutableList.of("3")
     );
+    assertFilterMatches(
+        NotDimFilter.of(
+            new OrDimFilter(
+                ImmutableList.of(
+                    new SelectorDimFilter("dim0", "3", null),
+                    new SelectorDimFilter("dim1", "7", null)
+                )
+            )
+        ),
+        ImmutableList.of("0", "1", "2", "4", "5")
+    );
   }
 
   @Test
@@ -236,6 +264,18 @@ public class OrFilterTest extends BaseFilterTest
         ),
         ImmutableList.of()
     );
+
+    assertFilterMatches(
+        NotDimFilter.of(
+            new OrDimFilter(
+                ImmutableList.of(
+                    new SelectorDimFilter("dim1", "7", null),
+                    new SelectorDimFilter("dim0", "7", null)
+                )
+            )
+        ),
+        ImmutableList.of("0", "1", "2", "3", "4", "5")
+    );
   }
 
   @Test
@@ -254,6 +294,48 @@ public class OrFilterTest extends BaseFilterTest
         ),
         ImmutableList.of("0", "1", "2", "4", "5")
     );
+    assertFilterMatches(
+        NotDimFilter.of(
+          new AndDimFilter(
+              new InDimFilter("dim0", ImmutableSet.of("0", "1", "2", "4", 
"5")),
+              new OrDimFilter(
+                  ImmutableList.of(
+                      new SelectorDimFilter("dim0", "4", null),
+                      TrueDimFilter.instance(),
+                      new SelectorDimFilter("dim0", "7", null)
+                  )
+              )
+          )
+        ),
+        ImmutableList.of("3")
+    );
+  }
+
+  @Test
+  public void testNotOrWithNulls()
+  {
+    assertFilterMatches(
+        new OrDimFilter(
+            ImmutableList.of(
+                new SelectorDimFilter("dim0", "3", null),
+                new SelectorDimFilter("dim2", "c", null)
+            )
+        ),
+        ImmutableList.of("3", "4")
+    );
+
+    assertFilterMatches(
+        NotDimFilter.of(
+          new OrDimFilter(
+              ImmutableList.of(
+                  new SelectorDimFilter("dim0", "3", null),
+                  new SelectorDimFilter("dim2", "c", null)
+              )
+          )
+        ),
+        // dim2 null rows don't match when inverted because unknown
+        ImmutableList.of("0", "2")
+    );
   }
 
   @Test


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

Reply via email to