From 47f141d19f4fe6d4c356e7c490367f612d77cb38 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Wed, 22 Jan 2025 17:30:32 -0800 Subject: [PATCH] fix issue with OR filter vector value matcher for proper 3VL behavior when inverted --- .../apache/druid/segment/filter/OrFilter.java | 2 +- .../druid/segment/filter/OrFilterTest.java | 96 +++++++++++++++++-- 2 files changed, 90 insertions(+), 8 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 e8bdce85c9bf..8363fd29c9f7 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 ReadableVectorMatch match(final ReadableVectorMatch mask, boolean include } 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/OrFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/OrFilterTest.java index 54689d30d9de..60da4f0c12a8 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 @@ 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.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 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 void testTwoFilterFirstMatchesNoneSecondMatchesAll() ), 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 void testTwoFilterFirstMatchesSomeSecondMatchesNone() ), 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 void testTwoFilterFirstMatchesNoneSecondMatchesNone() ), 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 void testThreeFilterFirstMatchesSomeSecondLiterallyTrueThirdMatchesNone() ), 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