jpountz commented on issue #14887: URL: https://github.com/apache/lucene/issues/14887#issuecomment-3027457319
This would be a disruptive change indeed and there is no urgency, I have the next major in mind. Also I still need to finish convincing myself that it is the right thing to do, hence this issue. :) Having 2 sealed classes is what I tried first as well, but it doesn't work completely because of sub-classes of `DocIdSetIterator` that declare additional methods, such as `PostingsEnum`, `Spans` and `Intervals`. We can't have classes that extend both `PostingsEnum` and `AbstractDocIdSetIterator`. Another thing that made me open this issue is that I've been looking at how Vespa works lately, and it's quite similar to Lucene in how iterators can get arbitrarily nested via compound queries (e.g. boolean queries). Yet only the `advance` method of its iterator API (called `doSeek`) is virtual, the `docID` method (called `getDocId`) is not: https://github.com/vespa-engine/vespa/blob/a4ed083250f65b8acb82f6a41bfeddfedd772e30/searchlib/src/vespa/searchlib/queryeval/searchiterator.h#L37 Finally, if `docID()` was already final today, I don't believe that we'd be discussing making it bi-morphic? -- 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
