You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by cp...@apache.org on 2019/10/23 17:12:46 UTC

[lucene-solr] branch branch_8x updated: Revert "LUCENE-8996: maxScore was sometimes missing from distributed grouped responses."

This is an automated email from the ASF dual-hosted git repository.

cpoerschke pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new acaaf4c  Revert "LUCENE-8996: maxScore was sometimes missing from distributed grouped responses."
acaaf4c is described below

commit acaaf4c2702e6e0138916617fd2780a04c172522
Author: Christine Poerschke <cp...@apache.org>
AuthorDate: Wed Oct 23 18:12:24 2019 +0100

    Revert "LUCENE-8996: maxScore was sometimes missing from distributed grouped responses."
    
    This reverts commit fb37f89af27d881efb0f7b40eff2dc76ef5bc2c4.
---
 lucene/CHANGES.txt                                      |  3 ---
 .../org/apache/lucene/search/grouping/TopGroups.java    | 17 ++---------------
 .../apache/lucene/search/grouping/TopGroupsTest.java    |  4 ++++
 3 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index 1fec234..1d9d4b9 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -170,9 +170,6 @@ Bug Fixes
   subclauses, and then pull SHOULD, MUST_NOT and FILTER visitors from it rather than from
   the parent.  (Alan Woodward)
 
-* LUCENE-8996: maxScore was sometimes missing from distributed grouped responses.
-  (Julien Massenet, Diego Ceccarelli, Christine Poerschke)
-
 Other
 
 * LUCENE-8778 LUCENE-8911 LUCENE-8957: Define analyzer SPI names as static final fields and document the names in Javadocs.
diff --git a/lucene/grouping/src/java/org/apache/lucene/search/grouping/TopGroups.java b/lucene/grouping/src/java/org/apache/lucene/search/grouping/TopGroups.java
index af1fffd..71338f9 100644
--- a/lucene/grouping/src/java/org/apache/lucene/search/grouping/TopGroups.java
+++ b/lucene/grouping/src/java/org/apache/lucene/search/grouping/TopGroups.java
@@ -80,19 +80,6 @@ public class TopGroups<T> {
     Avg,
   }
 
-  /**
-   * If either value is NaN then return the other value, otherwise
-   * return the greater of the two values by calling Math.max.
-   * @param a - one value
-   * @param b - another value
-   * @return ignoring any NaN return the greater of a and b
-   */
-  private static float nonNANmax(float a, float b) {
-    if (Float.isNaN(a)) return b;
-    if (Float.isNaN(b)) return a;
-    return Math.max(a, b);
-  }
-
   /** Merges an array of TopGroups, for example obtained
    *  from the second-pass collector across multiple
    *  shards.  Each TopGroups must have been sorted by the
@@ -182,7 +169,7 @@ public class TopGroups<T> {
               shardGroupDocs.scoreDocs,
               docSort.getSort());
         }
-        maxScore =  nonNANmax(maxScore, shardGroupDocs.maxScore);
+        maxScore = Math.max(maxScore, shardGroupDocs.maxScore);
         assert shardGroupDocs.totalHits.relation == Relation.EQUAL_TO;
         totalHits += shardGroupDocs.totalHits.value;
         scoreSum += shardGroupDocs.score;
@@ -236,7 +223,7 @@ public class TopGroups<T> {
                                                    mergedScoreDocs,
                                                    groupValue,
                                                    shardGroups[0].groups[groupIDX].groupSortValues);
-      totalMaxScore = nonNANmax(totalMaxScore, maxScore);
+      totalMaxScore = Math.max(totalMaxScore, maxScore);
     }
 
     if (totalGroupCount != null) {
diff --git a/lucene/grouping/src/test/org/apache/lucene/search/grouping/TopGroupsTest.java b/lucene/grouping/src/test/org/apache/lucene/search/grouping/TopGroupsTest.java
index e160bf4..8fb661d 100644
--- a/lucene/grouping/src/test/org/apache/lucene/search/grouping/TopGroupsTest.java
+++ b/lucene/grouping/src/test/org/apache/lucene/search/grouping/TopGroupsTest.java
@@ -21,12 +21,16 @@ import org.apache.lucene.search.Sort;
 import org.apache.lucene.search.TotalHits;
 import org.apache.lucene.util.LuceneTestCase;
 
+import org.junit.Ignore;
+
 public class TopGroupsTest extends LuceneTestCase {
 
+  @Ignore // https://issues.apache.org/jira/browse/LUCENE-8996
   public void testAllGroupsEmptyInSecondPass() {
     narrativeMergeTestImplementation(false, false, false, false);
   }
 
+  @Ignore // https://issues.apache.org/jira/browse/LUCENE-8996
   public void testSomeGroupsEmptyInSecondPass() {
     narrativeMergeTestImplementation(false, false, false, true);
     narrativeMergeTestImplementation(false, false, true, false);