salvatorecampagna commented on code in PR #15413:
URL: https://github.com/apache/lucene/pull/15413#discussion_r2513558762
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90LiveDocsFormat.java:
##########
@@ -67,7 +76,10 @@ public Bits readLiveDocs(Directory dir, SegmentCommitInfo
info, IOContext contex
throws IOException {
long gen = info.getDelGen();
String name = IndexFileNames.fileNameFromGeneration(info.info.name,
EXTENSION, gen);
Review Comment:
Yeah, right. The issue is `LiveDocsFormat.readLiveDocs()` returns `Bits` and
changing it would break all existing codecs, which feels too big for this PR.
That said, I added `LeafReader::getLiveDocsWithDeletedIterator()` that
returns `LiveDocs` directly. Callers can also just cast if they want the extra
methods:
```java
Bits bits = liveDocsFormat.readLiveDocs(...);
if (bits instanceof LiveDocs liveDocs) {
// Use deletedDocsIterator(), deletedCount(), etc.
}
```
Using `LeafReader::getLiveDocsWithDeletedIterator` lets consumers opt-in to
the optimizations they can't get from just `Bits`. To me it feels like a
reasonable compromise. We avoid breaking changes to existing code while still
giving consumers an efficient way to handle both live and deleted documents.
I'm thinking this is better for BWC and API evolution anyway: we can change
the return type in a future codec version. For now, `Bits` works everywhere and
`LiveDocs` is there when you need it.
Happy to file a follow-up issue for changing the return type in a future
codec if that makes sense?
--
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]
