maedhroz commented on code in PR #2540:
URL: https://github.com/apache/cassandra/pull/2540#discussion_r1289339550


##########
src/java/org/apache/cassandra/index/sai/disk/v1/sortedterms/SortedTermsReader.java:
##########
@@ -112,123 +108,259 @@ public SortedTermsReader(@Nonnull FileHandle 
termsDataFileHandle,
     public class Cursor implements AutoCloseable
     {
         private final IndexInputReader termsInput;
+        private final int blockShift;
+        private final int blockMask;
         private final long termsDataFp;
         private final LongArray blockOffsets;
 
         // The term the cursor currently points to. Initially empty.
         private final BytesRef currentTerm;
 
-        // The point id the cursor currently points to. BEFORE_START means 
before the first item.
-        private long pointId = BEFORE_START;
+        private final BytesRef nextBlockTerm;
+
+        // The point id the cursor currently points to.
+        private long currentPointId;
+        private long currentBlockIndex;
 
         Cursor(FileHandle termsFile, LongArray.Factory blockOffsetsFactory) 
throws IOException
         {
             this.termsInput = IndexInputReader.create(termsFile);
             SAICodecUtils.validate(this.termsInput);
+            this.blockShift = this.termsInput.readVInt();
+            this.blockMask = (1 << this.blockShift) - 1;
             this.termsDataFp = this.termsInput.getFilePointer();
             this.blockOffsets = new 
LongArray.DeferredLongArray(blockOffsetsFactory::open);
             this.currentTerm = new BytesRef(meta.maxTermLength);
+            this.nextBlockTerm = new BytesRef(meta.maxTermLength);
+            termsInput.seek(termsDataFp);
+            readTerm(currentPointId, currentTerm);
         }
 
         /**
-         * Returns the current position of the cursor.
-         * Initially, before the first call to {@link #advance}, the cursor is 
positioned at -1.
-         * After reading all the items, the cursor is positioned at index one
-         * greater than the position of the last item.
+         * Positions the cursor on the target point id and reads the term at 
target to the current term buffer.
+         * <p>
+         * It is allowed to position the cursor before the first item or after 
the last item;
+         * in these cases the internal buffer is cleared.
+         *
+         * @param nextPointId point id to lookup
+         * @return The {@link ByteComparable} containing the term
+         * @throws IndexOutOfBoundsException if the target point id is less 
than -1 or greater than the number of terms
          */
-        public long pointId()
+        public @Nonnull ByteComparable seekForwardToPointId(long nextPointId)
         {
-            return pointId;
-        }
+            if (nextPointId < 0 || nextPointId > meta.termCount)

Review Comment:
   Shouldn't this be `nextPointId >= meta.termCount`? (Corresponding change to 
the error message, etc.)
   
   Fun thing is that if you add...
   
   ```
   assertThatThrownBy(() -> 
cursor.seekForwardToPointId(4000L)).isInstanceOf(IndexOutOfBoundsException.class);
   ```
   
   ...to `testSeekToPointIdOutOfRange()`, the test passes, but only because the 
`LongArray` in `resetPosition()` throws `IOOBE` :)



-- 
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]

Reply via email to