dweiss commented on code in PR #16714:
URL: https://github.com/apache/lucene/pull/16714#discussion_r4152509250
##########
lucene/core/src/java/org/apache/lucene/search/SortedSetSelector.java:
##########
@@ -296,6 +298,7 @@ public long cost() {
@Override
public void intoBitSet(int upTo, FixedBitSet bitSet, int offset) throws
IOException {
in.intoBitSet(upTo, bitSet, offset);
+ setOrd();
Review Comment:
If you take a look at SortedSetSelector, Mike, it has these nested adapter
classes extending SortedDocValues, like this one:
```
static class MinValue extends SortedDocValues {
```
These classes keep a pointer to the current doc's ordinal (in a private
field) so that it can be returned efficiently from the implementation of
SortedDocValues.ordValue. But the intoBitSet method just delegated the call and
didn't update this ordinal properly. I've asked the LLM to write a simple
snippet of code demonstrating this bug prior to this patch and I think it shows
it clearly (although I didn't bother running):
```java
● This snippet has not been run; it is written against the pre-fix code to
show the stale ordinal.
try (Directory dir = new ByteBuffersDirectory();
IndexWriter w = new IndexWriter(dir, new IndexWriterConfig())) {
// Ords: a=0, b=1, c=2, d=3
Document doc0 = new Document();
doc0.add(new SortedSetDocValuesField("f", new BytesRef("a")));
w.addDocument(doc0);
// Two values here, so the codec can't hand back a singleton and
// SortedSetSelector.wrap() really returns the MinValue wrapper.
Document doc1 = new Document();
doc1.add(new SortedSetDocValuesField("f", new BytesRef("b")));
doc1.add(new SortedSetDocValuesField("f", new BytesRef("c")));
w.addDocument(doc1);
Document doc2 = new Document();
doc2.add(new SortedSetDocValuesField("f", new BytesRef("d")));
w.addDocument(doc2);
try (DirectoryReader r = DirectoryReader.open(w)) {
LeafReader leaf = r.leaves().get(0).reader();
SortedDocValues dv =
SortedSetSelector.wrap(
DocValues.getSortedSet(leaf, "f"), SortedSetSelector.Type.MIN);
dv.nextDoc(); // on doc 0, ord = 0 ("a")
FixedBitSet bits = new FixedBitSet(leaf.maxDoc());
dv.intoBitSet(2, bits, 0); // collects docs 0 and 1, lands on doc 2
assertEquals(2, dv.docID()); // passes: the iterator moved
assertEquals(3, dv.ordValue());
// before the fix: fails, ordValue() is still 0 ("a", from doc 0)
// after the fix: passes, ordValue() is 3 ("d")
}
}
The same shape fails for the other three selector types. For example, with
Type.MAX the stale value is also 0, while the correct one is
again 3.
```
--
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]
