Copilot commented on code in PR #16755:
URL: https://github.com/apache/pinot/pull/16755#discussion_r2322607845
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/MVScanDocIdIterator.java:
##########
@@ -258,4 +243,45 @@ public void close() {
_readerContext.close();
}
}
+
+ public static class Asc extends MVScanDocIdIterator {
+ public Asc(PredicateEvaluator predicateEvaluator, DataSource dataSource,
int numDocs) {
+ super(predicateEvaluator, dataSource, numDocs);
+ }
+
+ @Override
+ public int next() {
+ while (_nextDocId < _numDocs) {
+ int nextDocId = _nextDocId++;
+ // TODO: The performance can be improved by batching the docID lookups
similar to how it's done in
+ // SVScanDocIdIterator
+ boolean doesValueMatch = _valueMatcher.doesValueMatch(nextDocId);
+ if (doesValueMatch) {
+ return nextDocId;
+ }
+ }
+ close();
+ return Constants.EOF;
+ }
+ }
+
+ public static class Desc extends MVScanDocIdIterator {
+ public Desc(PredicateEvaluator predicateEvaluator, DataSource dataSource,
int numDocs) {
+ super(predicateEvaluator, dataSource, numDocs);
Review Comment:
The `Desc` constructor doesn't initialize `_nextDocId` to start from the end
of the document range like the ascending version does. It should set
`_nextDocId = numDocs - 1` to properly iterate in descending order.
```suggestion
super(predicateEvaluator, dataSource, numDocs);
_nextDocId = numDocs - 1;
```
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BitmapBasedFilterOperator.java:
##########
@@ -79,4 +83,9 @@ public List<Operator> getChildOperators() {
public String toExplainString() {
return EXPLAIN_NAME;
}
+
+ @Override
+ protected BaseFilterOperator reverse() {
+ return new BitmapBasedFilterOperator(_docIds, _ascending, _numDocs,
!_ascending);
Review Comment:
The constructor call at line 89 passes `_ascending` as the second parameter
(exclusive) and `!_ascending` as the fourth parameter (ascending). This creates
a logical inconsistency where the exclusive flag equals the ascending flag,
which doesn't match the expected behavior.
```suggestion
return new BitmapBasedFilterOperator(_docIds, !_exclusive, _numDocs,
!_ascending);
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
