This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new e84f3f5667 ensure inverse And/OrFilterOperator implementations match
the query (#13199)
e84f3f5667 is described below
commit e84f3f5667d5e78db60f5b09de61e89dae532c7e
Author: Christopher Peck <[email protected]>
AuthorDate: Fri May 24 20:53:09 2024 -0700
ensure inverse And/OrFilterOperator implementations match the query (#13199)
---
.../core/operator/filter/AndFilterOperator.java | 29 ++++++--
.../core/operator/filter/BaseFilterOperator.java | 18 +++--
.../core/operator/filter/OrFilterOperator.java | 30 ++++++--
.../core/operator/filter/TestFilterOperator.java | 11 +++
.../operator/filter/AndFilterOperatorTest.java | 42 ++++++++++++
.../operator/filter/BaseFilterOperatorTest.java | 79 ++++++++++++++++++++++
.../core/operator/filter/OrFilterOperatorTest.java | 43 ++++++++++++
7 files changed, 237 insertions(+), 15 deletions(-)
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 93c79871fa..94545d8889 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 @@ public class AndFilterOperator extends BaseFilterOperator {
protected BlockDocIdSet getFalses() {
List<BlockDocIdSet> 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 75140d73c4..7acbd6ed7c 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.common.BlockDocIdSet;
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 @@ public abstract class BaseFilterOperator extends
BaseOperator<FilterBlock> {
* @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 5b99d58353..9f8477f96d 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 @@ public class OrFilterOperator extends BaseFilterOperator {
protected BlockDocIdSet getFalses() {
List<BlockDocIdSet> 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 3aff520c87..270e4cee39 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 java.util.List;
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 class TestFilterOperator extends BaseFilterOperator {
@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 fa7320969a..230a3ead55 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 class AndFilterOperatorTest {
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 class AndFilterOperatorTest {
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 0000000000..4d9ecf5cc4
--- /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 6e634351e9..b1c4aac1f8 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 @@ package org.apache.pinot.core.operator.filter;
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 class OrFilterOperatorTest {
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 class OrFilterOperatorTest {
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));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
