mikemccand commented on code in PR #11888:
URL: https://github.com/apache/lucene/pull/11888#discussion_r1539192140
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene90/blocktree/SegmentTermsEnumFrame.java:
##########
@@ -523,7 +526,9 @@ public void scanToSubBlock(long subFP) {
// NOTE: sets startBytePos/suffix as a side effect
public SeekStatus scanToTerm(BytesRef target, boolean exactOnly) throws
IOException {
- return isLeafBlock ? scanToTermLeaf(target, exactOnly) :
scanToTermNonLeaf(target, exactOnly);
+ return isLeafBlock
Review Comment:
I know this was a pre-existing ternary :) But now we are embedding another
confusing ternary inside the first one -- could we instead spell all of this
out as verbose `if`?
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene90/blocktree/SegmentTermsEnumFrame.java:
##########
@@ -568,8 +573,6 @@ public SeekStatus scanToTermLeaf(BytesRef target, boolean
exactOnly) throws IOEx
assert prefixMatches(target);
- // TODO: binary search when all terms have the same length, which is
common for ID fields,
Review Comment:
Aha! Another `TODO` gone, thank you @vsop-479!
##########
lucene/core/src/test/org/apache/lucene/codecs/lucene99/TestLucene99PostingsFormat.java:
##########
@@ -143,4 +141,13 @@ private void doTestImpactSerialization(List<Impact>
impacts) throws IOException
}
}
}
+
+ @Override
+ protected void subCheckBinarySearch(TermsEnum termsEnum) throws Exception {
+ // 10004a matched block's entries: [100001, 100003, ..., 100049].
+ // if target greater than the last entry of the matched block,
+ // termsEnum.term should be the last entry.
+ assertFalse(termsEnum.seekExact(new BytesRef("10004a")));
+ assertEquals(termsEnum.term(), new BytesRef("100049"));
Review Comment:
Hmm, when `seekExact` returns `false`, the `TermsEnum` is unpositioned and
calling `.term()` (and other methods e.g. `.postings()`) is not allowed (the
behavior is undefined -- it could throw an exception or corrupt its internal
state or so) ... why are we testing that here :)
--
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: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
