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/04/12 11:58:58 UTC
[GitHub] [pinot] richardstartin opened a new pull request, #8513: push counts down to range index when v2 is in use
richardstartin opened a new pull request, #8513:
URL: https://github.com/apache/pinot/pull/8513
This extends the fast count mechanism to v2 range indexes now that the underlying data structure supports counting. Also does some code cleanup in this area.
--
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] walterddr commented on a diff in pull request #8513: push counts down to range index when v2 is in use
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #8513:
URL: https://github.com/apache/pinot/pull/8513#discussion_r850830798
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/RangeIndexBasedFilterOperator.java:
##########
@@ -145,10 +127,178 @@ public List<Operator> getChildOperators() {
@Override
public String toExplainString() {
- StringBuilder stringBuilder = new StringBuilder(EXPLAIN_NAME).append("(indexLookUp:range_index");
- stringBuilder.append(",operator:").append(_rangePredicateEvaluator.getPredicateType());
- stringBuilder.append(",predicate:").append(_rangePredicateEvaluator.getPredicate().toString());
- return stringBuilder.append(')').toString();
+ return EXPLAIN_NAME + "(indexLookUp:range_index"
+ + ",operator:" + _rangePredicateEvaluator.getPredicateType()
+ + ",predicate:" + _rangePredicateEvaluator.getPredicate().toString()
+ + ')';
+ }
+
+ interface RangeEvaluator {
+ static RangeEvaluator of(RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader,
+ PredicateEvaluator predicateEvaluator) {
+ if (predicateEvaluator.isDictionaryBased()) {
+ return new IntRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ } else {
+ switch (predicateEvaluator.getDataType()) {
+ case INT:
+ return new IntRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case LONG:
+ return new LongRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case FLOAT:
+ return new FloatRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case DOUBLE:
+ return new DoubleRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ default:
+ throw new IllegalStateException("String and Bytes data type not supported for Range Indexing");
+ }
+ }
+ }
+
+ ImmutableRoaringBitmap getMatchingDocIds();
+
+ ImmutableRoaringBitmap getPartiallyMatchingDocIds();
+
+ int getMatchingDocCount();
+
+ boolean isExact();
+ }
+
+ private static final class IntRangeEvaluator implements RangeEvaluator {
+ final RangeIndexReader<ImmutableRoaringBitmap> _rangeIndexReader;
+ final int _min;
+ final int _max;
+
+ IntRangeEvaluator(
+ RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader, PredicateEvaluator predicateEvaluator) {
+ _rangeIndexReader = rangeIndexReader;
+ if (predicateEvaluator instanceof SortedDictionaryBasedRangePredicateEvaluator) {
+ // NOTE: End dictionary id is exclusive in OfflineDictionaryBasedRangePredicateEvaluator.
+ _min = ((SortedDictionaryBasedRangePredicateEvaluator) predicateEvaluator).getStartDictId();
+ _max = ((SortedDictionaryBasedRangePredicateEvaluator) predicateEvaluator).getEndDictId() - 1;
Review Comment:
why is the DictionaryBasedRangePredicateEvaluator branch only exist in the IntRangeEvaluate?
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/RangeIndexReader.java:
##########
@@ -26,6 +26,51 @@
* @param <T>
*/
public interface RangeIndexReader<T> extends Closeable {
+
+ /**
+ * @return true if the results are exact and don't need refinement by scanning.
+ * This means {@see getPartiallyMatchingDocIds} will return null.
+ */
+ default boolean isExact() {
+ return true;
Review Comment:
I would not make this a default. shouldn't the implementer decide?
--
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] richardstartin commented on a diff in pull request #8513: push counts down to range index when v2 is in use
Posted by GitBox <gi...@apache.org>.
richardstartin commented on code in PR #8513:
URL: https://github.com/apache/pinot/pull/8513#discussion_r850854605
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/RangeIndexReader.java:
##########
@@ -26,6 +26,51 @@
* @param <T>
*/
public interface RangeIndexReader<T> extends Closeable {
+
+ /**
+ * @return true if the results are exact and don't need refinement by scanning.
+ * This means {@see getPartiallyMatchingDocIds} will return null.
+ */
+ default boolean isExact() {
+ return true;
Review Comment:
> is the intent here to not allow any non-exact impl in the future?
exactly, you need to opt out of being exact and have a good reason
--
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] richardstartin commented on a diff in pull request #8513: push counts down to range index when v2 is in use
Posted by GitBox <gi...@apache.org>.
richardstartin commented on code in PR #8513:
URL: https://github.com/apache/pinot/pull/8513#discussion_r853088744
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/creator/BitSlicedIndexCreatorTest.java:
##########
@@ -192,7 +194,9 @@ private void testLong(Dataset<long[]> dataset)
for (long quantile : dataset.quantiles()) {
ImmutableRoaringBitmap reference = dataset.scan(prev, quantile);
ImmutableRoaringBitmap result = reader.getMatchingDocIds(prev, quantile);
+ int resultCount = reader.getMatchingDocCount(prev, quantile);
assertEquals(result, reference);
+ assertEquals(resultCount, result == null ? 0 : result.getCardinality());
Review Comment:
It's nullable and this is just to avoid an IDE warning, I can replace it with `Objects.requireNonNull` (but this is the same thing inlined).
--
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] richardstartin commented on a diff in pull request #8513: push counts down to range index when v2 is in use
Posted by GitBox <gi...@apache.org>.
richardstartin commented on code in PR #8513:
URL: https://github.com/apache/pinot/pull/8513#discussion_r850843713
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/RangeIndexBasedFilterOperator.java:
##########
@@ -145,10 +127,178 @@ public List<Operator> getChildOperators() {
@Override
public String toExplainString() {
- StringBuilder stringBuilder = new StringBuilder(EXPLAIN_NAME).append("(indexLookUp:range_index");
- stringBuilder.append(",operator:").append(_rangePredicateEvaluator.getPredicateType());
- stringBuilder.append(",predicate:").append(_rangePredicateEvaluator.getPredicate().toString());
- return stringBuilder.append(')').toString();
+ return EXPLAIN_NAME + "(indexLookUp:range_index"
+ + ",operator:" + _rangePredicateEvaluator.getPredicateType()
+ + ",predicate:" + _rangePredicateEvaluator.getPredicate().toString()
+ + ')';
+ }
+
+ interface RangeEvaluator {
+ static RangeEvaluator of(RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader,
+ PredicateEvaluator predicateEvaluator) {
+ if (predicateEvaluator.isDictionaryBased()) {
+ return new IntRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ } else {
+ switch (predicateEvaluator.getDataType()) {
+ case INT:
+ return new IntRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case LONG:
+ return new LongRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case FLOAT:
+ return new FloatRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case DOUBLE:
+ return new DoubleRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ default:
+ throw new IllegalStateException("String and Bytes data type not supported for Range Indexing");
+ }
+ }
+ }
+
+ ImmutableRoaringBitmap getMatchingDocIds();
+
+ ImmutableRoaringBitmap getPartiallyMatchingDocIds();
+
+ int getMatchingDocCount();
+
+ boolean isExact();
+ }
+
+ private static final class IntRangeEvaluator implements RangeEvaluator {
+ final RangeIndexReader<ImmutableRoaringBitmap> _rangeIndexReader;
+ final int _min;
+ final int _max;
+
+ IntRangeEvaluator(
+ RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader, PredicateEvaluator predicateEvaluator) {
+ _rangeIndexReader = rangeIndexReader;
+ if (predicateEvaluator instanceof SortedDictionaryBasedRangePredicateEvaluator) {
+ // NOTE: End dictionary id is exclusive in OfflineDictionaryBasedRangePredicateEvaluator.
+ _min = ((SortedDictionaryBasedRangePredicateEvaluator) predicateEvaluator).getStartDictId();
+ _max = ((SortedDictionaryBasedRangePredicateEvaluator) predicateEvaluator).getEndDictId() - 1;
Review Comment:
the purpose of this is to abstract over the range bound types - int, long, etc. Only raw int and dictionary have int bounds.
--
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] richardstartin merged pull request #8513: Enable range index on raw columns and push counts down to range index when v2 is in use
Posted by GitBox <gi...@apache.org>.
richardstartin merged PR #8513:
URL: https://github.com/apache/pinot/pull/8513
--
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] richardstartin commented on a diff in pull request #8513: Enable range index on raw columns and push counts down to range index when v2 is in use
Posted by GitBox <gi...@apache.org>.
richardstartin commented on code in PR #8513:
URL: https://github.com/apache/pinot/pull/8513#discussion_r853305443
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/creator/BitSlicedIndexCreatorTest.java:
##########
@@ -192,7 +194,9 @@ private void testLong(Dataset<long[]> dataset)
for (long quantile : dataset.quantiles()) {
ImmutableRoaringBitmap reference = dataset.scan(prev, quantile);
ImmutableRoaringBitmap result = reader.getMatchingDocIds(prev, quantile);
+ int resultCount = reader.getMatchingDocCount(prev, quantile);
assertEquals(result, reference);
+ assertEquals(resultCount, result == null ? 0 : result.getCardinality());
Review Comment:
I haven't changed this, I would prefer the unnecessary code to the IDE warning.
--
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] richardstartin commented on a diff in pull request #8513: push counts down to range index when v2 is in use
Posted by GitBox <gi...@apache.org>.
richardstartin commented on code in PR #8513:
URL: https://github.com/apache/pinot/pull/8513#discussion_r850844404
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/RangeIndexReader.java:
##########
@@ -26,6 +26,51 @@
* @param <T>
*/
public interface RangeIndexReader<T> extends Closeable {
+
+ /**
+ * @return true if the results are exact and don't need refinement by scanning.
+ * This means {@see getPartiallyMatchingDocIds} will return null.
+ */
+ default boolean isExact() {
+ return true;
Review Comment:
There are two implementations, the first one was inexact and could be up to 10x slower than the one which is exact. If we implement another range index, we will want it to be exact.
--
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 #8513: push counts down to range index when v2 is in use
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #8513:
URL: https://github.com/apache/pinot/pull/8513#issuecomment-1096678988
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8513?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 [#8513](https://codecov.io/gh/apache/pinot/pull/8513?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (49fed85) into [master](https://codecov.io/gh/apache/pinot/commit/44da1a098fd47ffb533da6ca1386c64fb86e50a9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (44da1a0) will **decrease** coverage by `6.52%`.
> The diff coverage is `31.11%`.
```diff
@@ Coverage Diff @@
## master #8513 +/- ##
============================================
- Coverage 70.59% 64.06% -6.53%
- Complexity 4227 4292 +65
============================================
Files 1674 1630 -44
Lines 87618 85757 -1861
Branches 13272 13075 -197
============================================
- Hits 61852 54943 -6909
- Misses 21477 26829 +5352
+ Partials 4289 3985 -304
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `66.96% <31.11%> (-0.05%)` | :arrow_down: |
| unittests2 | `14.08% <0.00%> (-0.06%)` | :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/8513?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...lter/predicate/RangePredicateEvaluatorFactory.java](https://codecov.io/gh/apache/pinot/pull/8513/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvcHJlZGljYXRlL1JhbmdlUHJlZGljYXRlRXZhbHVhdG9yRmFjdG9yeS5qYXZh) | `69.26% <ø> (-14.64%)` | :arrow_down: |
| [...al/segment/index/readers/RangeIndexReaderImpl.java](https://codecov.io/gh/apache/pinot/pull/8513/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvUmFuZ2VJbmRleFJlYWRlckltcGwuamF2YQ==) | `80.21% <0.00%> (-8.81%)` | :arrow_down: |
| [...operator/filter/RangeIndexBasedFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/8513/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvUmFuZ2VJbmRleEJhc2VkRmlsdGVyT3BlcmF0b3IuamF2YQ==) | `30.26% <25.00%> (-2.43%)` | :arrow_down: |
| [...gment/index/readers/BitSlicedRangeIndexReader.java](https://codecov.io/gh/apache/pinot/pull/8513/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvQml0U2xpY2VkUmFuZ2VJbmRleFJlYWRlci5qYXZh) | `81.25% <83.33%> (+0.69%)` | :arrow_up: |
| [...not/segment/spi/index/reader/RangeIndexReader.java](https://codecov.io/gh/apache/pinot/pull/8513/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-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L3JlYWRlci9SYW5nZUluZGV4UmVhZGVyLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...va/org/apache/pinot/core/routing/RoutingTable.java](https://codecov.io/gh/apache/pinot/pull/8513/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yb3V0aW5nL1JvdXRpbmdUYWJsZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/common/config/NettyConfig.java](https://codecov.io/gh/apache/pinot/pull/8513/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL05ldHR5Q29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/8513/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/8513/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: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/8513/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [376 more](https://codecov.io/gh/apache/pinot/pull/8513/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/8513?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/8513?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 [44da1a0...49fed85](https://codecov.io/gh/apache/pinot/pull/8513?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 diff in pull request #8513: push counts down to range index when v2 is in use
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #8513:
URL: https://github.com/apache/pinot/pull/8513#discussion_r852266617
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/RangeIndexReader.java:
##########
@@ -26,6 +26,51 @@
* @param <T>
*/
public interface RangeIndexReader<T> extends Closeable {
+
+ /**
+ * @return true if the results are exact and don't need refinement by scanning.
+ * This means {@see getPartiallyMatchingDocIds} will return null.
+ */
+ default boolean isExact() {
+ return true;
+ }
Review Comment:
(minor) Add an empty line after
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/RangeIndexReader.java:
##########
@@ -26,6 +26,51 @@
* @param <T>
*/
public interface RangeIndexReader<T> extends Closeable {
+
+ /**
+ * @return true if the results are exact and don't need refinement by scanning.
+ * This means {@see getPartiallyMatchingDocIds} will return null.
+ */
+ default boolean isExact() {
+ return true;
+ }
+ /**
+ * Returns the number of docs with a value between min and max, both inclusive.
+ * Doc ids returned by this method must correspond to values which
+ * satisfy the query.
+ * @param min the inclusive lower bound.
+ * @param max the inclusive upper bound.
+ * @return the matching doc ids.
+ */
+ int getMatchingDocCount(int min, int max);
Review Comment:
(minor) Suggest renaming to `getNumMatchingDocs` to be consistent with `BaseFilterOperator`
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/creator/BitSlicedIndexCreatorTest.java:
##########
@@ -192,7 +194,9 @@ private void testLong(Dataset<long[]> dataset)
for (long quantile : dataset.quantiles()) {
ImmutableRoaringBitmap reference = dataset.scan(prev, quantile);
ImmutableRoaringBitmap result = reader.getMatchingDocIds(prev, quantile);
+ int resultCount = reader.getMatchingDocCount(prev, quantile);
assertEquals(result, reference);
+ assertEquals(resultCount, result == null ? 0 : result.getCardinality());
Review Comment:
Per the implementation seems `result` can never be `null`, and we rely on it not being `null` in `RangeIndexBasedFilterOperator.getNextBlock()` or it will throw NPE
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/RangeIndexBasedFilterOperator.java:
##########
@@ -145,10 +127,178 @@ public List<Operator> getChildOperators() {
@Override
public String toExplainString() {
- StringBuilder stringBuilder = new StringBuilder(EXPLAIN_NAME).append("(indexLookUp:range_index");
- stringBuilder.append(",operator:").append(_rangePredicateEvaluator.getPredicateType());
- stringBuilder.append(",predicate:").append(_rangePredicateEvaluator.getPredicate().toString());
- return stringBuilder.append(')').toString();
+ return EXPLAIN_NAME + "(indexLookUp:range_index"
+ + ",operator:" + _rangePredicateEvaluator.getPredicateType()
+ + ",predicate:" + _rangePredicateEvaluator.getPredicate().toString()
+ + ')';
+ }
+
+ interface RangeEvaluator {
+ static RangeEvaluator of(RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader,
+ PredicateEvaluator predicateEvaluator) {
+ if (predicateEvaluator.isDictionaryBased()) {
+ return new IntRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ } else {
+ switch (predicateEvaluator.getDataType()) {
+ case INT:
+ return new IntRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case LONG:
+ return new LongRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case FLOAT:
+ return new FloatRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case DOUBLE:
+ return new DoubleRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ default:
+ throw new IllegalStateException("String and Bytes data type not supported for Range Indexing");
+ }
+ }
+ }
+
+ ImmutableRoaringBitmap getMatchingDocIds();
+
+ ImmutableRoaringBitmap getPartiallyMatchingDocIds();
+
+ int getMatchingDocCount();
Review Comment:
(minor) Suggest renaming to `getNumMatchingDocs` to be consistent with `BaseFilterOperator`
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/RangeIndexBasedFilterOperator.java:
##########
@@ -145,10 +127,178 @@ public List<Operator> getChildOperators() {
@Override
public String toExplainString() {
- StringBuilder stringBuilder = new StringBuilder(EXPLAIN_NAME).append("(indexLookUp:range_index");
- stringBuilder.append(",operator:").append(_rangePredicateEvaluator.getPredicateType());
- stringBuilder.append(",predicate:").append(_rangePredicateEvaluator.getPredicate().toString());
- return stringBuilder.append(')').toString();
+ return EXPLAIN_NAME + "(indexLookUp:range_index"
+ + ",operator:" + _rangePredicateEvaluator.getPredicateType()
+ + ",predicate:" + _rangePredicateEvaluator.getPredicate().toString()
+ + ')';
+ }
+
+ interface RangeEvaluator {
+ static RangeEvaluator of(RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader,
+ PredicateEvaluator predicateEvaluator) {
+ if (predicateEvaluator.isDictionaryBased()) {
+ return new IntRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ } else {
+ switch (predicateEvaluator.getDataType()) {
+ case INT:
+ return new IntRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case LONG:
+ return new LongRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case FLOAT:
+ return new FloatRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case DOUBLE:
+ return new DoubleRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ default:
+ throw new IllegalStateException("String and Bytes data type not supported for Range Indexing");
+ }
+ }
+ }
+
+ ImmutableRoaringBitmap getMatchingDocIds();
+
+ ImmutableRoaringBitmap getPartiallyMatchingDocIds();
+
+ int getMatchingDocCount();
+
+ boolean isExact();
+ }
+
+ private static final class IntRangeEvaluator implements RangeEvaluator {
+ final RangeIndexReader<ImmutableRoaringBitmap> _rangeIndexReader;
+ final int _min;
+ final int _max;
+
+ IntRangeEvaluator(
+ RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader, PredicateEvaluator predicateEvaluator) {
+ _rangeIndexReader = rangeIndexReader;
+ if (predicateEvaluator instanceof SortedDictionaryBasedRangePredicateEvaluator) {
+ // NOTE: End dictionary id is exclusive in OfflineDictionaryBasedRangePredicateEvaluator.
+ _min = ((SortedDictionaryBasedRangePredicateEvaluator) predicateEvaluator).getStartDictId();
+ _max = ((SortedDictionaryBasedRangePredicateEvaluator) predicateEvaluator).getEndDictId() - 1;
+ } else {
+ _min = ((IntRawValueBasedRangePredicateEvaluator) predicateEvaluator).getLowerBound();
+ _max = ((IntRawValueBasedRangePredicateEvaluator) predicateEvaluator).getUpperBound();
+ }
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getMatchingDocIds() {
+ return _rangeIndexReader.getMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getPartiallyMatchingDocIds() {
+ return _rangeIndexReader.getPartiallyMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public int getMatchingDocCount() {
+ return _rangeIndexReader.getMatchingDocCount(_min, _max);
+ }
+
+ @Override
+ public boolean isExact() {
+ return _rangeIndexReader.isExact();
+ }
+ }
+
+ private static final class LongRangeEvaluator implements RangeEvaluator {
+ final RangeIndexReader<ImmutableRoaringBitmap> _rangeIndexReader;
+ final long _min;
+ final long _max;
+
+ LongRangeEvaluator(RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader,
+ PredicateEvaluator predicateEvaluator) {
+ _rangeIndexReader = rangeIndexReader;
+ _min = ((LongRawValueBasedRangePredicateEvaluator) predicateEvaluator).getLowerBound();
+ _max = ((LongRawValueBasedRangePredicateEvaluator) predicateEvaluator).getUpperBound();
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getMatchingDocIds() {
+ return _rangeIndexReader.getMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getPartiallyMatchingDocIds() {
+ return _rangeIndexReader.getPartiallyMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public int getMatchingDocCount() {
+ return _rangeIndexReader.getMatchingDocCount(_min, _max);
+ }
+
+ @Override
+ public boolean isExact() {
+ return _rangeIndexReader.isExact();
+ }
+ }
+
+ private static final class FloatRangeEvaluator implements RangeEvaluator {
+ final RangeIndexReader<ImmutableRoaringBitmap> _rangeIndexReader;
+ final float _min;
+ final float _max;
+
+ FloatRangeEvaluator(RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader,
+ PredicateEvaluator predicateEvaluator) {
+ _rangeIndexReader = rangeIndexReader;
+ _min = ((FloatRawValueBasedRangePredicateEvaluator) predicateEvaluator).geLowerBound();
+ _max = ((FloatRawValueBasedRangePredicateEvaluator) predicateEvaluator).getUpperBound();
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getMatchingDocIds() {
+ return _rangeIndexReader.getMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getPartiallyMatchingDocIds() {
+ return _rangeIndexReader.getPartiallyMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public int getMatchingDocCount() {
+ return _rangeIndexReader.getMatchingDocCount(_min, _max);
+ }
+
+ @Override
+ public boolean isExact() {
+ return _rangeIndexReader.isExact();
+ }
+ }
+
+ private static final class DoubleRangeEvaluator implements RangeEvaluator {
+ final RangeIndexReader<ImmutableRoaringBitmap> _rangeIndexReader;
+ final double _min;
+ final double _max;
+
+ DoubleRangeEvaluator(RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader,
+ PredicateEvaluator predicateEvaluator) {
+ _rangeIndexReader = rangeIndexReader;
+ _min = ((FloatRawValueBasedRangePredicateEvaluator) predicateEvaluator).geLowerBound();
Review Comment:
(MAJOR) This should be `DoubleRawValueBasedRangePredicateEvaluator`. We should probably add a test which can catch this
--
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] richardstartin commented on a diff in pull request #8513: push counts down to range index when v2 is in use
Posted by GitBox <gi...@apache.org>.
richardstartin commented on code in PR #8513:
URL: https://github.com/apache/pinot/pull/8513#discussion_r853087705
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/RangeIndexBasedFilterOperator.java:
##########
@@ -145,10 +127,178 @@ public List<Operator> getChildOperators() {
@Override
public String toExplainString() {
- StringBuilder stringBuilder = new StringBuilder(EXPLAIN_NAME).append("(indexLookUp:range_index");
- stringBuilder.append(",operator:").append(_rangePredicateEvaluator.getPredicateType());
- stringBuilder.append(",predicate:").append(_rangePredicateEvaluator.getPredicate().toString());
- return stringBuilder.append(')').toString();
+ return EXPLAIN_NAME + "(indexLookUp:range_index"
+ + ",operator:" + _rangePredicateEvaluator.getPredicateType()
+ + ",predicate:" + _rangePredicateEvaluator.getPredicate().toString()
+ + ')';
+ }
+
+ interface RangeEvaluator {
+ static RangeEvaluator of(RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader,
+ PredicateEvaluator predicateEvaluator) {
+ if (predicateEvaluator.isDictionaryBased()) {
+ return new IntRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ } else {
+ switch (predicateEvaluator.getDataType()) {
+ case INT:
+ return new IntRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case LONG:
+ return new LongRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case FLOAT:
+ return new FloatRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case DOUBLE:
+ return new DoubleRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ default:
+ throw new IllegalStateException("String and Bytes data type not supported for Range Indexing");
+ }
+ }
+ }
+
+ ImmutableRoaringBitmap getMatchingDocIds();
+
+ ImmutableRoaringBitmap getPartiallyMatchingDocIds();
+
+ int getMatchingDocCount();
+
+ boolean isExact();
+ }
+
+ private static final class IntRangeEvaluator implements RangeEvaluator {
+ final RangeIndexReader<ImmutableRoaringBitmap> _rangeIndexReader;
+ final int _min;
+ final int _max;
+
+ IntRangeEvaluator(
+ RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader, PredicateEvaluator predicateEvaluator) {
+ _rangeIndexReader = rangeIndexReader;
+ if (predicateEvaluator instanceof SortedDictionaryBasedRangePredicateEvaluator) {
+ // NOTE: End dictionary id is exclusive in OfflineDictionaryBasedRangePredicateEvaluator.
+ _min = ((SortedDictionaryBasedRangePredicateEvaluator) predicateEvaluator).getStartDictId();
+ _max = ((SortedDictionaryBasedRangePredicateEvaluator) predicateEvaluator).getEndDictId() - 1;
+ } else {
+ _min = ((IntRawValueBasedRangePredicateEvaluator) predicateEvaluator).getLowerBound();
+ _max = ((IntRawValueBasedRangePredicateEvaluator) predicateEvaluator).getUpperBound();
+ }
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getMatchingDocIds() {
+ return _rangeIndexReader.getMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getPartiallyMatchingDocIds() {
+ return _rangeIndexReader.getPartiallyMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public int getMatchingDocCount() {
+ return _rangeIndexReader.getMatchingDocCount(_min, _max);
+ }
+
+ @Override
+ public boolean isExact() {
+ return _rangeIndexReader.isExact();
+ }
+ }
+
+ private static final class LongRangeEvaluator implements RangeEvaluator {
+ final RangeIndexReader<ImmutableRoaringBitmap> _rangeIndexReader;
+ final long _min;
+ final long _max;
+
+ LongRangeEvaluator(RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader,
+ PredicateEvaluator predicateEvaluator) {
+ _rangeIndexReader = rangeIndexReader;
+ _min = ((LongRawValueBasedRangePredicateEvaluator) predicateEvaluator).getLowerBound();
+ _max = ((LongRawValueBasedRangePredicateEvaluator) predicateEvaluator).getUpperBound();
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getMatchingDocIds() {
+ return _rangeIndexReader.getMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getPartiallyMatchingDocIds() {
+ return _rangeIndexReader.getPartiallyMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public int getMatchingDocCount() {
+ return _rangeIndexReader.getMatchingDocCount(_min, _max);
+ }
+
+ @Override
+ public boolean isExact() {
+ return _rangeIndexReader.isExact();
+ }
+ }
+
+ private static final class FloatRangeEvaluator implements RangeEvaluator {
+ final RangeIndexReader<ImmutableRoaringBitmap> _rangeIndexReader;
+ final float _min;
+ final float _max;
+
+ FloatRangeEvaluator(RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader,
+ PredicateEvaluator predicateEvaluator) {
+ _rangeIndexReader = rangeIndexReader;
+ _min = ((FloatRawValueBasedRangePredicateEvaluator) predicateEvaluator).geLowerBound();
+ _max = ((FloatRawValueBasedRangePredicateEvaluator) predicateEvaluator).getUpperBound();
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getMatchingDocIds() {
+ return _rangeIndexReader.getMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getPartiallyMatchingDocIds() {
+ return _rangeIndexReader.getPartiallyMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public int getMatchingDocCount() {
+ return _rangeIndexReader.getMatchingDocCount(_min, _max);
+ }
+
+ @Override
+ public boolean isExact() {
+ return _rangeIndexReader.isExact();
+ }
+ }
+
+ private static final class DoubleRangeEvaluator implements RangeEvaluator {
+ final RangeIndexReader<ImmutableRoaringBitmap> _rangeIndexReader;
+ final double _min;
+ final double _max;
+
+ DoubleRangeEvaluator(RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader,
+ PredicateEvaluator predicateEvaluator) {
+ _rangeIndexReader = rangeIndexReader;
+ _min = ((FloatRawValueBasedRangePredicateEvaluator) predicateEvaluator).geLowerBound();
Review Comment:
Oh wow, thanks for spotting this! I was sure this code path was tested. I will find out why not.
--
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] walterddr commented on a diff in pull request #8513: push counts down to range index when v2 is in use
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #8513:
URL: https://github.com/apache/pinot/pull/8513#discussion_r850853486
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/reader/RangeIndexReader.java:
##########
@@ -26,6 +26,51 @@
* @param <T>
*/
public interface RangeIndexReader<T> extends Closeable {
+
+ /**
+ * @return true if the results are exact and don't need refinement by scanning.
+ * This means {@see getPartiallyMatchingDocIds} will return null.
+ */
+ default boolean isExact() {
+ return true;
Review Comment:
oh yes. what I meant is we should not implement a default --> whether the implementation is exact or not cannot be determined in the interface level - e.g. if I accidentally deleted the override in `RangeIndexReaderImpl` i will get no compile time error until i run into test failures.
is the intent here to not allow any non-exact impl in the future?
--
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] richardstartin commented on a diff in pull request #8513: Enable range index on raw columns and push counts down to range index when v2 is in use
Posted by GitBox <gi...@apache.org>.
richardstartin commented on code in PR #8513:
URL: https://github.com/apache/pinot/pull/8513#discussion_r853304906
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/RangeIndexBasedFilterOperator.java:
##########
@@ -145,10 +127,178 @@ public List<Operator> getChildOperators() {
@Override
public String toExplainString() {
- StringBuilder stringBuilder = new StringBuilder(EXPLAIN_NAME).append("(indexLookUp:range_index");
- stringBuilder.append(",operator:").append(_rangePredicateEvaluator.getPredicateType());
- stringBuilder.append(",predicate:").append(_rangePredicateEvaluator.getPredicate().toString());
- return stringBuilder.append(')').toString();
+ return EXPLAIN_NAME + "(indexLookUp:range_index"
+ + ",operator:" + _rangePredicateEvaluator.getPredicateType()
+ + ",predicate:" + _rangePredicateEvaluator.getPredicate().toString()
+ + ')';
+ }
+
+ interface RangeEvaluator {
+ static RangeEvaluator of(RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader,
+ PredicateEvaluator predicateEvaluator) {
+ if (predicateEvaluator.isDictionaryBased()) {
+ return new IntRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ } else {
+ switch (predicateEvaluator.getDataType()) {
+ case INT:
+ return new IntRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case LONG:
+ return new LongRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case FLOAT:
+ return new FloatRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ case DOUBLE:
+ return new DoubleRangeEvaluator(rangeIndexReader, predicateEvaluator);
+ default:
+ throw new IllegalStateException("String and Bytes data type not supported for Range Indexing");
+ }
+ }
+ }
+
+ ImmutableRoaringBitmap getMatchingDocIds();
+
+ ImmutableRoaringBitmap getPartiallyMatchingDocIds();
+
+ int getMatchingDocCount();
+
+ boolean isExact();
+ }
+
+ private static final class IntRangeEvaluator implements RangeEvaluator {
+ final RangeIndexReader<ImmutableRoaringBitmap> _rangeIndexReader;
+ final int _min;
+ final int _max;
+
+ IntRangeEvaluator(
+ RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader, PredicateEvaluator predicateEvaluator) {
+ _rangeIndexReader = rangeIndexReader;
+ if (predicateEvaluator instanceof SortedDictionaryBasedRangePredicateEvaluator) {
+ // NOTE: End dictionary id is exclusive in OfflineDictionaryBasedRangePredicateEvaluator.
+ _min = ((SortedDictionaryBasedRangePredicateEvaluator) predicateEvaluator).getStartDictId();
+ _max = ((SortedDictionaryBasedRangePredicateEvaluator) predicateEvaluator).getEndDictId() - 1;
+ } else {
+ _min = ((IntRawValueBasedRangePredicateEvaluator) predicateEvaluator).getLowerBound();
+ _max = ((IntRawValueBasedRangePredicateEvaluator) predicateEvaluator).getUpperBound();
+ }
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getMatchingDocIds() {
+ return _rangeIndexReader.getMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getPartiallyMatchingDocIds() {
+ return _rangeIndexReader.getPartiallyMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public int getMatchingDocCount() {
+ return _rangeIndexReader.getMatchingDocCount(_min, _max);
+ }
+
+ @Override
+ public boolean isExact() {
+ return _rangeIndexReader.isExact();
+ }
+ }
+
+ private static final class LongRangeEvaluator implements RangeEvaluator {
+ final RangeIndexReader<ImmutableRoaringBitmap> _rangeIndexReader;
+ final long _min;
+ final long _max;
+
+ LongRangeEvaluator(RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader,
+ PredicateEvaluator predicateEvaluator) {
+ _rangeIndexReader = rangeIndexReader;
+ _min = ((LongRawValueBasedRangePredicateEvaluator) predicateEvaluator).getLowerBound();
+ _max = ((LongRawValueBasedRangePredicateEvaluator) predicateEvaluator).getUpperBound();
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getMatchingDocIds() {
+ return _rangeIndexReader.getMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getPartiallyMatchingDocIds() {
+ return _rangeIndexReader.getPartiallyMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public int getMatchingDocCount() {
+ return _rangeIndexReader.getMatchingDocCount(_min, _max);
+ }
+
+ @Override
+ public boolean isExact() {
+ return _rangeIndexReader.isExact();
+ }
+ }
+
+ private static final class FloatRangeEvaluator implements RangeEvaluator {
+ final RangeIndexReader<ImmutableRoaringBitmap> _rangeIndexReader;
+ final float _min;
+ final float _max;
+
+ FloatRangeEvaluator(RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader,
+ PredicateEvaluator predicateEvaluator) {
+ _rangeIndexReader = rangeIndexReader;
+ _min = ((FloatRawValueBasedRangePredicateEvaluator) predicateEvaluator).geLowerBound();
+ _max = ((FloatRawValueBasedRangePredicateEvaluator) predicateEvaluator).getUpperBound();
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getMatchingDocIds() {
+ return _rangeIndexReader.getMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public ImmutableRoaringBitmap getPartiallyMatchingDocIds() {
+ return _rangeIndexReader.getPartiallyMatchingDocIds(_min, _max);
+ }
+
+ @Override
+ public int getMatchingDocCount() {
+ return _rangeIndexReader.getMatchingDocCount(_min, _max);
+ }
+
+ @Override
+ public boolean isExact() {
+ return _rangeIndexReader.isExact();
+ }
+ }
+
+ private static final class DoubleRangeEvaluator implements RangeEvaluator {
+ final RangeIndexReader<ImmutableRoaringBitmap> _rangeIndexReader;
+ final double _min;
+ final double _max;
+
+ DoubleRangeEvaluator(RangeIndexReader<ImmutableRoaringBitmap> rangeIndexReader,
+ PredicateEvaluator predicateEvaluator) {
+ _rangeIndexReader = rangeIndexReader;
+ _min = ((FloatRawValueBasedRangePredicateEvaluator) predicateEvaluator).geLowerBound();
Review Comment:
Fixed.
--
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] richardstartin commented on pull request #8513: Enable range index on raw columns and push counts down to range index when v2 is in use
Posted by GitBox <gi...@apache.org>.
richardstartin commented on PR #8513:
URL: https://github.com/apache/pinot/pull/8513#issuecomment-1102888641
@Jackie-Jiang the copy-paste bug you spotted was revealing - range indexes over raw columns was not actually supported, despite #5853 aiming to add support. Commit fe28919f0dd9b0da143263266f4bba2ddab5b3e0 addresses that. Commit f30099e9b654fc2d92aba20afd72722b3a1003dc addresses your review comments, and adds a test to verify the bug is fixed (and that the code executes).
--
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