diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/AndFilterOperator.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/AndFilterOperator.java index 93c79871fa13..94545d888906 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/AndFilterOperator.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/AndFilterOperator.java @@ -19,13 +19,16 @@ package org.apache.pinot.core.operator.filter; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; import javax.annotation.Nullable; import org.apache.pinot.core.common.BlockDocIdSet; import org.apache.pinot.core.common.Operator; import org.apache.pinot.core.operator.docidsets.AndDocIdSet; +import org.apache.pinot.core.operator.docidsets.EmptyDocIdSet; import org.apache.pinot.core.operator.docidsets.MatchAllDocIdSet; +import org.apache.pinot.core.operator.docidsets.NotDocIdSet; import org.apache.pinot.core.operator.docidsets.OrDocIdSet; import org.apache.pinot.spi.trace.Tracing; import org.roaringbitmap.buffer.BufferFastAggregation; @@ -59,13 +62,29 @@ protected BlockDocIdSet getTrues() { protected BlockDocIdSet getFalses() { List blockDocIdSets = new ArrayList<>(_filterOperators.size()); for (BaseFilterOperator filterOperator : _filterOperators) { - if (filterOperator.isResultEmpty()) { - blockDocIdSets.add(new MatchAllDocIdSet(_numDocs)); - } else { - blockDocIdSets.add(filterOperator.getFalses()); + BlockDocIdSet trues = filterOperator.getTrues(); + if (trues instanceof EmptyDocIdSet) { + return new MatchAllDocIdSet(_numDocs); } + if (trues instanceof MatchAllDocIdSet) { + continue; + } + if (_nullHandlingEnabled) { + BlockDocIdSet nulls = filterOperator.getNulls(); + if (!(nulls instanceof EmptyDocIdSet)) { + blockDocIdSets.add(new OrDocIdSet(Arrays.asList(trues, nulls), _numDocs)); + continue; + } + } + blockDocIdSets.add(trues); + } + if (blockDocIdSets.isEmpty()) { + return EmptyDocIdSet.getInstance(); + } + if (blockDocIdSets.size() == 1) { + return new NotDocIdSet(blockDocIdSets.get(0), _numDocs); } - return new OrDocIdSet(blockDocIdSets, _numDocs); + return new NotDocIdSet(new AndDocIdSet(blockDocIdSets, _queryOptions), _numDocs); } @Override diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BaseFilterOperator.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BaseFilterOperator.java index 75140d73c458..7acbd6ed7c95 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BaseFilterOperator.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BaseFilterOperator.java @@ -23,6 +23,7 @@ import org.apache.pinot.core.operator.BaseOperator; import org.apache.pinot.core.operator.blocks.FilterBlock; import org.apache.pinot.core.operator.docidsets.EmptyDocIdSet; +import org.apache.pinot.core.operator.docidsets.MatchAllDocIdSet; import org.apache.pinot.core.operator.docidsets.NotDocIdSet; import org.apache.pinot.core.operator.docidsets.OrDocIdSet; @@ -102,11 +103,20 @@ protected BlockDocIdSet getNulls() { * @return document IDs in which the predicate evaluates to false. */ protected BlockDocIdSet getFalses() { + BlockDocIdSet trues = getTrues(); + if (trues instanceof MatchAllDocIdSet) { + return EmptyDocIdSet.getInstance(); + } if (_nullHandlingEnabled) { - return new NotDocIdSet(new OrDocIdSet(Arrays.asList(getTrues(), getNulls()), _numDocs), - _numDocs); - } else { - return new NotDocIdSet(getTrues(), _numDocs); + BlockDocIdSet nulls = getNulls(); + if (!(nulls instanceof EmptyDocIdSet)) { + return new NotDocIdSet(new OrDocIdSet(Arrays.asList(trues, nulls), _numDocs), + _numDocs); + } + } + if (trues instanceof EmptyDocIdSet) { + return new MatchAllDocIdSet(_numDocs); } + return new NotDocIdSet(trues, _numDocs); } } diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/OrFilterOperator.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/OrFilterOperator.java index 5b99d583531a..9f8477f96d5f 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/OrFilterOperator.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/OrFilterOperator.java @@ -19,13 +19,15 @@ package org.apache.pinot.core.operator.filter; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; import javax.annotation.Nullable; import org.apache.pinot.core.common.BlockDocIdSet; import org.apache.pinot.core.common.Operator; -import org.apache.pinot.core.operator.docidsets.AndDocIdSet; +import org.apache.pinot.core.operator.docidsets.EmptyDocIdSet; import org.apache.pinot.core.operator.docidsets.MatchAllDocIdSet; +import org.apache.pinot.core.operator.docidsets.NotDocIdSet; import org.apache.pinot.core.operator.docidsets.OrDocIdSet; import org.apache.pinot.spi.trace.Tracing; import org.roaringbitmap.buffer.BufferFastAggregation; @@ -59,13 +61,29 @@ protected BlockDocIdSet getTrues() { protected BlockDocIdSet getFalses() { List blockDocIdSets = new ArrayList<>(_filterOperators.size()); for (BaseFilterOperator filterOperator : _filterOperators) { - if (filterOperator.isResultEmpty()) { - blockDocIdSets.add(new MatchAllDocIdSet(_numDocs)); - } else { - blockDocIdSets.add(filterOperator.getFalses()); + BlockDocIdSet trues = filterOperator.getTrues(); + if (trues instanceof MatchAllDocIdSet) { + return EmptyDocIdSet.getInstance(); } + if (trues instanceof EmptyDocIdSet) { + continue; + } + if (_nullHandlingEnabled) { + BlockDocIdSet nulls = filterOperator.getNulls(); + if (!(nulls instanceof EmptyDocIdSet)) { + blockDocIdSets.add(new OrDocIdSet(Arrays.asList(trues, nulls), _numDocs)); + continue; + } + } + blockDocIdSets.add(trues); + } + if (blockDocIdSets.isEmpty()) { + return new MatchAllDocIdSet(_numDocs); + } + if (blockDocIdSets.size() == 1) { + return new NotDocIdSet(blockDocIdSets.get(0), _numDocs); } - return new AndDocIdSet(blockDocIdSets, _queryOptions); + return new NotDocIdSet(new OrDocIdSet(blockDocIdSets, _numDocs), _numDocs); } @Override diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/TestFilterOperator.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/TestFilterOperator.java index 3aff520c87ae..270e4cee39f3 100644 --- a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/TestFilterOperator.java +++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/TestFilterOperator.java @@ -23,6 +23,8 @@ import org.apache.pinot.core.common.BlockDocIdIterator; import org.apache.pinot.core.common.BlockDocIdSet; import org.apache.pinot.core.common.Operator; +import org.apache.pinot.core.operator.docidsets.EmptyDocIdSet; +import org.apache.pinot.core.operator.docidsets.MatchAllDocIdSet; import org.apache.pinot.segment.spi.Constants; @@ -56,11 +58,20 @@ public List getChildOperators() { @Override protected BlockDocIdSet getTrues() { + if (_trueDocIds.length == _numDocs) { + return new MatchAllDocIdSet(_numDocs); + } + if (_trueDocIds.length == 0) { + return EmptyDocIdSet.getInstance(); + } return new TestBlockDocIdSet(_trueDocIds); } @Override protected BlockDocIdSet getNulls() { + if (_nullDocIds.length == 0) { + return EmptyDocIdSet.getInstance(); + } return new TestBlockDocIdSet(_nullDocIds); } diff --git a/pinot-core/src/test/java/org/apache/pinot/core/operator/filter/AndFilterOperatorTest.java b/pinot-core/src/test/java/org/apache/pinot/core/operator/filter/AndFilterOperatorTest.java index fa7320969ab3..230a3ead557e 100644 --- a/pinot-core/src/test/java/org/apache/pinot/core/operator/filter/AndFilterOperatorTest.java +++ b/pinot-core/src/test/java/org/apache/pinot/core/operator/filter/AndFilterOperatorTest.java @@ -178,6 +178,22 @@ public void testAndWithNull() { Assert.assertEquals(TestUtils.getDocIds(andFilterOperator.getFalses()), ImmutableList.of(0, 7, 8, 9)); } + @Test + public void testAndWithNullHandlingDisabled() { + int numDocs = 4; + int[] docIds1 = new int[]{0, 3}; + int[] docIds2 = new int[]{0, 1}; + int[] nullDocIds1 = new int[]{}; + int[] nullDocIds2 = new int[]{}; + + AndFilterOperator andFilterOperator = new AndFilterOperator( + Arrays.asList(new TestFilterOperator(docIds1, nullDocIds1, numDocs), + new TestFilterOperator(docIds2, nullDocIds2, numDocs)), null, numDocs, false); + + Assert.assertEquals(TestUtils.getDocIds(andFilterOperator.getTrues()), ImmutableList.of(0)); + Assert.assertEquals(TestUtils.getDocIds(andFilterOperator.getFalses()), ImmutableList.of(1, 2, 3)); + } + @Test public void testAndWithNullOneFilterIsEmpty() { int numDocs = 10; @@ -192,4 +208,30 @@ public void testAndWithNullOneFilterIsEmpty() { Assert.assertEquals(TestUtils.getDocIds(andFilterOperator.getFalses()), ImmutableList.of(0, 1, 2, 3, 4, 5, 6, 7, 8, 9)); } + + @Test + public void testAndWithNullOneFilterMatchesAll() { + int numDocs = 10; + int[] docIds1 = new int[]{1, 2, 3}; + int[] nullDocIds1 = new int[]{4, 5, 6}; + + AndFilterOperator andFilterOperator = new AndFilterOperator( + Arrays.asList(new TestFilterOperator(docIds1, nullDocIds1, numDocs), new MatchAllFilterOperator(numDocs)), null, + numDocs, true); + + Assert.assertEquals(TestUtils.getDocIds(andFilterOperator.getTrues()), ImmutableList.of(1, 2, 3)); + Assert.assertEquals(TestUtils.getDocIds(andFilterOperator.getFalses()), ImmutableList.of(0, 7, 8, 9)); + } + + @Test + public void testAndWithAllMatchesAll() { + int numDocs = 10; + AndFilterOperator andFilterOperator = + new AndFilterOperator(Arrays.asList(new MatchAllFilterOperator(numDocs), new MatchAllFilterOperator(numDocs)), + null, numDocs, true); + + Assert.assertEquals(TestUtils.getDocIds(andFilterOperator.getTrues()), + ImmutableList.of(0, 1, 2, 3, 4, 5, 6, 7, 8, 9)); + Assert.assertEquals(TestUtils.getDocIds(andFilterOperator.getFalses()), Collections.emptyList()); + } } diff --git a/pinot-core/src/test/java/org/apache/pinot/core/operator/filter/BaseFilterOperatorTest.java b/pinot-core/src/test/java/org/apache/pinot/core/operator/filter/BaseFilterOperatorTest.java new file mode 100644 index 000000000000..4d9ecf5cc4b7 --- /dev/null +++ b/pinot-core/src/test/java/org/apache/pinot/core/operator/filter/BaseFilterOperatorTest.java @@ -0,0 +1,79 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.core.operator.filter; + +import com.google.common.collect.ImmutableList; +import java.util.Collections; +import org.testng.Assert; +import org.testng.annotations.Test; + + +public class BaseFilterOperatorTest { + + @Test + public void testBaseWithFalses() { + int numDocs = 10; + int[] docIds = new int[]{0, 1, 2, 3}; + TestFilterOperator testFilterOperator = new TestFilterOperator(docIds, numDocs); + + Assert.assertEquals(TestUtils.getDocIds(testFilterOperator.getTrues()), ImmutableList.of(0, 1, 2, 3)); + Assert.assertEquals(TestUtils.getDocIds(testFilterOperator.getFalses()), ImmutableList.of(4, 5, 6, 7, 8, 9)); + } + + @Test + public void testBaseWithNullHandling() { + int numDocs = 10; + int[] docIds = new int[]{0, 1, 2, 3}; + int[] nullDocIds = new int[]{4, 5, 6, 7, 8, 9}; + TestFilterOperator testFilterOperator = new TestFilterOperator(docIds, nullDocIds, numDocs); + + Assert.assertEquals(TestUtils.getDocIds(testFilterOperator.getTrues()), ImmutableList.of(0, 1, 2, 3)); + Assert.assertEquals(TestUtils.getDocIds(testFilterOperator.getFalses()), Collections.emptyList()); + } + + @Test + public void testBaseWithAllTrue() { + int numDocs = 10; + int[] docIds = new int[]{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; + TestFilterOperator testFilterOperator = new TestFilterOperator(docIds, numDocs); + Assert.assertEquals(TestUtils.getDocIds(testFilterOperator.getTrues()), + ImmutableList.of(0, 1, 2, 3, 4, 5, 6, 7, 8, 9)); + Assert.assertEquals(TestUtils.getDocIds(testFilterOperator.getFalses()), Collections.emptyList()); + } + + @Test + public void testBaseWithAllFalse() { + int numDocs = 10; + int[] docIds = new int[]{}; + TestFilterOperator testFilterOperator = new TestFilterOperator(docIds, numDocs); + Assert.assertEquals(TestUtils.getDocIds(testFilterOperator.getTrues()), Collections.emptyList()); + Assert.assertEquals(TestUtils.getDocIds(testFilterOperator.getFalses()), + ImmutableList.of(0, 1, 2, 3, 4, 5, 6, 7, 8, 9)); + } + + @Test + public void testBaseWithAllNulls() { + int numDocs = 10; + int[] docIds = new int[]{}; + int[] nullDocIds = new int[]{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; + TestFilterOperator testFilterOperator = new TestFilterOperator(docIds, nullDocIds, numDocs); + Assert.assertEquals(TestUtils.getDocIds(testFilterOperator.getTrues()), Collections.emptyList()); + Assert.assertEquals(TestUtils.getDocIds(testFilterOperator.getFalses()), Collections.emptyList()); + } +} diff --git a/pinot-core/src/test/java/org/apache/pinot/core/operator/filter/OrFilterOperatorTest.java b/pinot-core/src/test/java/org/apache/pinot/core/operator/filter/OrFilterOperatorTest.java index 6e634351e94b..b1c4aac1f803 100644 --- a/pinot-core/src/test/java/org/apache/pinot/core/operator/filter/OrFilterOperatorTest.java +++ b/pinot-core/src/test/java/org/apache/pinot/core/operator/filter/OrFilterOperatorTest.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableList; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.TreeSet; @@ -125,6 +126,22 @@ public void testOrWithNull() { Assert.assertEquals(TestUtils.getDocIds(orFilterOperator.getFalses()), ImmutableList.of(8, 9)); } + @Test + public void testOrWithNullHandlingDisabled() { + int numDocs = 10; + int[] docIds1 = new int[]{1, 2, 3}; + int[] docIds2 = new int[]{0, 1, 2}; + int[] nullDocIds1 = new int[]{}; + int[] nullDocIds2 = new int[]{}; + + OrFilterOperator orFilterOperator = new OrFilterOperator( + Arrays.asList(new TestFilterOperator(docIds1, nullDocIds1, numDocs), + new TestFilterOperator(docIds2, nullDocIds2, numDocs)), null, numDocs, false); + + Assert.assertEquals(TestUtils.getDocIds(orFilterOperator.getTrues()), ImmutableList.of(0, 1, 2, 3)); + Assert.assertEquals(TestUtils.getDocIds(orFilterOperator.getFalses()), ImmutableList.of(4, 5, 6, 7, 8, 9)); + } + @Test public void testOrWithNullOneFilterIsEmpty() { int numDocs = 10; @@ -138,4 +155,30 @@ public void testOrWithNullOneFilterIsEmpty() { Assert.assertEquals(TestUtils.getDocIds(orFilterOperator.getTrues()), Arrays.asList(1, 2, 3)); Assert.assertEquals(TestUtils.getDocIds(orFilterOperator.getFalses()), Arrays.asList(0, 7, 8, 9)); } + + @Test + public void testOrWithNullOneFilterMatchesAll() { + int numDocs = 10; + int[] docIds1 = new int[]{1, 2, 3}; + int[] nullDocIds1 = new int[]{4, 5, 6}; + + OrFilterOperator orFilterOperator = new OrFilterOperator( + Arrays.asList(new TestFilterOperator(docIds1, nullDocIds1, numDocs), new MatchAllFilterOperator(numDocs)), null, + numDocs, true); + + Assert.assertEquals(TestUtils.getDocIds(orFilterOperator.getTrues()), Arrays.asList(0, 1, 2, 3, 4, 5, 6, 7, 8, 9)); + Assert.assertEquals(TestUtils.getDocIds(orFilterOperator.getFalses()), Collections.emptyList()); + } + + @Test + public void testOrWithAllEmpty() { + int numDocs = 10; + + OrFilterOperator orFilterOperator = + new OrFilterOperator(Arrays.asList(EmptyFilterOperator.getInstance(), EmptyFilterOperator.getInstance()), null, + numDocs, true); + + Assert.assertEquals(TestUtils.getDocIds(orFilterOperator.getTrues()), Collections.emptyList()); + Assert.assertEquals(TestUtils.getDocIds(orFilterOperator.getFalses()), Arrays.asList(0, 1, 2, 3, 4, 5, 6, 7, 8, 9)); + } }