You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/02/07 10:21:51 UTC
[GitHub] [pinot] atris opened a new pull request #8148: Implement NOT Operator
atris opened a new pull request #8148:
URL: https://github.com/apache/pinot/pull/8148
This PR implements the NOT operator.
NOT operator is implemented in a generic manner i.e. it can support most underlying operators.
The operator operates by "skipping" documents that are valid for the underlying predicate and iterating over all other valid docIDs.
Support for LIKE operator is not present yet due to Calcite's slightly different parsing of the LIKE operator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#issuecomment-1033511038
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8148](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9d299b3) into [master](https://codecov.io/gh/apache/pinot/commit/5fa4737072d7b4e38164f44bdf563bbf3d6a62ba?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5fa4737) will **decrease** coverage by `6.70%`.
> The diff coverage is `76.92%`.
[](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8148 +/- ##
============================================
- Coverage 71.41% 64.70% -6.71%
Complexity 4302 4302
============================================
Files 1623 1581 -42
Lines 84312 82496 -1816
Branches 12639 12445 -194
============================================
- Hits 60213 53383 -6830
- Misses 19974 25298 +5324
+ Partials 4125 3815 -310
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `67.89% <76.92%> (+0.01%)` | :arrow_up: |
| unittests2 | `14.16% <0.00%> (-0.02%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...inot/core/operator/filter/FilterOperatorUtils.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvRmlsdGVyT3BlcmF0b3JVdGlscy5qYXZh) | `80.00% <42.85%> (-7.68%)` | :arrow_down: |
| [.../pinot/core/operator/filter/NotFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvTm90RmlsdGVyT3BlcmF0b3IuamF2YQ==) | `50.00% <50.00%> (ø)` | |
| [...ava/org/apache/pinot/core/plan/FilterPlanNode.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0ZpbHRlclBsYW5Ob2RlLmphdmE=) | `87.85% <75.00%> (-1.37%)` | :arrow_down: |
| [...che/pinot/core/operator/docidsets/NotDocIdSet.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZHNldHMvTm90RG9jSWRTZXQuamF2YQ==) | `83.33% <83.33%> (ø)` | |
| [...core/operator/dociditerators/NotDocIdIterator.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9Ob3REb2NJZEl0ZXJhdG9yLmphdmE=) | `87.09% <87.09%> (ø)` | |
| [...he/pinot/common/request/context/FilterContext.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L0ZpbHRlckNvbnRleHQuamF2YQ==) | `78.37% <100.00%> (ø)` | |
| [...ot/common/request/context/RequestContextUtils.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L1JlcXVlc3RDb250ZXh0VXRpbHMuamF2YQ==) | `74.01% <100.00%> (-0.36%)` | :arrow_down: |
| [.../apache/pinot/pql/parsers/pql2/ast/FilterKind.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9GaWx0ZXJLaW5kLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [379 more](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5fa4737...9d299b3](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#issuecomment-1033511038
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8148](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bcb3886) into [master](https://codecov.io/gh/apache/pinot/commit/5fa4737072d7b4e38164f44bdf563bbf3d6a62ba?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5fa4737) will **decrease** coverage by `57.31%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8148 +/- ##
=============================================
- Coverage 71.41% 14.09% -57.32%
+ Complexity 4302 81 -4221
=============================================
Files 1623 1584 -39
Lines 84312 83250 -1062
Branches 12639 12608 -31
=============================================
- Hits 60213 11737 -48476
- Misses 19974 70636 +50662
+ Partials 4125 877 -3248
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.09% <0.00%> (-0.09%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...he/pinot/common/request/context/FilterContext.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L0ZpbHRlckNvbnRleHQuamF2YQ==) | `0.00% <0.00%> (-78.38%)` | :arrow_down: |
| [...ot/common/request/context/RequestContextUtils.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L1JlcXVlc3RDb250ZXh0VXRpbHMuamF2YQ==) | `0.00% <0.00%> (-74.38%)` | :arrow_down: |
| [.../apache/pinot/pql/parsers/pql2/ast/FilterKind.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9GaWx0ZXJLaW5kLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...core/operator/dociditerators/NotDocIdIterator.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9Ob3REb2NJZEl0ZXJhdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/core/operator/docidsets/NotDocIdSet.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZHNldHMvTm90RG9jSWRTZXQuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...inot/core/operator/filter/FilterOperatorUtils.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvRmlsdGVyT3BlcmF0b3JVdGlscy5qYXZh) | `0.00% <0.00%> (-87.68%)` | :arrow_down: |
| [.../pinot/core/operator/filter/NotFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvTm90RmlsdGVyT3BlcmF0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ava/org/apache/pinot/core/plan/FilterPlanNode.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0ZpbHRlclBsYW5Ob2RlLmphdmE=) | `0.00% <0.00%> (-89.22%)` | :arrow_down: |
| [...ery/optimizer/filter/NumericalFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvZmlsdGVyL051bWVyaWNhbEZpbHRlck9wdGltaXplci5qYXZh) | `0.00% <ø> (-85.72%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1322 more](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5fa4737...bcb3886](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#discussion_r806346742
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIteratorTest.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+
+
+public class NotDocIdIteratorTest {
+ @Test
+ public void testNotDocIdIterator() {
+ // OR result: [0, 1, 2, 4, 5, 6, 8, 10, 13, 15, 16, 17, 18, 19, 20]
+ int[] docIds1 = new int[]{1, 4, 6, 10, 15, 17, 18, 20};
+ int[] docIds2 = new int[]{0, 1, 5, 8, 15, 18};
+ int[] docIds3 = new int[]{1, 2, 6, 13, 16, 19};
+ int[] docIds4 = new int[]{0, 1, 2, 3, 4, 5};
+
+ MutableRoaringBitmap bitmap1 = new MutableRoaringBitmap();
+ bitmap1.add(docIds1);
+ MutableRoaringBitmap bitmap2 = new MutableRoaringBitmap();
+ bitmap2.add(docIds2);
+ MutableRoaringBitmap bitmap3 = new MutableRoaringBitmap();
+ bitmap3.add(docIds3);
+ MutableRoaringBitmap bitmap4 = new MutableRoaringBitmap();
+ bitmap4.add(docIds4);
+ OrDocIdIterator orDocIdIterator = new OrDocIdIterator(new BlockDocIdIterator[]{
Review comment:
Why do we need to build this `OrDocIdIterator`? We don't want to test `OrDocIdIterator` in this test
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIteratorTest.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+
+
+public class NotDocIdIteratorTest {
+ @Test
+ public void testNotDocIdIterator() {
+ // OR result: [0, 1, 2, 4, 5, 6, 8, 10, 13, 15, 16, 17, 18, 19, 20]
+ int[] docIds1 = new int[]{1, 4, 6, 10, 15, 17, 18, 20};
+ int[] docIds2 = new int[]{0, 1, 5, 8, 15, 18};
+ int[] docIds3 = new int[]{1, 2, 6, 13, 16, 19};
+ int[] docIds4 = new int[]{0, 1, 2, 3, 4, 5};
+
+ MutableRoaringBitmap bitmap1 = new MutableRoaringBitmap();
+ bitmap1.add(docIds1);
+ MutableRoaringBitmap bitmap2 = new MutableRoaringBitmap();
+ bitmap2.add(docIds2);
+ MutableRoaringBitmap bitmap3 = new MutableRoaringBitmap();
+ bitmap3.add(docIds3);
+ MutableRoaringBitmap bitmap4 = new MutableRoaringBitmap();
+ bitmap4.add(docIds4);
+ OrDocIdIterator orDocIdIterator = new OrDocIdIterator(new BlockDocIdIterator[]{
+ new RangelessBitmapDocIdIterator(bitmap1), new RangelessBitmapDocIdIterator(
+ bitmap2), new RangelessBitmapDocIdIterator(bitmap3)
+ });
+ NotDocIdIterator notDocIdIterator = new NotDocIdIterator(new RangelessBitmapDocIdIterator(bitmap1), 25);
+
+ assertEquals(notDocIdIterator.advance(1), 2);
+ assertEquals(notDocIdIterator.next(), 3);
+ assertEquals(notDocIdIterator.next(), 5);
+ assertEquals(notDocIdIterator.advance(7), 8);
Review comment:
(major) Per the contract, this should return 7
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIterator.java
##########
@@ -0,0 +1,97 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+
+
+/**
+ * The iterator performs a linear pass through the underlying child iterator and returns
+ * the complement of the result set.
+ */
+public class NotDocIdIterator implements BlockDocIdIterator {
+ private final BlockDocIdIterator _childDocIdIterator;
+ private final int _numDocs;
+ private int _nextDocId;
+ private int _nextNonMatchingDocId;
+
+ public NotDocIdIterator(BlockDocIdIterator childDocIdIterator, int numDocs) {
+ _childDocIdIterator = childDocIdIterator;
+ _nextDocId = 0;
+
+ int currentDocIdFromChildIterator = childDocIdIterator.next();
+ _nextNonMatchingDocId = currentDocIdFromChildIterator == Constants.EOF ? numDocs : currentDocIdFromChildIterator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public int next() {
+ while (_nextDocId == _nextNonMatchingDocId) {
+ _nextDocId = _nextNonMatchingDocId + 1;
+
+ int nextMatchingDocId = _childDocIdIterator.next();
+
+ if (nextMatchingDocId == Constants.EOF) {
+ _nextNonMatchingDocId = _numDocs;
+ } else {
+ _nextNonMatchingDocId = nextMatchingDocId;
+ }
+ }
+
+ if (_nextDocId >= _numDocs) {
+ return Constants.EOF;
+ }
+
+ return _nextDocId++;
+ }
+
+ @Override
+ public int advance(int targetDocId) {
+ if (targetDocId == Constants.EOF || targetDocId > _numDocs) {
+ return Constants.EOF;
+ }
+
+ if (targetDocId < _nextDocId) {
+ return _nextDocId;
+ }
+
+ _nextDocId = targetDocId + 1;
+
+ int upperLimit = findUpperLimitGreaterThanDocId(targetDocId);
+
+ if (upperLimit == Constants.EOF) {
+ _nextNonMatchingDocId = _numDocs;
+ } else {
+ _nextNonMatchingDocId = upperLimit;
+ }
+
+ return _nextDocId++;
+ }
+
+ private int findUpperLimitGreaterThanDocId(int currentDocId) {
+ int result = _childDocIdIterator.advance(currentDocId);
Review comment:
(major) need to check `_nextNonMatchingDocId < currentDocId` before advancing the pointer
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIterator.java
##########
@@ -0,0 +1,97 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+
+
+/**
+ * The iterator performs a linear pass through the underlying child iterator and returns
+ * the complement of the result set.
+ */
+public class NotDocIdIterator implements BlockDocIdIterator {
+ private final BlockDocIdIterator _childDocIdIterator;
+ private final int _numDocs;
+ private int _nextDocId;
+ private int _nextNonMatchingDocId;
+
+ public NotDocIdIterator(BlockDocIdIterator childDocIdIterator, int numDocs) {
+ _childDocIdIterator = childDocIdIterator;
+ _nextDocId = 0;
+
+ int currentDocIdFromChildIterator = childDocIdIterator.next();
+ _nextNonMatchingDocId = currentDocIdFromChildIterator == Constants.EOF ? numDocs : currentDocIdFromChildIterator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public int next() {
+ while (_nextDocId == _nextNonMatchingDocId) {
+ _nextDocId = _nextNonMatchingDocId + 1;
+
+ int nextMatchingDocId = _childDocIdIterator.next();
+
+ if (nextMatchingDocId == Constants.EOF) {
+ _nextNonMatchingDocId = _numDocs;
+ } else {
+ _nextNonMatchingDocId = nextMatchingDocId;
+ }
+ }
+
+ if (_nextDocId >= _numDocs) {
+ return Constants.EOF;
+ }
+
+ return _nextDocId++;
+ }
+
+ @Override
+ public int advance(int targetDocId) {
+ if (targetDocId == Constants.EOF || targetDocId > _numDocs) {
+ return Constants.EOF;
+ }
+
+ if (targetDocId < _nextDocId) {
+ return _nextDocId;
+ }
Review comment:
These checks are redundant because by contract, `targetDocId >= _nextDocId && targetDocId < _numDocs`
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIterator.java
##########
@@ -0,0 +1,97 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+
+
+/**
+ * The iterator performs a linear pass through the underlying child iterator and returns
+ * the complement of the result set.
+ */
+public class NotDocIdIterator implements BlockDocIdIterator {
+ private final BlockDocIdIterator _childDocIdIterator;
+ private final int _numDocs;
+ private int _nextDocId;
+ private int _nextNonMatchingDocId;
+
+ public NotDocIdIterator(BlockDocIdIterator childDocIdIterator, int numDocs) {
+ _childDocIdIterator = childDocIdIterator;
+ _nextDocId = 0;
+
+ int currentDocIdFromChildIterator = childDocIdIterator.next();
+ _nextNonMatchingDocId = currentDocIdFromChildIterator == Constants.EOF ? numDocs : currentDocIdFromChildIterator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public int next() {
+ while (_nextDocId == _nextNonMatchingDocId) {
+ _nextDocId = _nextNonMatchingDocId + 1;
+
+ int nextMatchingDocId = _childDocIdIterator.next();
+
+ if (nextMatchingDocId == Constants.EOF) {
+ _nextNonMatchingDocId = _numDocs;
+ } else {
+ _nextNonMatchingDocId = nextMatchingDocId;
+ }
+ }
+
+ if (_nextDocId >= _numDocs) {
+ return Constants.EOF;
+ }
+
+ return _nextDocId++;
+ }
+
+ @Override
+ public int advance(int targetDocId) {
+ if (targetDocId == Constants.EOF || targetDocId > _numDocs) {
+ return Constants.EOF;
+ }
+
+ if (targetDocId < _nextDocId) {
+ return _nextDocId;
+ }
+
+ _nextDocId = targetDocId + 1;
Review comment:
(major) This will skip the `targetDocId`
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIteratorTest.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+
+
+public class NotDocIdIteratorTest {
+ @Test
+ public void testNotDocIdIterator() {
+ // OR result: [0, 1, 2, 4, 5, 6, 8, 10, 13, 15, 16, 17, 18, 19, 20]
+ int[] docIds1 = new int[]{1, 4, 6, 10, 15, 17, 18, 20};
+ int[] docIds2 = new int[]{0, 1, 5, 8, 15, 18};
+ int[] docIds3 = new int[]{1, 2, 6, 13, 16, 19};
+ int[] docIds4 = new int[]{0, 1, 2, 3, 4, 5};
+
+ MutableRoaringBitmap bitmap1 = new MutableRoaringBitmap();
+ bitmap1.add(docIds1);
+ MutableRoaringBitmap bitmap2 = new MutableRoaringBitmap();
+ bitmap2.add(docIds2);
+ MutableRoaringBitmap bitmap3 = new MutableRoaringBitmap();
+ bitmap3.add(docIds3);
+ MutableRoaringBitmap bitmap4 = new MutableRoaringBitmap();
+ bitmap4.add(docIds4);
+ OrDocIdIterator orDocIdIterator = new OrDocIdIterator(new BlockDocIdIterator[]{
+ new RangelessBitmapDocIdIterator(bitmap1), new RangelessBitmapDocIdIterator(
+ bitmap2), new RangelessBitmapDocIdIterator(bitmap3)
+ });
+ NotDocIdIterator notDocIdIterator = new NotDocIdIterator(new RangelessBitmapDocIdIterator(bitmap1), 25);
+
+ assertEquals(notDocIdIterator.advance(1), 2);
+ assertEquals(notDocIdIterator.next(), 3);
+ assertEquals(notDocIdIterator.next(), 5);
+ assertEquals(notDocIdIterator.advance(7), 8);
+ assertEquals(notDocIdIterator.advance(13), 14);
Review comment:
(major) This should return 13, same for other places
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#issuecomment-1033511038
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8148](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (90ab312) into [master](https://codecov.io/gh/apache/pinot/commit/5fa4737072d7b4e38164f44bdf563bbf3d6a62ba?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5fa4737) will **decrease** coverage by `57.22%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8148 +/- ##
=============================================
- Coverage 71.41% 14.19% -57.23%
+ Complexity 4302 81 -4221
=============================================
Files 1623 1581 -42
Lines 84312 82545 -1767
Branches 12639 12463 -176
=============================================
- Hits 60213 11716 -48497
- Misses 19974 69960 +49986
+ Partials 4125 869 -3256
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.19% <0.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...he/pinot/common/request/context/FilterContext.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L0ZpbHRlckNvbnRleHQuamF2YQ==) | `0.00% <0.00%> (-78.38%)` | :arrow_down: |
| [...ot/common/request/context/RequestContextUtils.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L1JlcXVlc3RDb250ZXh0VXRpbHMuamF2YQ==) | `0.00% <0.00%> (-74.38%)` | :arrow_down: |
| [.../apache/pinot/pql/parsers/pql2/ast/FilterKind.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9GaWx0ZXJLaW5kLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...core/operator/dociditerators/NotDocIdIterator.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9Ob3REb2NJZEl0ZXJhdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/core/operator/docidsets/NotDocIdSet.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZHNldHMvTm90RG9jSWRTZXQuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...inot/core/operator/filter/FilterOperatorUtils.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvRmlsdGVyT3BlcmF0b3JVdGlscy5qYXZh) | `0.00% <0.00%> (-87.68%)` | :arrow_down: |
| [.../pinot/core/operator/filter/NotFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvTm90RmlsdGVyT3BlcmF0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ava/org/apache/pinot/core/plan/FilterPlanNode.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0ZpbHRlclBsYW5Ob2RlLmphdmE=) | `0.00% <0.00%> (-89.22%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1304 more](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5fa4737...90ab312](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] atris commented on a change in pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#discussion_r805614108
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIterator.java
##########
@@ -0,0 +1,97 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+
+
+/**
+ * The iterator performs a linear pass through the underlying child iterator and returns
+ * the complement of the result set.
+ */
+public class NotDocIdIterator implements BlockDocIdIterator {
+ private BlockDocIdIterator _childDocIdIterator;
+ private int _lowerLimit;
+ private int _upperLimit;
+ private int _numDocs;
+
+ public NotDocIdIterator(BlockDocIdIterator childDocIdIterator, int numDocs) {
+ _childDocIdIterator = childDocIdIterator;
+ _lowerLimit = 0;
+
+ int currentDocIdFromChildIterator = childDocIdIterator.next();
+ _upperLimit = currentDocIdFromChildIterator == Constants.EOF ? numDocs : currentDocIdFromChildIterator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public int next() {
+ while (_lowerLimit == _upperLimit) {
+ _lowerLimit = _upperLimit + 1;
+
+ int nextMatchingDocId = _childDocIdIterator.next();
+
+ if (nextMatchingDocId == Constants.EOF) {
+ _upperLimit = _numDocs;
+ } else {
+ _upperLimit = nextMatchingDocId;
+ }
+ }
+
+ if (_lowerLimit >= _numDocs) {
+ return Constants.EOF;
+ }
+
+ return _lowerLimit++;
+ }
+
+ @Override
+ public int advance(int targetDocId) {
+ if (targetDocId == Constants.EOF || targetDocId > _numDocs) {
Review comment:
Not sure if that is valid -- doing that causes tests in NotDocIteratorTest to fail
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#discussion_r803136508
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIterator.java
##########
@@ -0,0 +1,97 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+
+
+/**
+ * The iterator performs a linear pass through the underlying child iterator and returns
+ * the complement of the result set.
+ */
+public class NotDocIdIterator implements BlockDocIdIterator {
+ private BlockDocIdIterator _childDocIdIterator;
+ private int _lowerLimit;
+ private int _upperLimit;
Review comment:
Suggest renaming `_upperLimit` to `_nextNonMatchingDocId`
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIterator.java
##########
@@ -0,0 +1,97 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+
+
+/**
+ * The iterator performs a linear pass through the underlying child iterator and returns
+ * the complement of the result set.
+ */
+public class NotDocIdIterator implements BlockDocIdIterator {
+ private BlockDocIdIterator _childDocIdIterator;
+ private int _lowerLimit;
+ private int _upperLimit;
+ private int _numDocs;
+
+ public NotDocIdIterator(BlockDocIdIterator childDocIdIterator, int numDocs) {
+ _childDocIdIterator = childDocIdIterator;
+ _lowerLimit = 0;
+
+ int currentDocIdFromChildIterator = childDocIdIterator.next();
+ _upperLimit = currentDocIdFromChildIterator == Constants.EOF ? numDocs : currentDocIdFromChildIterator;
Review comment:
No need to initialize the upper limit here. The value check in `next()` should be able to handle that
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/request/context/RequestContextUtils.java
##########
@@ -183,6 +183,12 @@ public static FilterContext getFilter(Expression thriftExpression) {
children.add(getFilter(operand));
}
return new FilterContext(FilterContext.Type.OR, children, null);
+ case NOT:
+ children = new ArrayList<>(numOperands);
+ for (Expression operand : operands) {
+ children.add(getFilter(operand));
+ }
+ return new FilterContext(FilterContext.Type.NOT, children, null);
Review comment:
(minor) `numOperands` should always be 1, so this part can be optimized
```suggestion
assert numOperands == 1;
return new FilterContext(FilterContext.Type.NOT, Collections.singletonList(getFilter(operands.get(0))), null);
```
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/filter/FilterOperatorUtils.java
##########
@@ -165,6 +179,9 @@ int getPriority(BaseFilterOperator filterOperator) {
if (filterOperator instanceof OrFilterOperator) {
return 4;
}
+ if (filterOperator instanceof NotFilterOperator) {
Review comment:
`NotFilterOperator` should have the same priority as it's child operator
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIterator.java
##########
@@ -0,0 +1,97 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+
+
+/**
+ * The iterator performs a linear pass through the underlying child iterator and returns
+ * the complement of the result set.
+ */
+public class NotDocIdIterator implements BlockDocIdIterator {
+ private BlockDocIdIterator _childDocIdIterator;
+ private int _lowerLimit;
Review comment:
Suggest renaming `_lowerLimit` to `_nextDocId` to be consistent with other iterators?
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIterator.java
##########
@@ -0,0 +1,97 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+
+
+/**
+ * The iterator performs a linear pass through the underlying child iterator and returns
+ * the complement of the result set.
+ */
+public class NotDocIdIterator implements BlockDocIdIterator {
+ private BlockDocIdIterator _childDocIdIterator;
+ private int _lowerLimit;
+ private int _upperLimit;
+ private int _numDocs;
+
+ public NotDocIdIterator(BlockDocIdIterator childDocIdIterator, int numDocs) {
+ _childDocIdIterator = childDocIdIterator;
+ _lowerLimit = 0;
+
+ int currentDocIdFromChildIterator = childDocIdIterator.next();
+ _upperLimit = currentDocIdFromChildIterator == Constants.EOF ? numDocs : currentDocIdFromChildIterator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public int next() {
+ while (_lowerLimit == _upperLimit) {
Review comment:
Change this to `while (_lowerLimit >= _upperLimit)`, then both constructor and `advance()` can be simplified
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIterator.java
##########
@@ -0,0 +1,97 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+
+
+/**
+ * The iterator performs a linear pass through the underlying child iterator and returns
+ * the complement of the result set.
+ */
+public class NotDocIdIterator implements BlockDocIdIterator {
+ private BlockDocIdIterator _childDocIdIterator;
+ private int _lowerLimit;
+ private int _upperLimit;
+ private int _numDocs;
+
+ public NotDocIdIterator(BlockDocIdIterator childDocIdIterator, int numDocs) {
+ _childDocIdIterator = childDocIdIterator;
+ _lowerLimit = 0;
+
+ int currentDocIdFromChildIterator = childDocIdIterator.next();
+ _upperLimit = currentDocIdFromChildIterator == Constants.EOF ? numDocs : currentDocIdFromChildIterator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public int next() {
+ while (_lowerLimit == _upperLimit) {
+ _lowerLimit = _upperLimit + 1;
+
+ int nextMatchingDocId = _childDocIdIterator.next();
+
+ if (nextMatchingDocId == Constants.EOF) {
+ _upperLimit = _numDocs;
+ } else {
+ _upperLimit = nextMatchingDocId;
+ }
+ }
+
+ if (_lowerLimit >= _numDocs) {
+ return Constants.EOF;
+ }
+
+ return _lowerLimit++;
+ }
+
+ @Override
+ public int advance(int targetDocId) {
+ if (targetDocId == Constants.EOF || targetDocId > _numDocs) {
Review comment:
This can be simplified to:
```
_nextDocId = targetDocId;
return next();
```
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIterator.java
##########
@@ -0,0 +1,97 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+
+
+/**
+ * The iterator performs a linear pass through the underlying child iterator and returns
+ * the complement of the result set.
+ */
+public class NotDocIdIterator implements BlockDocIdIterator {
+ private BlockDocIdIterator _childDocIdIterator;
Review comment:
Make `_childDocIdIterator` and `_numDocs` final
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/filter/NotFilterOperator.java
##########
@@ -0,0 +1,63 @@
+/**
+ * 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 java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.core.common.Operator;
+import org.apache.pinot.core.operator.blocks.FilterBlock;
+import org.apache.pinot.core.operator.docidsets.NotDocIdSet;
+
+
+public class NotFilterOperator extends BaseFilterOperator {
+ private static final String OPERATOR_NAME = "NotFilterOperator";
+ private static final String EXPLAIN_NAME = "FILTER_NOT";
+ private final BaseFilterOperator _filterOperator;
+ private final int _numDocs;
+
+ public NotFilterOperator(BaseFilterOperator filterOperator, int numDocs) {
+ _filterOperator = filterOperator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public String getOperatorName() {
+ return OPERATOR_NAME;
+ }
+
+ @Override
+ public List<Operator> getChildOperators() {
+ List<Operator> resultList = new ArrayList<>();
+ resultList.add(_filterOperator);
+
+ return resultList;
Review comment:
(minor)
```suggestion
return Collections.singletonList(_filterOperator);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] ashishkf commented on pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
ashishkf commented on pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#issuecomment-1046168510
When I tried the change from the controller UI, I got the following exception:
>
> pinot-broker-2 broker 2022/02/20 05:46:39.065 ERROR [PinotClientRequest] [jersey-server-managed-async-executor-2] Caught exception while processing POST request
> pinot-broker-2 broker java.lang.IndexOutOfBoundsException: Index 1 out of bounds for length 1
> pinot-broker-2 broker at jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64) ~[?:?]
> pinot-broker-2 broker at jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70) ~[?:?]
> pinot-broker-2 broker at jdk.internal.util.Preconditions.checkIndex(Preconditions.java:248) ~[?:?]
> pinot-broker-2 broker at java.util.Objects.checkIndex(Objects.java:372) ~[?:?]
> pinot-broker-2 broker at java.util.ArrayList.get(ArrayList.java:459) ~[?:?]
> pinot-broker-2 broker at org.apache.pinot.core.query.optimizer.filter.NumericalFilterOptimizer.optimize(NumericalFilterOptimizer.java:102) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> pinot-broker-2 broker at org.apache.pinot.core.query.optimizer.QueryOptimizer.optimize(QueryOptimizer.java:81) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> pinot-broker-2 broker at org.apache.pinot.broker.requesthandler.BaseBrokerRequestHandler.handleSQLRequest(BaseBrokerRequestHandler.java:381) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> pinot-broker-2 broker at org.apache.pinot.broker.requesthandler.BaseBrokerRequestHandler.handleRequest(BaseBrokerRequestHandler.java:197) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> pinot-broker-2 broker at org.apache.pinot.broker.requesthandler.BaseBrokerRequestHandler.handleRequest(BaseBrokerRequestHandler.java:102) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> pinot-broker-2 broker at org.apache.pinot.broker.api.resources.PinotClientRequest.processSqlQueryPost(PinotClientRequest.java:191) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> pinot-broker-2 broker at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:?]
> pinot-broker-2 broker at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:?]
> pinot-broker-2 broker at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:?]
> pinot-broker-2 broker at java.lang.reflect.Method.invoke(Method.java:566) ~[?:?]
> pinot-broker-2 broker at org.glassfish.jersey.server.model.internal.ResourceMethodInvocationHandlerFactory.lambda$static$0(ResourceMethodInvocationHandlerFactory.java:52) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> pinot-broker-2 broker at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher$1.run(AbstractJavaResourceMethodDispatcher.java:124) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> pinot-broker-2 broker at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.invoke(AbstractJavaResourceMethodDispatcher.java:167) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> pinot-broker-2 broker at org.glassfish.jersey.server.model.internal.JavaResourceMethodDispatcherProvider$VoidOutInvoker.doDispatch(JavaResourceMethodDispatcherProvider.java:159) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> pinot-broker-2 broker at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.dispatch(AbstractJavaResourceMethodDispatcher.java:79) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> pinot-broker-2 broker at org.glassfish.jersey.server.model.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:469) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> pinot-broker-2 broker at org.glassfish.jersey.server.model.ResourceMethodInvoker.lambda$apply$0(ResourceMethodInvoker.java:381) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> pinot-broker-2 broker at org.glassfish.jersey.server.ServerRuntime$AsyncResponder$2$1.run(ServerRuntime.java:819) [pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29
]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#issuecomment-1033511038
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8148](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (90ab312) into [master](https://codecov.io/gh/apache/pinot/commit/5fa4737072d7b4e38164f44bdf563bbf3d6a62ba?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5fa4737) will **decrease** coverage by `6.67%`.
> The diff coverage is `76.66%`.
[](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8148 +/- ##
============================================
- Coverage 71.41% 64.74% -6.68%
- Complexity 4302 4312 +10
============================================
Files 1623 1581 -42
Lines 84312 82545 -1767
Branches 12639 12463 -176
============================================
- Hits 60213 53442 -6771
- Misses 19974 25284 +5310
+ Partials 4125 3819 -306
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `67.94% <76.66%> (+0.06%)` | :arrow_up: |
| unittests2 | `14.19% <0.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...inot/core/operator/filter/FilterOperatorUtils.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvRmlsdGVyT3BlcmF0b3JVdGlscy5qYXZh) | `80.00% <42.85%> (-7.68%)` | :arrow_down: |
| [...ot/common/request/context/RequestContextUtils.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L1JlcXVlc3RDb250ZXh0VXRpbHMuamF2YQ==) | `72.77% <50.00%> (-1.60%)` | :arrow_down: |
| [.../pinot/core/operator/filter/NotFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvTm90RmlsdGVyT3BlcmF0b3IuamF2YQ==) | `62.50% <62.50%> (ø)` | |
| [...ava/org/apache/pinot/core/plan/FilterPlanNode.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0ZpbHRlclBsYW5Ob2RlLmphdmE=) | `87.85% <75.00%> (-1.37%)` | :arrow_down: |
| [...che/pinot/core/operator/docidsets/NotDocIdSet.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZHNldHMvTm90RG9jSWRTZXQuamF2YQ==) | `83.33% <83.33%> (ø)` | |
| [...core/operator/dociditerators/NotDocIdIterator.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9Ob3REb2NJZEl0ZXJhdG9yLmphdmE=) | `87.09% <87.09%> (ø)` | |
| [...he/pinot/common/request/context/FilterContext.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L0ZpbHRlckNvbnRleHQuamF2YQ==) | `78.37% <100.00%> (ø)` | |
| [.../apache/pinot/pql/parsers/pql2/ast/FilterKind.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9GaWx0ZXJLaW5kLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [409 more](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5fa4737...90ab312](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] atris merged pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
atris merged pull request #8148:
URL: https://github.com/apache/pinot/pull/8148
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#issuecomment-1033511038
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8148](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9d299b3) into [master](https://codecov.io/gh/apache/pinot/commit/5fa4737072d7b4e38164f44bdf563bbf3d6a62ba?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5fa4737) will **decrease** coverage by `57.25%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8148 +/- ##
=============================================
- Coverage 71.41% 14.16% -57.26%
+ Complexity 4302 81 -4221
=============================================
Files 1623 1581 -42
Lines 84312 82496 -1816
Branches 12639 12445 -194
=============================================
- Hits 60213 11685 -48528
- Misses 19974 69944 +49970
+ Partials 4125 867 -3258
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.16% <0.00%> (-0.02%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...he/pinot/common/request/context/FilterContext.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L0ZpbHRlckNvbnRleHQuamF2YQ==) | `0.00% <0.00%> (-78.38%)` | :arrow_down: |
| [...ot/common/request/context/RequestContextUtils.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L1JlcXVlc3RDb250ZXh0VXRpbHMuamF2YQ==) | `0.00% <0.00%> (-74.38%)` | :arrow_down: |
| [.../apache/pinot/pql/parsers/pql2/ast/FilterKind.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9GaWx0ZXJLaW5kLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...core/operator/dociditerators/NotDocIdIterator.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9Ob3REb2NJZEl0ZXJhdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/core/operator/docidsets/NotDocIdSet.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZHNldHMvTm90RG9jSWRTZXQuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...inot/core/operator/filter/FilterOperatorUtils.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvRmlsdGVyT3BlcmF0b3JVdGlscy5qYXZh) | `0.00% <0.00%> (-87.68%)` | :arrow_down: |
| [.../pinot/core/operator/filter/NotFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvTm90RmlsdGVyT3BlcmF0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ava/org/apache/pinot/core/plan/FilterPlanNode.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0ZpbHRlclBsYW5Ob2RlLmphdmE=) | `0.00% <0.00%> (-89.22%)` | :arrow_down: |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1300 more](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5fa4737...9d299b3](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#issuecomment-1031736349
Thanks for the change, @atris . I will review today
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] atris commented on a change in pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#discussion_r806530206
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIteratorTest.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+
+
+public class NotDocIdIteratorTest {
+ @Test
+ public void testNotDocIdIterator() {
+ // OR result: [0, 1, 2, 4, 5, 6, 8, 10, 13, 15, 16, 17, 18, 19, 20]
+ int[] docIds1 = new int[]{1, 4, 6, 10, 15, 17, 18, 20};
+ int[] docIds2 = new int[]{0, 1, 5, 8, 15, 18};
+ int[] docIds3 = new int[]{1, 2, 6, 13, 16, 19};
+ int[] docIds4 = new int[]{0, 1, 2, 3, 4, 5};
+
+ MutableRoaringBitmap bitmap1 = new MutableRoaringBitmap();
+ bitmap1.add(docIds1);
+ MutableRoaringBitmap bitmap2 = new MutableRoaringBitmap();
+ bitmap2.add(docIds2);
+ MutableRoaringBitmap bitmap3 = new MutableRoaringBitmap();
+ bitmap3.add(docIds3);
+ MutableRoaringBitmap bitmap4 = new MutableRoaringBitmap();
+ bitmap4.add(docIds4);
+ OrDocIdIterator orDocIdIterator = new OrDocIdIterator(new BlockDocIdIterator[]{
+ new RangelessBitmapDocIdIterator(bitmap1), new RangelessBitmapDocIdIterator(
+ bitmap2), new RangelessBitmapDocIdIterator(bitmap3)
+ });
+ NotDocIdIterator notDocIdIterator = new NotDocIdIterator(new RangelessBitmapDocIdIterator(bitmap1), 25);
+
+ assertEquals(notDocIdIterator.advance(1), 2);
+ assertEquals(notDocIdIterator.next(), 3);
+ assertEquals(notDocIdIterator.next(), 5);
+ assertEquals(notDocIdIterator.advance(7), 8);
Review comment:
The comment above advance() BlockDocIdIterator is a bit confusing. It says:
Returns the first matching document whose id is greater than or equal to the given target document id
I interpreted it as being ok if we return the first id greater than the target document id, which is what this test demonstrates. If that is not the case, should we reword the API spec to be explicit?
Fixed this issue, thanks for sharing the proposed solution!
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIteratorTest.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+
+
+public class NotDocIdIteratorTest {
+ @Test
+ public void testNotDocIdIterator() {
+ // OR result: [0, 1, 2, 4, 5, 6, 8, 10, 13, 15, 16, 17, 18, 19, 20]
+ int[] docIds1 = new int[]{1, 4, 6, 10, 15, 17, 18, 20};
+ int[] docIds2 = new int[]{0, 1, 5, 8, 15, 18};
+ int[] docIds3 = new int[]{1, 2, 6, 13, 16, 19};
+ int[] docIds4 = new int[]{0, 1, 2, 3, 4, 5};
+
+ MutableRoaringBitmap bitmap1 = new MutableRoaringBitmap();
+ bitmap1.add(docIds1);
+ MutableRoaringBitmap bitmap2 = new MutableRoaringBitmap();
+ bitmap2.add(docIds2);
+ MutableRoaringBitmap bitmap3 = new MutableRoaringBitmap();
+ bitmap3.add(docIds3);
+ MutableRoaringBitmap bitmap4 = new MutableRoaringBitmap();
+ bitmap4.add(docIds4);
+ OrDocIdIterator orDocIdIterator = new OrDocIdIterator(new BlockDocIdIterator[]{
Review comment:
We use it to test NotDocIdIterator later in the test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] atris commented on pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
atris commented on pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#issuecomment-1046560448
> When I tried the change from the controller UI, I got the following exception:
>
> > pinot-broker-2 broker 2022/02/20 05:46:39.065 ERROR [PinotClientRequest] [jersey-server-managed-async-executor-2] Caught exception while processing POST request
> > pinot-broker-2 broker java.lang.IndexOutOfBoundsException: Index 1 out of bounds for length 1
> > pinot-broker-2 broker at jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64) ~[?:?]
> > pinot-broker-2 broker at jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70) ~[?:?]
> > pinot-broker-2 broker at jdk.internal.util.Preconditions.checkIndex(Preconditions.java:248) ~[?:?]
> > pinot-broker-2 broker at java.util.Objects.checkIndex(Objects.java:372) ~[?:?]
> > pinot-broker-2 broker at java.util.ArrayList.get(ArrayList.java:459) ~[?:?]
> > pinot-broker-2 broker at org.apache.pinot.core.query.optimizer.filter.NumericalFilterOptimizer.optimize(NumericalFilterOptimizer.java:102) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> > pinot-broker-2 broker at org.apache.pinot.core.query.optimizer.QueryOptimizer.optimize(QueryOptimizer.java:81) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> > pinot-broker-2 broker at org.apache.pinot.broker.requesthandler.BaseBrokerRequestHandler.handleSQLRequest(BaseBrokerRequestHandler.java:381) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> > pinot-broker-2 broker at org.apache.pinot.broker.requesthandler.BaseBrokerRequestHandler.handleRequest(BaseBrokerRequestHandler.java:197) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> > pinot-broker-2 broker at org.apache.pinot.broker.requesthandler.BaseBrokerRequestHandler.handleRequest(BaseBrokerRequestHandler.java:102) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> > pinot-broker-2 broker at org.apache.pinot.broker.api.resources.PinotClientRequest.processSqlQueryPost(PinotClientRequest.java:191) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> > pinot-broker-2 broker at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:?]
> > pinot-broker-2 broker at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:?]
> > pinot-broker-2 broker at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:?]
> > pinot-broker-2 broker at java.lang.reflect.Method.invoke(Method.java:566) ~[?:?]
> > pinot-broker-2 broker at org.glassfish.jersey.server.model.internal.ResourceMethodInvocationHandlerFactory.lambda$static$0(ResourceMethodInvocationHandlerFactory.java:52) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> > pinot-broker-2 broker at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher$1.run(AbstractJavaResourceMethodDispatcher.java:124) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> > pinot-broker-2 broker at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.invoke(AbstractJavaResourceMethodDispatcher.java:167) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> > pinot-broker-2 broker at org.glassfish.jersey.server.model.internal.JavaResourceMethodDispatcherProvider$VoidOutInvoker.doDispatch(JavaResourceMethodDispatcherProvider.java:159) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> > pinot-broker-2 broker at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.dispatch(AbstractJavaResourceMethodDispatcher.java:79) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> > pinot-broker-2 broker at org.glassfish.jersey.server.model.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:469) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> > pinot-broker-2 broker at org.glassfish.jersey.server.model.ResourceMethodInvoker.lambda$apply$0(ResourceMethodInvoker.java:381) ~[pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29]
> > pinot-broker-2 broker at org.glassfish.jersey.server.ServerRuntime$AsyncResponder$2$1.run(ServerRuntime.java:819) [pinot-all-0.9.0-jar-with-dependencies.jar:0.9.0-02f03f4a81d44564e818ceb720ba25e2ab31bc29
>
> ]
Thanks for trying it out. Can you check with the latest commit?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] atris commented on a change in pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#discussion_r806530206
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIteratorTest.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+
+
+public class NotDocIdIteratorTest {
+ @Test
+ public void testNotDocIdIterator() {
+ // OR result: [0, 1, 2, 4, 5, 6, 8, 10, 13, 15, 16, 17, 18, 19, 20]
+ int[] docIds1 = new int[]{1, 4, 6, 10, 15, 17, 18, 20};
+ int[] docIds2 = new int[]{0, 1, 5, 8, 15, 18};
+ int[] docIds3 = new int[]{1, 2, 6, 13, 16, 19};
+ int[] docIds4 = new int[]{0, 1, 2, 3, 4, 5};
+
+ MutableRoaringBitmap bitmap1 = new MutableRoaringBitmap();
+ bitmap1.add(docIds1);
+ MutableRoaringBitmap bitmap2 = new MutableRoaringBitmap();
+ bitmap2.add(docIds2);
+ MutableRoaringBitmap bitmap3 = new MutableRoaringBitmap();
+ bitmap3.add(docIds3);
+ MutableRoaringBitmap bitmap4 = new MutableRoaringBitmap();
+ bitmap4.add(docIds4);
+ OrDocIdIterator orDocIdIterator = new OrDocIdIterator(new BlockDocIdIterator[]{
+ new RangelessBitmapDocIdIterator(bitmap1), new RangelessBitmapDocIdIterator(
+ bitmap2), new RangelessBitmapDocIdIterator(bitmap3)
+ });
+ NotDocIdIterator notDocIdIterator = new NotDocIdIterator(new RangelessBitmapDocIdIterator(bitmap1), 25);
+
+ assertEquals(notDocIdIterator.advance(1), 2);
+ assertEquals(notDocIdIterator.next(), 3);
+ assertEquals(notDocIdIterator.next(), 5);
+ assertEquals(notDocIdIterator.advance(7), 8);
Review comment:
The comment above advance() BlockDocIdIterator is a bit confusing. It says:
Returns the first matching document whose id is greater than or equal to the given target document id
I interpreted it as being ok if we return the first id greater than the target document id, which is what this test demonstrates. If that is not the case, should we reword the API spec to be explicit?
Fixed this issue, thanks for sharing the proposed solution!
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIteratorTest.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+
+
+public class NotDocIdIteratorTest {
+ @Test
+ public void testNotDocIdIterator() {
+ // OR result: [0, 1, 2, 4, 5, 6, 8, 10, 13, 15, 16, 17, 18, 19, 20]
+ int[] docIds1 = new int[]{1, 4, 6, 10, 15, 17, 18, 20};
+ int[] docIds2 = new int[]{0, 1, 5, 8, 15, 18};
+ int[] docIds3 = new int[]{1, 2, 6, 13, 16, 19};
+ int[] docIds4 = new int[]{0, 1, 2, 3, 4, 5};
+
+ MutableRoaringBitmap bitmap1 = new MutableRoaringBitmap();
+ bitmap1.add(docIds1);
+ MutableRoaringBitmap bitmap2 = new MutableRoaringBitmap();
+ bitmap2.add(docIds2);
+ MutableRoaringBitmap bitmap3 = new MutableRoaringBitmap();
+ bitmap3.add(docIds3);
+ MutableRoaringBitmap bitmap4 = new MutableRoaringBitmap();
+ bitmap4.add(docIds4);
+ OrDocIdIterator orDocIdIterator = new OrDocIdIterator(new BlockDocIdIterator[]{
Review comment:
We use it to test NotDocIdIterator later in the test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#issuecomment-1033511038
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8148](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bcb3886) into [master](https://codecov.io/gh/apache/pinot/commit/5fa4737072d7b4e38164f44bdf563bbf3d6a62ba?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5fa4737) will **decrease** coverage by `7.09%`.
> The diff coverage is `78.00%`.
[](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #8148 +/- ##
============================================
- Coverage 71.41% 64.31% -7.10%
- Complexity 4302 4319 +17
============================================
Files 1623 1584 -39
Lines 84312 83250 -1062
Branches 12639 12608 -31
============================================
- Hits 60213 53546 -6667
- Misses 19974 25864 +5890
+ Partials 4125 3840 -285
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `67.37% <78.00%> (-0.51%)` | :arrow_down: |
| unittests2 | `14.09% <0.00%> (-0.09%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ery/optimizer/filter/NumericalFilterOptimizer.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9vcHRpbWl6ZXIvZmlsdGVyL051bWVyaWNhbEZpbHRlck9wdGltaXplci5qYXZh) | `83.22% <ø> (-2.49%)` | :arrow_down: |
| [...inot/core/operator/filter/FilterOperatorUtils.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvRmlsdGVyT3BlcmF0b3JVdGlscy5qYXZh) | `80.00% <42.85%> (-7.68%)` | :arrow_down: |
| [...ot/common/request/context/RequestContextUtils.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L1JlcXVlc3RDb250ZXh0VXRpbHMuamF2YQ==) | `72.77% <50.00%> (-1.60%)` | :arrow_down: |
| [.../pinot/core/operator/filter/NotFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvTm90RmlsdGVyT3BlcmF0b3IuamF2YQ==) | `55.55% <55.55%> (ø)` | |
| [...ava/org/apache/pinot/core/plan/FilterPlanNode.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0ZpbHRlclBsYW5Ob2RlLmphdmE=) | `87.85% <75.00%> (-1.37%)` | :arrow_down: |
| [...core/operator/dociditerators/NotDocIdIterator.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZGl0ZXJhdG9ycy9Ob3REb2NJZEl0ZXJhdG9yLmphdmE=) | `95.00% <95.00%> (ø)` | |
| [...he/pinot/common/request/context/FilterContext.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVxdWVzdC9jb250ZXh0L0ZpbHRlckNvbnRleHQuamF2YQ==) | `78.37% <100.00%> (ø)` | |
| [.../apache/pinot/pql/parsers/pql2/ast/FilterKind.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9wcWwvcGFyc2Vycy9wcWwyL2FzdC9GaWx0ZXJLaW5kLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...che/pinot/core/operator/docidsets/NotDocIdSet.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9kb2NpZHNldHMvTm90RG9jSWRTZXQuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [448 more](https://codecov.io/gh/apache/pinot/pull/8148/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [5fa4737...bcb3886](https://codecov.io/gh/apache/pinot/pull/8148?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#issuecomment-1063534627
@atris This is a great feature. Could you please add some release notes to the PR description which we can refer to when cutting the next release?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] atris commented on a change in pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#discussion_r805598421
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIterator.java
##########
@@ -0,0 +1,97 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+
+
+/**
+ * The iterator performs a linear pass through the underlying child iterator and returns
+ * the complement of the result set.
+ */
+public class NotDocIdIterator implements BlockDocIdIterator {
+ private BlockDocIdIterator _childDocIdIterator;
+ private int _lowerLimit;
+ private int _upperLimit;
+ private int _numDocs;
+
+ public NotDocIdIterator(BlockDocIdIterator childDocIdIterator, int numDocs) {
+ _childDocIdIterator = childDocIdIterator;
+ _lowerLimit = 0;
+
+ int currentDocIdFromChildIterator = childDocIdIterator.next();
+ _upperLimit = currentDocIdFromChildIterator == Constants.EOF ? numDocs : currentDocIdFromChildIterator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public int next() {
+ while (_lowerLimit == _upperLimit) {
Review comment:
I dont think that works -- since that can lead to skipping of the first docID (0) if that is a valid docID for the not iterator to return. Tests in NotDocIdIteratorTest highlight that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] atris commented on a change in pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#discussion_r805610955
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIterator.java
##########
@@ -0,0 +1,97 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+
+
+/**
+ * The iterator performs a linear pass through the underlying child iterator and returns
+ * the complement of the result set.
+ */
+public class NotDocIdIterator implements BlockDocIdIterator {
+ private BlockDocIdIterator _childDocIdIterator;
+ private int _lowerLimit;
+ private int _upperLimit;
+ private int _numDocs;
+
+ public NotDocIdIterator(BlockDocIdIterator childDocIdIterator, int numDocs) {
+ _childDocIdIterator = childDocIdIterator;
+ _lowerLimit = 0;
+
+ int currentDocIdFromChildIterator = childDocIdIterator.next();
+ _upperLimit = currentDocIdFromChildIterator == Constants.EOF ? numDocs : currentDocIdFromChildIterator;
Review comment:
That doesn't work, since it will lead to skipping the first document (i.e. not initialising upper limit implicitly implies that the first document is matching the inner iterator)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#discussion_r807130020
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/filter/FilterOperatorUtils.java
##########
@@ -165,6 +179,9 @@ int getPriority(BaseFilterOperator filterOperator) {
if (filterOperator instanceof OrFilterOperator) {
return 4;
}
+ if (filterOperator instanceof NotFilterOperator) {
+ return getPriority((BaseFilterOperator) filterOperator.getChildOperators().get(0));
Review comment:
(minor) We can add a `getChildFilterOperator()` in `NotFilterOperator` which can save an unnecessary array allocation
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIteratorTest.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+
+
+public class NotDocIdIteratorTest {
+ @Test
+ public void testNotDocIdIterator() {
+ // OR result: [0, 1, 2, 4, 5, 6, 8, 10, 13, 15, 16, 17, 18, 19, 20]
+ int[] docIds1 = new int[]{1, 4, 6, 10, 15, 17, 18, 20};
+ int[] docIds2 = new int[]{0, 1, 5, 8, 15, 18};
+ int[] docIds3 = new int[]{1, 2, 6, 13, 16, 19};
+ int[] docIds4 = new int[]{0, 1, 2, 3, 4, 5};
+
+ MutableRoaringBitmap bitmap1 = new MutableRoaringBitmap();
+ bitmap1.add(docIds1);
+ MutableRoaringBitmap bitmap2 = new MutableRoaringBitmap();
+ bitmap2.add(docIds2);
+ MutableRoaringBitmap bitmap3 = new MutableRoaringBitmap();
+ bitmap3.add(docIds3);
+ MutableRoaringBitmap bitmap4 = new MutableRoaringBitmap();
+ bitmap4.add(docIds4);
+ OrDocIdIterator orDocIdIterator = new OrDocIdIterator(new BlockDocIdIterator[]{
Review comment:
Understood. What I'm saying is that we should directly set the `docIds` (similar to `docIds1`) instead of creating an `OrDocIdIterator` because that is more clear, and not mixing the scope of this test.
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/NotOperatorQueriesTest.java
##########
@@ -0,0 +1,225 @@
+/**
+ * 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.queries;
+
+import java.io.File;
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.core.common.Operator;
+import org.apache.pinot.core.operator.blocks.IntermediateResultsBlock;
+import org.apache.pinot.segment.local.indexsegment.immutable.ImmutableSegmentLoader;
+import org.apache.pinot.segment.local.segment.creator.impl.SegmentIndexCreationDriverImpl;
+import org.apache.pinot.segment.local.segment.index.loader.IndexLoadingConfig;
+import org.apache.pinot.segment.local.segment.readers.GenericRowRecordReader;
+import org.apache.pinot.segment.spi.ImmutableSegment;
+import org.apache.pinot.segment.spi.IndexSegment;
+import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
+import org.apache.pinot.spi.config.table.FSTType;
+import org.apache.pinot.spi.config.table.FieldConfig;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.data.readers.RecordReader;
+import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+
+public class NotOperatorQueriesTest extends BaseQueriesTest {
+ private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(), "NotOperatorQueriesTest");
+ private static final String TABLE_NAME = "MyTable";
+ private static final String SEGMENT_NAME = "testSegment";
+ private static final String FIRST_INT_COL_NAME = "FIRST_INT_COL";
+ private static final String SECOND_INT_COL_NAME = "SECOND_INT_COL";
+ private static final String DOMAIN_NAMES_COL = "DOMAIN_NAMES";
+ private static final Integer INT_BASE_VALUE = 1000;
+ private static final Integer NUM_ROWS = 1024;
+
+ private IndexSegment _indexSegment;
+ private List<IndexSegment> _indexSegments;
+
+ @Override
+ protected String getFilter() {
+ return "";
+ }
+
+ @Override
+ protected IndexSegment getIndexSegment() {
+ return _indexSegment;
+ }
+
+ @Override
+ protected List<IndexSegment> getIndexSegments() {
+ return _indexSegments;
+ }
+
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ FileUtils.deleteQuietly(INDEX_DIR);
+
+ List<IndexSegment> segments = new ArrayList<>();
+ buildSegment();
+ IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig();
+ Set<String> invertedIndexCols = new HashSet<>();
+ invertedIndexCols.add(FIRST_INT_COL_NAME);
+ indexLoadingConfig.setInvertedIndexColumns(invertedIndexCols);
+ Set<String> fstIndexCols = new HashSet<>();
+ fstIndexCols.add(DOMAIN_NAMES_COL);
+ indexLoadingConfig.setFSTIndexColumns(fstIndexCols);
+ indexLoadingConfig.setFSTIndexType(FSTType.LUCENE);
+ ImmutableSegment segment = ImmutableSegmentLoader.load(new File(INDEX_DIR, SEGMENT_NAME), indexLoadingConfig);
+ segments.add(segment);
+
+ _indexSegment = segment;
+ _indexSegments = segments;
+ }
+
+ private List<String> getDomainNames() {
+ return Arrays.asList("www.domain1.com", "www.domain1.co.ab", "www.domain1.co.bc", "www.domain1.co.cd",
+ "www.sd.domain1.com", "www.sd.domain1.co.ab", "www.sd.domain1.co.bc", "www.sd.domain1.co.cd", "www.domain2.com",
+ "www.domain2.co.ab", "www.domain2.co.bc", "www.domain2.co.cd", "www.sd.domain2.com", "www.sd.domain2.co.ab",
+ "www.sd.domain2.co.bc", "www.sd.domain2.co.cd");
+ }
+
+ private List<GenericRow> createTestData(int numRows) {
+ List<GenericRow> rows = new ArrayList<>();
+ List<String> domainNames = getDomainNames();
+ for (int i = 0; i < numRows; i++) {
+ String domain = domainNames.get(i % domainNames.size());
+ GenericRow row = new GenericRow();
+ row.putField(FIRST_INT_COL_NAME, i);
+ row.putField(SECOND_INT_COL_NAME, INT_BASE_VALUE + i);
+ row.putField(DOMAIN_NAMES_COL, domain);
+ rows.add(row);
+ }
+ return rows;
+ }
+
+ private void buildSegment()
+ throws Exception {
+ List<GenericRow> rows = createTestData(NUM_ROWS);
+ List<FieldConfig> fieldConfigs = new ArrayList<>();
+ fieldConfigs.add(
+ new FieldConfig(DOMAIN_NAMES_COL, FieldConfig.EncodingType.DICTIONARY, FieldConfig.IndexType.FST, null, null));
+ fieldConfigs.add(
+ new FieldConfig(FIRST_INT_COL_NAME, FieldConfig.EncodingType.DICTIONARY, FieldConfig.IndexType.INVERTED, null,
+ null));
+ TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME)
+ .setInvertedIndexColumns(Arrays.asList(FIRST_INT_COL_NAME)).setFieldConfigList(fieldConfigs).build();
+ Schema schema = new Schema.SchemaBuilder().setSchemaName(TABLE_NAME)
+ .addSingleValueDimension(FIRST_INT_COL_NAME, FieldSpec.DataType.INT)
+ .addSingleValueDimension(DOMAIN_NAMES_COL, FieldSpec.DataType.STRING)
+ .addMetric(SECOND_INT_COL_NAME, FieldSpec.DataType.INT).build();
+ SegmentGeneratorConfig config = new SegmentGeneratorConfig(tableConfig, schema);
+ config.setOutDir(INDEX_DIR.getPath());
+ config.setTableName(TABLE_NAME);
+ config.setSegmentName(SEGMENT_NAME);
+
+ SegmentIndexCreationDriverImpl driver = new SegmentIndexCreationDriverImpl();
+ try (RecordReader recordReader = new GenericRowRecordReader(rows)) {
+ driver.init(config, recordReader);
+ driver.build();
+ }
+ }
+
+ @AfterClass
+ public void tearDown() {
+ _indexSegment.destroy();
+ FileUtils.deleteQuietly(INDEX_DIR);
+ }
+
+ private void testSelectionResults(String query, int expectedResultSize, List<Serializable[]> expectedResults) {
+ Operator<IntermediateResultsBlock> operator = getOperatorForSqlQuery(query);
+ IntermediateResultsBlock operatorResult = operator.nextBlock();
+ List<Object[]> resultset = (List<Object[]>) operatorResult.getSelectionResult();
+ Assert.assertNotNull(resultset);
+ Assert.assertEquals(resultset.size(), expectedResultSize);
+ if (expectedResults != null) {
+ for (int i = 0; i < expectedResultSize; i++) {
+ Object[] actualRow = resultset.get(i);
+ Object[] expectedRow = expectedResults.get(i);
+ Assert.assertEquals(actualRow.length, expectedRow.length);
+ for (int j = 0; j < actualRow.length; j++) {
+ Object actualColValue = actualRow[j];
+ Object expectedColValue = expectedRow[j];
+ Assert.assertEquals(actualColValue, expectedColValue);
+ }
+ }
+ }
+ }
+
+ @Test
+ public void testLikeBasedNotOperator() {
+ String query =
+ "SELECT FIRST_INT_COL, SECOND_INT_COL FROM MyTable WHERE NOT REGEXP_LIKE(DOMAIN_NAMES, 'www.domain1.*') LIMIT "
+ + "50000";
+ testSelectionResults(query, 768, null);
+
+ query = "SELECT FIRST_INT_COL, SECOND_INT_COL FROM MyTable WHERE NOT REGEXP_LIKE(DOMAIN_NAMES, 'www.sd.domain1.*') "
+ + "LIMIT 50000";
+ testSelectionResults(query, 768, null);
+
+ query =
+ "SELECT FIRST_INT_COL, SECOND_INT_COL FROM MyTable WHERE NOT REGEXP_LIKE(DOMAIN_NAMES, '.*domain1.*') LIMIT "
+ + "50000";
+ testSelectionResults(query, 512, null);
+
+ query = "SELECT FIRST_INT_COL, SECOND_INT_COL FROM MyTable WHERE NOT REGEXP_LIKE(DOMAIN_NAMES, '.*domain.*') LIMIT "
+ + "50000";
+ testSelectionResults(query, 0, null);
+
+ query =
+ "SELECT FIRST_INT_COL, SECOND_INT_COL FROM MyTable WHERE NOT REGEXP_LIKE(DOMAIN_NAMES, '.*com') LIMIT 50000";
+ testSelectionResults(query, 768, null);
+ }
+
+ @Test
+ public void testWeirdPredicates() {
+ String query = "SELECT FIRST_INT_COL, SECOND_INT_COL FROM MyTable WHERE NOT FIRST_INT_COL = 5 LIMIT " + "50000";
Review comment:
(minor) Remove the unnecessary `+`, same for other places
```suggestion
String query = "SELECT FIRST_INT_COL, SECOND_INT_COL FROM MyTable WHERE NOT FIRST_INT_COL = 5 LIMIT 50000";
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #8148: Implement NOT Operator
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #8148:
URL: https://github.com/apache/pinot/pull/8148#discussion_r806346742
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIteratorTest.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+
+
+public class NotDocIdIteratorTest {
+ @Test
+ public void testNotDocIdIterator() {
+ // OR result: [0, 1, 2, 4, 5, 6, 8, 10, 13, 15, 16, 17, 18, 19, 20]
+ int[] docIds1 = new int[]{1, 4, 6, 10, 15, 17, 18, 20};
+ int[] docIds2 = new int[]{0, 1, 5, 8, 15, 18};
+ int[] docIds3 = new int[]{1, 2, 6, 13, 16, 19};
+ int[] docIds4 = new int[]{0, 1, 2, 3, 4, 5};
+
+ MutableRoaringBitmap bitmap1 = new MutableRoaringBitmap();
+ bitmap1.add(docIds1);
+ MutableRoaringBitmap bitmap2 = new MutableRoaringBitmap();
+ bitmap2.add(docIds2);
+ MutableRoaringBitmap bitmap3 = new MutableRoaringBitmap();
+ bitmap3.add(docIds3);
+ MutableRoaringBitmap bitmap4 = new MutableRoaringBitmap();
+ bitmap4.add(docIds4);
+ OrDocIdIterator orDocIdIterator = new OrDocIdIterator(new BlockDocIdIterator[]{
Review comment:
Why do we need to build this `OrDocIdIterator`? We don't want to test `OrDocIdIterator` in this test
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIteratorTest.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+
+
+public class NotDocIdIteratorTest {
+ @Test
+ public void testNotDocIdIterator() {
+ // OR result: [0, 1, 2, 4, 5, 6, 8, 10, 13, 15, 16, 17, 18, 19, 20]
+ int[] docIds1 = new int[]{1, 4, 6, 10, 15, 17, 18, 20};
+ int[] docIds2 = new int[]{0, 1, 5, 8, 15, 18};
+ int[] docIds3 = new int[]{1, 2, 6, 13, 16, 19};
+ int[] docIds4 = new int[]{0, 1, 2, 3, 4, 5};
+
+ MutableRoaringBitmap bitmap1 = new MutableRoaringBitmap();
+ bitmap1.add(docIds1);
+ MutableRoaringBitmap bitmap2 = new MutableRoaringBitmap();
+ bitmap2.add(docIds2);
+ MutableRoaringBitmap bitmap3 = new MutableRoaringBitmap();
+ bitmap3.add(docIds3);
+ MutableRoaringBitmap bitmap4 = new MutableRoaringBitmap();
+ bitmap4.add(docIds4);
+ OrDocIdIterator orDocIdIterator = new OrDocIdIterator(new BlockDocIdIterator[]{
+ new RangelessBitmapDocIdIterator(bitmap1), new RangelessBitmapDocIdIterator(
+ bitmap2), new RangelessBitmapDocIdIterator(bitmap3)
+ });
+ NotDocIdIterator notDocIdIterator = new NotDocIdIterator(new RangelessBitmapDocIdIterator(bitmap1), 25);
+
+ assertEquals(notDocIdIterator.advance(1), 2);
+ assertEquals(notDocIdIterator.next(), 3);
+ assertEquals(notDocIdIterator.next(), 5);
+ assertEquals(notDocIdIterator.advance(7), 8);
Review comment:
(major) Per the contract, this should return 7
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIterator.java
##########
@@ -0,0 +1,97 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+
+
+/**
+ * The iterator performs a linear pass through the underlying child iterator and returns
+ * the complement of the result set.
+ */
+public class NotDocIdIterator implements BlockDocIdIterator {
+ private final BlockDocIdIterator _childDocIdIterator;
+ private final int _numDocs;
+ private int _nextDocId;
+ private int _nextNonMatchingDocId;
+
+ public NotDocIdIterator(BlockDocIdIterator childDocIdIterator, int numDocs) {
+ _childDocIdIterator = childDocIdIterator;
+ _nextDocId = 0;
+
+ int currentDocIdFromChildIterator = childDocIdIterator.next();
+ _nextNonMatchingDocId = currentDocIdFromChildIterator == Constants.EOF ? numDocs : currentDocIdFromChildIterator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public int next() {
+ while (_nextDocId == _nextNonMatchingDocId) {
+ _nextDocId = _nextNonMatchingDocId + 1;
+
+ int nextMatchingDocId = _childDocIdIterator.next();
+
+ if (nextMatchingDocId == Constants.EOF) {
+ _nextNonMatchingDocId = _numDocs;
+ } else {
+ _nextNonMatchingDocId = nextMatchingDocId;
+ }
+ }
+
+ if (_nextDocId >= _numDocs) {
+ return Constants.EOF;
+ }
+
+ return _nextDocId++;
+ }
+
+ @Override
+ public int advance(int targetDocId) {
+ if (targetDocId == Constants.EOF || targetDocId > _numDocs) {
+ return Constants.EOF;
+ }
+
+ if (targetDocId < _nextDocId) {
+ return _nextDocId;
+ }
+
+ _nextDocId = targetDocId + 1;
+
+ int upperLimit = findUpperLimitGreaterThanDocId(targetDocId);
+
+ if (upperLimit == Constants.EOF) {
+ _nextNonMatchingDocId = _numDocs;
+ } else {
+ _nextNonMatchingDocId = upperLimit;
+ }
+
+ return _nextDocId++;
+ }
+
+ private int findUpperLimitGreaterThanDocId(int currentDocId) {
+ int result = _childDocIdIterator.advance(currentDocId);
Review comment:
(major) need to check `_nextNonMatchingDocId < currentDocId` before advancing the pointer
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIterator.java
##########
@@ -0,0 +1,97 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+
+
+/**
+ * The iterator performs a linear pass through the underlying child iterator and returns
+ * the complement of the result set.
+ */
+public class NotDocIdIterator implements BlockDocIdIterator {
+ private final BlockDocIdIterator _childDocIdIterator;
+ private final int _numDocs;
+ private int _nextDocId;
+ private int _nextNonMatchingDocId;
+
+ public NotDocIdIterator(BlockDocIdIterator childDocIdIterator, int numDocs) {
+ _childDocIdIterator = childDocIdIterator;
+ _nextDocId = 0;
+
+ int currentDocIdFromChildIterator = childDocIdIterator.next();
+ _nextNonMatchingDocId = currentDocIdFromChildIterator == Constants.EOF ? numDocs : currentDocIdFromChildIterator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public int next() {
+ while (_nextDocId == _nextNonMatchingDocId) {
+ _nextDocId = _nextNonMatchingDocId + 1;
+
+ int nextMatchingDocId = _childDocIdIterator.next();
+
+ if (nextMatchingDocId == Constants.EOF) {
+ _nextNonMatchingDocId = _numDocs;
+ } else {
+ _nextNonMatchingDocId = nextMatchingDocId;
+ }
+ }
+
+ if (_nextDocId >= _numDocs) {
+ return Constants.EOF;
+ }
+
+ return _nextDocId++;
+ }
+
+ @Override
+ public int advance(int targetDocId) {
+ if (targetDocId == Constants.EOF || targetDocId > _numDocs) {
+ return Constants.EOF;
+ }
+
+ if (targetDocId < _nextDocId) {
+ return _nextDocId;
+ }
Review comment:
These checks are redundant because by contract, `targetDocId >= _nextDocId && targetDocId < _numDocs`
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIterator.java
##########
@@ -0,0 +1,97 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+
+
+/**
+ * The iterator performs a linear pass through the underlying child iterator and returns
+ * the complement of the result set.
+ */
+public class NotDocIdIterator implements BlockDocIdIterator {
+ private final BlockDocIdIterator _childDocIdIterator;
+ private final int _numDocs;
+ private int _nextDocId;
+ private int _nextNonMatchingDocId;
+
+ public NotDocIdIterator(BlockDocIdIterator childDocIdIterator, int numDocs) {
+ _childDocIdIterator = childDocIdIterator;
+ _nextDocId = 0;
+
+ int currentDocIdFromChildIterator = childDocIdIterator.next();
+ _nextNonMatchingDocId = currentDocIdFromChildIterator == Constants.EOF ? numDocs : currentDocIdFromChildIterator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public int next() {
+ while (_nextDocId == _nextNonMatchingDocId) {
+ _nextDocId = _nextNonMatchingDocId + 1;
+
+ int nextMatchingDocId = _childDocIdIterator.next();
+
+ if (nextMatchingDocId == Constants.EOF) {
+ _nextNonMatchingDocId = _numDocs;
+ } else {
+ _nextNonMatchingDocId = nextMatchingDocId;
+ }
+ }
+
+ if (_nextDocId >= _numDocs) {
+ return Constants.EOF;
+ }
+
+ return _nextDocId++;
+ }
+
+ @Override
+ public int advance(int targetDocId) {
+ if (targetDocId == Constants.EOF || targetDocId > _numDocs) {
+ return Constants.EOF;
+ }
+
+ if (targetDocId < _nextDocId) {
+ return _nextDocId;
+ }
+
+ _nextDocId = targetDocId + 1;
Review comment:
(major) This will skip the `targetDocId`
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIteratorTest.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+
+
+public class NotDocIdIteratorTest {
+ @Test
+ public void testNotDocIdIterator() {
+ // OR result: [0, 1, 2, 4, 5, 6, 8, 10, 13, 15, 16, 17, 18, 19, 20]
+ int[] docIds1 = new int[]{1, 4, 6, 10, 15, 17, 18, 20};
+ int[] docIds2 = new int[]{0, 1, 5, 8, 15, 18};
+ int[] docIds3 = new int[]{1, 2, 6, 13, 16, 19};
+ int[] docIds4 = new int[]{0, 1, 2, 3, 4, 5};
+
+ MutableRoaringBitmap bitmap1 = new MutableRoaringBitmap();
+ bitmap1.add(docIds1);
+ MutableRoaringBitmap bitmap2 = new MutableRoaringBitmap();
+ bitmap2.add(docIds2);
+ MutableRoaringBitmap bitmap3 = new MutableRoaringBitmap();
+ bitmap3.add(docIds3);
+ MutableRoaringBitmap bitmap4 = new MutableRoaringBitmap();
+ bitmap4.add(docIds4);
+ OrDocIdIterator orDocIdIterator = new OrDocIdIterator(new BlockDocIdIterator[]{
+ new RangelessBitmapDocIdIterator(bitmap1), new RangelessBitmapDocIdIterator(
+ bitmap2), new RangelessBitmapDocIdIterator(bitmap3)
+ });
+ NotDocIdIterator notDocIdIterator = new NotDocIdIterator(new RangelessBitmapDocIdIterator(bitmap1), 25);
+
+ assertEquals(notDocIdIterator.advance(1), 2);
+ assertEquals(notDocIdIterator.next(), 3);
+ assertEquals(notDocIdIterator.next(), 5);
+ assertEquals(notDocIdIterator.advance(7), 8);
+ assertEquals(notDocIdIterator.advance(13), 14);
Review comment:
(major) This should return 13, same for other places
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/filter/FilterOperatorUtils.java
##########
@@ -165,6 +179,9 @@ int getPriority(BaseFilterOperator filterOperator) {
if (filterOperator instanceof OrFilterOperator) {
return 4;
}
+ if (filterOperator instanceof NotFilterOperator) {
+ return getPriority((BaseFilterOperator) filterOperator.getChildOperators().get(0));
Review comment:
(minor) We can add a `getChildFilterOperator()` in `NotFilterOperator` which can save an unnecessary array allocation
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/dociditerators/NotDocIdIteratorTest.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.dociditerators;
+
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+
+
+public class NotDocIdIteratorTest {
+ @Test
+ public void testNotDocIdIterator() {
+ // OR result: [0, 1, 2, 4, 5, 6, 8, 10, 13, 15, 16, 17, 18, 19, 20]
+ int[] docIds1 = new int[]{1, 4, 6, 10, 15, 17, 18, 20};
+ int[] docIds2 = new int[]{0, 1, 5, 8, 15, 18};
+ int[] docIds3 = new int[]{1, 2, 6, 13, 16, 19};
+ int[] docIds4 = new int[]{0, 1, 2, 3, 4, 5};
+
+ MutableRoaringBitmap bitmap1 = new MutableRoaringBitmap();
+ bitmap1.add(docIds1);
+ MutableRoaringBitmap bitmap2 = new MutableRoaringBitmap();
+ bitmap2.add(docIds2);
+ MutableRoaringBitmap bitmap3 = new MutableRoaringBitmap();
+ bitmap3.add(docIds3);
+ MutableRoaringBitmap bitmap4 = new MutableRoaringBitmap();
+ bitmap4.add(docIds4);
+ OrDocIdIterator orDocIdIterator = new OrDocIdIterator(new BlockDocIdIterator[]{
Review comment:
Understood. What I'm saying is that we should directly set the `docIds` (similar to `docIds1`) instead of creating an `OrDocIdIterator` because that is more clear, and not mixing the scope of this test.
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/NotOperatorQueriesTest.java
##########
@@ -0,0 +1,225 @@
+/**
+ * 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.queries;
+
+import java.io.File;
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.core.common.Operator;
+import org.apache.pinot.core.operator.blocks.IntermediateResultsBlock;
+import org.apache.pinot.segment.local.indexsegment.immutable.ImmutableSegmentLoader;
+import org.apache.pinot.segment.local.segment.creator.impl.SegmentIndexCreationDriverImpl;
+import org.apache.pinot.segment.local.segment.index.loader.IndexLoadingConfig;
+import org.apache.pinot.segment.local.segment.readers.GenericRowRecordReader;
+import org.apache.pinot.segment.spi.ImmutableSegment;
+import org.apache.pinot.segment.spi.IndexSegment;
+import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
+import org.apache.pinot.spi.config.table.FSTType;
+import org.apache.pinot.spi.config.table.FieldConfig;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.data.readers.RecordReader;
+import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+
+public class NotOperatorQueriesTest extends BaseQueriesTest {
+ private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(), "NotOperatorQueriesTest");
+ private static final String TABLE_NAME = "MyTable";
+ private static final String SEGMENT_NAME = "testSegment";
+ private static final String FIRST_INT_COL_NAME = "FIRST_INT_COL";
+ private static final String SECOND_INT_COL_NAME = "SECOND_INT_COL";
+ private static final String DOMAIN_NAMES_COL = "DOMAIN_NAMES";
+ private static final Integer INT_BASE_VALUE = 1000;
+ private static final Integer NUM_ROWS = 1024;
+
+ private IndexSegment _indexSegment;
+ private List<IndexSegment> _indexSegments;
+
+ @Override
+ protected String getFilter() {
+ return "";
+ }
+
+ @Override
+ protected IndexSegment getIndexSegment() {
+ return _indexSegment;
+ }
+
+ @Override
+ protected List<IndexSegment> getIndexSegments() {
+ return _indexSegments;
+ }
+
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ FileUtils.deleteQuietly(INDEX_DIR);
+
+ List<IndexSegment> segments = new ArrayList<>();
+ buildSegment();
+ IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig();
+ Set<String> invertedIndexCols = new HashSet<>();
+ invertedIndexCols.add(FIRST_INT_COL_NAME);
+ indexLoadingConfig.setInvertedIndexColumns(invertedIndexCols);
+ Set<String> fstIndexCols = new HashSet<>();
+ fstIndexCols.add(DOMAIN_NAMES_COL);
+ indexLoadingConfig.setFSTIndexColumns(fstIndexCols);
+ indexLoadingConfig.setFSTIndexType(FSTType.LUCENE);
+ ImmutableSegment segment = ImmutableSegmentLoader.load(new File(INDEX_DIR, SEGMENT_NAME), indexLoadingConfig);
+ segments.add(segment);
+
+ _indexSegment = segment;
+ _indexSegments = segments;
+ }
+
+ private List<String> getDomainNames() {
+ return Arrays.asList("www.domain1.com", "www.domain1.co.ab", "www.domain1.co.bc", "www.domain1.co.cd",
+ "www.sd.domain1.com", "www.sd.domain1.co.ab", "www.sd.domain1.co.bc", "www.sd.domain1.co.cd", "www.domain2.com",
+ "www.domain2.co.ab", "www.domain2.co.bc", "www.domain2.co.cd", "www.sd.domain2.com", "www.sd.domain2.co.ab",
+ "www.sd.domain2.co.bc", "www.sd.domain2.co.cd");
+ }
+
+ private List<GenericRow> createTestData(int numRows) {
+ List<GenericRow> rows = new ArrayList<>();
+ List<String> domainNames = getDomainNames();
+ for (int i = 0; i < numRows; i++) {
+ String domain = domainNames.get(i % domainNames.size());
+ GenericRow row = new GenericRow();
+ row.putField(FIRST_INT_COL_NAME, i);
+ row.putField(SECOND_INT_COL_NAME, INT_BASE_VALUE + i);
+ row.putField(DOMAIN_NAMES_COL, domain);
+ rows.add(row);
+ }
+ return rows;
+ }
+
+ private void buildSegment()
+ throws Exception {
+ List<GenericRow> rows = createTestData(NUM_ROWS);
+ List<FieldConfig> fieldConfigs = new ArrayList<>();
+ fieldConfigs.add(
+ new FieldConfig(DOMAIN_NAMES_COL, FieldConfig.EncodingType.DICTIONARY, FieldConfig.IndexType.FST, null, null));
+ fieldConfigs.add(
+ new FieldConfig(FIRST_INT_COL_NAME, FieldConfig.EncodingType.DICTIONARY, FieldConfig.IndexType.INVERTED, null,
+ null));
+ TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME)
+ .setInvertedIndexColumns(Arrays.asList(FIRST_INT_COL_NAME)).setFieldConfigList(fieldConfigs).build();
+ Schema schema = new Schema.SchemaBuilder().setSchemaName(TABLE_NAME)
+ .addSingleValueDimension(FIRST_INT_COL_NAME, FieldSpec.DataType.INT)
+ .addSingleValueDimension(DOMAIN_NAMES_COL, FieldSpec.DataType.STRING)
+ .addMetric(SECOND_INT_COL_NAME, FieldSpec.DataType.INT).build();
+ SegmentGeneratorConfig config = new SegmentGeneratorConfig(tableConfig, schema);
+ config.setOutDir(INDEX_DIR.getPath());
+ config.setTableName(TABLE_NAME);
+ config.setSegmentName(SEGMENT_NAME);
+
+ SegmentIndexCreationDriverImpl driver = new SegmentIndexCreationDriverImpl();
+ try (RecordReader recordReader = new GenericRowRecordReader(rows)) {
+ driver.init(config, recordReader);
+ driver.build();
+ }
+ }
+
+ @AfterClass
+ public void tearDown() {
+ _indexSegment.destroy();
+ FileUtils.deleteQuietly(INDEX_DIR);
+ }
+
+ private void testSelectionResults(String query, int expectedResultSize, List<Serializable[]> expectedResults) {
+ Operator<IntermediateResultsBlock> operator = getOperatorForSqlQuery(query);
+ IntermediateResultsBlock operatorResult = operator.nextBlock();
+ List<Object[]> resultset = (List<Object[]>) operatorResult.getSelectionResult();
+ Assert.assertNotNull(resultset);
+ Assert.assertEquals(resultset.size(), expectedResultSize);
+ if (expectedResults != null) {
+ for (int i = 0; i < expectedResultSize; i++) {
+ Object[] actualRow = resultset.get(i);
+ Object[] expectedRow = expectedResults.get(i);
+ Assert.assertEquals(actualRow.length, expectedRow.length);
+ for (int j = 0; j < actualRow.length; j++) {
+ Object actualColValue = actualRow[j];
+ Object expectedColValue = expectedRow[j];
+ Assert.assertEquals(actualColValue, expectedColValue);
+ }
+ }
+ }
+ }
+
+ @Test
+ public void testLikeBasedNotOperator() {
+ String query =
+ "SELECT FIRST_INT_COL, SECOND_INT_COL FROM MyTable WHERE NOT REGEXP_LIKE(DOMAIN_NAMES, 'www.domain1.*') LIMIT "
+ + "50000";
+ testSelectionResults(query, 768, null);
+
+ query = "SELECT FIRST_INT_COL, SECOND_INT_COL FROM MyTable WHERE NOT REGEXP_LIKE(DOMAIN_NAMES, 'www.sd.domain1.*') "
+ + "LIMIT 50000";
+ testSelectionResults(query, 768, null);
+
+ query =
+ "SELECT FIRST_INT_COL, SECOND_INT_COL FROM MyTable WHERE NOT REGEXP_LIKE(DOMAIN_NAMES, '.*domain1.*') LIMIT "
+ + "50000";
+ testSelectionResults(query, 512, null);
+
+ query = "SELECT FIRST_INT_COL, SECOND_INT_COL FROM MyTable WHERE NOT REGEXP_LIKE(DOMAIN_NAMES, '.*domain.*') LIMIT "
+ + "50000";
+ testSelectionResults(query, 0, null);
+
+ query =
+ "SELECT FIRST_INT_COL, SECOND_INT_COL FROM MyTable WHERE NOT REGEXP_LIKE(DOMAIN_NAMES, '.*com') LIMIT 50000";
+ testSelectionResults(query, 768, null);
+ }
+
+ @Test
+ public void testWeirdPredicates() {
+ String query = "SELECT FIRST_INT_COL, SECOND_INT_COL FROM MyTable WHERE NOT FIRST_INT_COL = 5 LIMIT " + "50000";
Review comment:
(minor) Remove the unnecessary `+`, same for other places
```suggestion
String query = "SELECT FIRST_INT_COL, SECOND_INT_COL FROM MyTable WHERE NOT FIRST_INT_COL = 5 LIMIT 50000";
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org