ctargett commented on pull request #1625: URL: https://github.com/apache/lucene-solr/pull/1625#issuecomment-651138896
I commented with a specific recommendation on the change, but in general wonder if it's worth it? LUCENE-9328 is marked as an Improvement, which would imply to me that we never should have expected grouping to be faster with docValues? If that's the case, then the docs have been incorrect and this change makes sense. However, if grouping was correctly documented as a specific thing that should be faster but isn't today because of a regression, then the Jira should be a Bug and this change makes less sense because we don't document every single Bug in Solr or Lucene in the Ref Guide - we'd have hundreds of tiny edits as things break and get fixed and we'd miss *a lot* of them. There's obviously differences of degrees here - if SSL totally broke for an entire release, we'd probably want to document that, but grouping being slower for a release or two (if that's the case, I didn't study the Jiras so maybe it's been longer), that's a less pressing edit IMO. A related point is that if this has been wrong all along but is now going to be supported in the upcoming release (not sure the timing of LUCENE-9328), making this change now means you'll have to add the word back in another commit before release and it wouldn't be worth removing it now. ---------------------------------------------------------------- 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. 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
