gsmiller commented on code in PR #13592:
URL: https://github.com/apache/lucene/pull/13592#discussion_r1723543404
##########
lucene/core/src/java/org/apache/lucene/index/DocValuesSkipper.java:
##########
@@ -98,4 +98,29 @@ public abstract class DocValuesSkipper {
/** Return the global number of documents with a value for the field. */
public abstract int docCount();
+
+ /**
+ * Advance this skipper so that all levels intersects the range given by
{@code minValue} and
+ * {@code maxValue}. If there are no intersecting levels, the skipper is
exhausted.
+ *
+ * <p><b>NOTE</b>: The behavior is undefined if this method is called and
{@link #advance(int)}
+ * has not been called yet.
+ */
+ public final void advance(long minValue, long maxValue) throws IOException {
+ while (true) {
Review Comment:
If it's required that `#advance` has already been called at least once
before this is used, should we add something like `assert minDocID(0) > -1`?
##########
lucene/core/src/java/org/apache/lucene/index/DocValuesSkipper.java:
##########
@@ -98,4 +98,29 @@ public abstract class DocValuesSkipper {
/** Return the global number of documents with a value for the field. */
public abstract int docCount();
+
+ /**
+ * Advance this skipper so that all levels intersects the range given by
{@code minValue} and
+ * {@code maxValue}. If there are no intersecting levels, the skipper is
exhausted.
+ *
+ * <p><b>NOTE</b>: The behavior is undefined if this method is called and
{@link #advance(int)}
+ * has not been called yet.
+ */
+ public final void advance(long minValue, long maxValue) throws IOException {
+ while (true) {
+ if (minDocID(0) == DocIdSetIterator.NO_MORE_DOCS
+ || (minValue(0) <= maxValue && maxValue(0) >= minValue)) {
+ break;
+ } else {
+ int maxDocID = maxDocID(0);
Review Comment:
This might just be me, but I got really confused for a while until I
realized that `maxDocID` does not necessarily return the docID associated with
`maxValue` (in the standard ascending sort case it would, but it's reversed in
the reverse sort case... assuming I'm getting this right). I had the same
confusing back in the calling code in the reverse-sort case. Would you mind
adding a couple comments to this code to make that a little more clear in case
someone else suffers from my same confusion?
--
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]
