adelapena commented on code in PR #2495:
URL: https://github.com/apache/cassandra/pull/2495#discussion_r1265386258
##########
src/java/org/apache/cassandra/index/sai/iterators/KeyRangeIterator.java:
##########
@@ -106,13 +107,21 @@ else if (next.compareTo(nextKey) >= 0)
return recomputeNext();
}
+ /**
+ * Skip to nextKey.
+ *
+ * That is, implementations should set up the iterator state such that
+ * calling computeNext() will return nextKey if present,
+ * or the first one after it if not present.
+ */
protected abstract void performSkipTo(PrimaryKey nextKey);
- protected PrimaryKey recomputeNext()
+ private PrimaryKey recomputeNext()
{
return tryToComputeNext() ? peek() : endOfData();
}
+ // protected because inherited from Guava. We don't want to expose this
method.
Review Comment:
We should probably annotate the method with `@Override`.
##########
src/java/org/apache/cassandra/index/sai/iterators/KeyRangeIntersectionIterator.java:
##########
@@ -249,14 +249,24 @@ private void updateStatistics(Statistics statistics,
KeyRangeIterator range)
private static class IntersectionStatistics extends
KeyRangeIterator.Builder.Statistics
{
+ private boolean empty = true;
+
@Override
public void update(KeyRangeIterator range)
{
// minimum of the intersection is the biggest minimum of
individual iterators
min = nullSafeMax(min, range.getMinimum());
// maximum of the intersection is the smallest maximum of
individual iterators
max = nullSafeMin(max, range.getMaximum());
- count += range.getCount();
+ if (empty)
+ {
+ empty = false;
+ count = range.getCount();
+ }
+ else
+ {
+ count = Math.min(count, range.getCount());
+ }
Review Comment:
Can this be `count = count == 0 ? range.getCount() : Math.min(count,
range.getCount());
`?
##########
src/java/org/apache/cassandra/index/sai/iterators/KeyRangeIterator.java:
##########
@@ -81,7 +81,8 @@ public final long getCount()
*
* @param nextKey value to skip the iterator forward until matching
*
- * @return The next current key after the skip was performed
+ * @return The key skipped to, which will be the key returned by the
+ * next call to `next()`, i.e., we are "peeking" at the next key as part
of the skip.
Review Comment:
```suggestion
* next call to {@link #next()}, i.e., we are "peeking" at the next key
as part of the skip.
```
--
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]
