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]