dsmiley commented on a change in pull request #1740:
URL: https://github.com/apache/lucene-solr/pull/1740#discussion_r470163319
##########
File path:
lucene/analysis/common/src/java/org/apache/lucene/analysis/miscellaneous/WordDelimiterFilter.java
##########
@@ -398,10 +399,20 @@ public void reset() throws IOException {
private class OffsetSorter extends InPlaceMergeSorter {
@Override
protected int compare(int i, int j) {
+ // earlier start offset
int cmp = Integer.compare(startOff[i], startOff[j]);
- if (cmp == 0) {
- cmp = Integer.compare(posInc[j], posInc[i]);
+ if (cmp != 0) {
+ return cmp;
}
+
+ // earlier position
+ cmp = Integer.compare(posInc[j], posInc[i]);
Review comment:
@bruno-roustant pointed out to me that the existing comparison here
looks faulty -- notice it's doing the *later* position due to the swap of "j"
and "i". This is how it was before. I think it's wrong, and makes me suspect
that it's likely this comparison yields a zero. If true, this is more evidence
that suggest the sort can merely look at start & end offsets.
----------------------------------------------------------------
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
