Copilot commented on code in PR #16755:
URL: https://github.com/apache/pinot/pull/16755#discussion_r2322607775
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/dociditerators/AndDocIdIterator.java:
##########
@@ -27,55 +27,94 @@
* <p>It keeps calling {@link BlockDocIdIterator#advance(int)} to gather the
common matching document ids from all child
* BlockDocIdIterators until one of them hits the end.
*/
-public final class AndDocIdIterator implements BlockDocIdIterator {
+public class AndDocIdIterator implements BlockDocIdIterator {
public final BlockDocIdIterator[] _docIdIterators;
- private int _nextDocId = 0;
+ protected int _nextDocId;
- public AndDocIdIterator(BlockDocIdIterator[] docIdIterators) {
+ private AndDocIdIterator(BlockDocIdIterator[] docIdIterators) {
_docIdIterators = docIdIterators;
}
+ public static AndDocIdIterator create(BlockDocIdIterator[] docIdIterators,
boolean asc) {
+ return asc ? new Asc(docIdIterators) : new Desc(docIdIterators);
+ }
+
@Override
- public int next() {
- int maxDocId = _nextDocId;
- int maxDocIdIndex = -1;
+ public int advance(int targetDocId) {
+ _nextDocId = targetDocId;
+ return next();
+ }
+
+ @Override
+ public void close() {
+ for (BlockDocIdIterator it : _docIdIterators) {
+ it.close();
+ }
+ }
+
+ /// Starting from [#_nextDocId], returns the next common matching document
id from all child BlockDocIdIterators or
+ /// Constants.EOF if there is no more common matching document id.
+ ///
+ /// Although the state of [#_nextDocId] is not updated, iterators will be
advanced, so this method have side effects.
+ protected int findNextCommonMatch() {
+ // CandidateId is the next did to try.
+ // It is initialized to the current _nextDocId and updated using advance()
calls on the child iterators.
+ int candidateId = _nextDocId;
+ // The index in _docIdIterators that returned the candidateId from
advance() call.
+ // Initialized to -1 to indicate that no iterator has returned the
candidateId yet.
+ int candidateIdIndex = -1;
int numDocIdIterators = _docIdIterators.length;
+ // The index of the next iterator to test. It can be reset to 0 when a new
candidateId is found.
int index = 0;
while (index < numDocIdIterators) {
- if (index == maxDocIdIndex) {
+ if (index == candidateIdIndex) {
// Skip the index with the max document id
index++;
continue;
}
- int docId = _docIdIterators[index].advance(maxDocId);
- if (docId != Constants.EOF) {
- if (docId == maxDocId) {
- index++;
- } else {
- // The current iterator does not contain the maxDocId, update
maxDocId and advance all other iterators
- maxDocId = docId;
- maxDocIdIndex = index;
- index = 0;
- }
- } else {
- closeIterators();
+ // docId is the next docId from the current iterator that matches
+ int docId = _docIdIterators[index].advance(candidateId);
+ if (docId == Constants.EOF) { // this iterator is exhausted. Given we
are an and we know there are no more matches
+ close();
return Constants.EOF;
}
+ if (docId == candidateId) { // The current iterator contains the
candidateId, move to the next iterator
+ index++;
+ } else {
+ // The current iterator does not contain the candidateId, update
candidateId
+ // and reset index to advance other iterators
+ candidateId = docId;
+ candidateIdIndex = index;
+ index = 0;
+ }
}
- _nextDocId = maxDocId;
- return _nextDocId++;
+ return candidateId;
}
- @Override
- public int advance(int targetDocId) {
- _nextDocId = targetDocId;
- return next();
+ public static class Asc extends AndDocIdIterator {
+ public Asc(BlockDocIdIterator[] docIdIterators) {
+ super(docIdIterators);
+ _nextDocId = 0;
+ }
+
+ @Override
+ public int next() {
+ _nextDocId = findNextCommonMatch();
+ return _nextDocId++;
+ }
}
- private void closeIterators() {
- for (BlockDocIdIterator it : _docIdIterators) {
- it.close();
+ public static class Desc extends AndDocIdIterator {
+ public Desc(BlockDocIdIterator[] docIdIterators) {
+ super(docIdIterators);
+ _nextDocId = Integer.MAX_VALUE;
}
}
+
+ @Override
+ public int next() {
+ _nextDocId = findNextCommonMatch();
+ return _nextDocId--;
Review Comment:
The `next()` method at line 117-118 is outside of any class definition. This
appears to be a structural error where the `Desc` class definition is
incomplete, leaving this method floating outside the class.
```suggestion
@Override
public int next() {
_nextDocId = findNextCommonMatch();
return _nextDocId--;
}
```
--
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]
