dsmiley commented on code in PR #1958:
URL: https://github.com/apache/solr/pull/1958#discussion_r2800028527
##########
solr/core/src/java/org/apache/solr/search/DelegatingCollector.java:
##########
@@ -80,9 +80,20 @@ protected void doSetNextReader(LeafReaderContext context)
throws IOException {
leafDelegate = delegate.getLeafCollector(context);
}
- public void finish() throws IOException {
+ /**
+ * From Solr 9.4 using Lucene 9.8 onwards
<code>DelegatingCollector.finish</code> clashes with the
+ * super class's <code>LeafCollector.finish</code> method. Please relocate
any finishing logic
+ * into the <code>DelegatingCollector.complete</code> replacement completion
method.
+ */
+ @Override
+ public final void finish() throws IOException {
+ super.finish();
+ }
Review Comment:
I'm looking at this now, and am surprised in what I see. It seems that Solr
had a finish concept before Lucene, and here comes Lucene with it, finally.
Right? I question if I understand this because the javadocs here encourage
users to use `complete`, which is custom/non-aligned with Lucene that we
apparently introduced in 9.4... so we broke users regardless, and defer an
eventual forced break on the eventual alignment with Lucene finish() - that
_still_ hasn't happened. IMO that should have been done immediately after this
PR merged on `main` that would become 10.0. We have upgrade notes to
communicate to users about thorny issues like this.
--
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]
