lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From uschind...@apache.org
Subject svn commit: r1468638 - in /lucene/dev/branches/branch_4x: lucene/ lucene/core/src/java/org/apache/lucene/index/ lucene/core/src/java/org/apache/lucene/search/ lucene/core/src/test/org/apache/lucene/search/ lucene/queries/src/java/org/apache/lucene/quer...
Date Tue, 16 Apr 2013 22:09:50 GMT
Author: uschindler
Date: Tue Apr 16 22:09:49 2013
New Revision: 1468638

URL: http://svn.apache.org/r1468638
Log:
LUCENE-4937: Fix incorrect sorting of float/double values (+/-0, NaN).

Modified:
    lucene/dev/branches/branch_4x/lucene/CHANGES.txt
    lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/LogMergePolicy.java
    lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/FieldComparator.java
    lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/SearcherLifetimeManager.java
    lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSort.java
    lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSortDocValues.java
    lucene/dev/branches/branch_4x/lucene/queries/src/java/org/apache/lucene/queries/function/ValueSource.java
    lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/schema/DocValuesTest.java

Modified: lucene/dev/branches/branch_4x/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/CHANGES.txt?rev=1468638&r1=1468637&r2=1468638&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/CHANGES.txt (original)
+++ lucene/dev/branches/branch_4x/lucene/CHANGES.txt Tue Apr 16 22:09:49 2013
@@ -274,6 +274,9 @@ Bug Fixes
 * LUCENE-4504: Fix broken sort comparator in ValueSource.getSortField,
   used when sorting by a function query.  (Tom Shally via Robert Muir)
 
+* LUCENE-4937: Fix incorrect sorting of float/double values (+/-0, NaN).
+  (Robert Muir, Uwe Schindler)
+
 Documentation
 
 * LUCENE-4841: Added example SimpleSortedSetFacetsExample to show how

Modified: lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/LogMergePolicy.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/LogMergePolicy.java?rev=1468638&r1=1468637&r2=1468638&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/LogMergePolicy.java
(original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/LogMergePolicy.java
Tue Apr 16 22:09:49 2013
@@ -546,13 +546,7 @@ public abstract class LogMergePolicy ext
     // Sorts largest to smallest
     @Override
     public int compareTo(SegmentInfoAndLevel other) {
-      if (level < other.level) {
-        return 1;
-      } else if (level > other.level) {
-        return -1;
-      } else {
-        return 0;
-      }
+      return Float.compare(other.level, level);
     }
   }
 

Modified: lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/FieldComparator.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/FieldComparator.java?rev=1468638&r1=1468637&r2=1468638&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/FieldComparator.java
(original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/FieldComparator.java
Tue Apr 16 22:09:49 2013
@@ -19,7 +19,6 @@ package org.apache.lucene.search;
 
 import java.io.IOException;
 
-import org.apache.lucene.index.AtomicReader; // javadocs
 import org.apache.lucene.index.AtomicReaderContext;
 import org.apache.lucene.index.BinaryDocValues;
 import org.apache.lucene.index.SortedDocValues;
@@ -307,15 +306,7 @@ public abstract class FieldComparator<T>
 
     @Override
     public int compare(int slot1, int slot2) {
-      final double v1 = values[slot1];
-      final double v2 = values[slot2];
-      if (v1 > v2) {
-        return 1;
-      } else if (v1 < v2) {
-        return -1;
-      } else {
-        return 0;
-      }
+      return Double.compare(values[slot1], values[slot2]);
     }
 
     @Override
@@ -327,13 +318,7 @@ public abstract class FieldComparator<T>
         v2 = missingValue;
       }
 
-      if (bottom > v2) {
-        return 1;
-      } else if (bottom < v2) {
-        return -1;
-      } else {
-        return 0;
-      }
+      return Double.compare(bottom, v2);
     }
 
     @Override
@@ -375,13 +360,7 @@ public abstract class FieldComparator<T>
       if (docsWithField != null && docValue == 0 && !docsWithField.get(doc))
{
         docValue = missingValue;
       }
-      if (docValue < value) {
-        return -1;
-      } else if (docValue > value) {
-        return 1;
-      } else {
-        return 0;
-      }
+      return Double.compare(docValue, value);
     }
   }
 
@@ -401,17 +380,7 @@ public abstract class FieldComparator<T>
     
     @Override
     public int compare(int slot1, int slot2) {
-      // TODO: are there sneaky non-branch ways to compute
-      // sign of float?
-      final float v1 = values[slot1];
-      final float v2 = values[slot2];
-      if (v1 > v2) {
-        return 1;
-      } else if (v1 < v2) {
-        return -1;
-      } else {
-        return 0;
-      }
+      return Float.compare(values[slot1], values[slot2]);
     }
 
     @Override
@@ -423,14 +392,8 @@ public abstract class FieldComparator<T>
       if (docsWithField != null && v2 == 0 && !docsWithField.get(doc)) {
         v2 = missingValue;
       }
-      
-      if (bottom > v2) {
-        return 1;
-      } else if (bottom < v2) {
-        return -1;
-      } else {
-        return 0;
-      }
+
+      return Float.compare(bottom, v2);
     }
 
     @Override
@@ -472,13 +435,7 @@ public abstract class FieldComparator<T>
       if (docsWithField != null && docValue == 0 && !docsWithField.get(doc))
{
         docValue = missingValue;
       }
-      if (docValue < value) {
-        return -1;
-      } else if (docValue > value) {
-        return 1;
-      } else {
-        return 0;
-      }
+      return Float.compare(docValue, value);
     }
   }
 
@@ -773,16 +730,14 @@ public abstract class FieldComparator<T>
 
     @Override
     public int compare(int slot1, int slot2) {
-      final float score1 = scores[slot1];
-      final float score2 = scores[slot2];
-      return score1 > score2 ? -1 : (score1 < score2 ? 1 : 0);
+      return Float.compare(scores[slot2], scores[slot1]);
     }
 
     @Override
     public int compareBottom(int doc) throws IOException {
       float score = scorer.score();
       assert !Float.isNaN(score);
-      return bottom > score ? -1 : (bottom < score ? 1 : 0);
+      return Float.compare(score, bottom);
     }
 
     @Override
@@ -831,15 +786,7 @@ public abstract class FieldComparator<T>
       final float value = valueObj.floatValue();
       float docValue = scorer.score();
       assert !Float.isNaN(docValue);
-      if (docValue < value) {
-        // reverse of FloatComparator
-        return 1;
-      } else if (docValue > value) {
-        // reverse of FloatComparator
-        return -1;
-      } else {
-        return 0;
-      }
+      return Float.compare(value, docValue);
     }
   }
 

Modified: lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/SearcherLifetimeManager.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/SearcherLifetimeManager.java?rev=1468638&r1=1468637&r2=1468638&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/SearcherLifetimeManager.java
(original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/search/SearcherLifetimeManager.java
Tue Apr 16 22:09:49 2013
@@ -119,16 +119,7 @@ public class SearcherLifetimeManager imp
     // Newer searchers are sort before older ones:
     @Override
     public int compareTo(SearcherTracker other) {
-      // Be defensive: cannot subtract since it could
-      // technically overflow long, though, we'd never hit
-      // that in practice:
-      if (recordTimeSec < other.recordTimeSec) {
-        return 1;
-      } else if (other.recordTimeSec < recordTimeSec) {
-        return -1;
-      } else {
-        return 0;
-      }
+      return Double.compare(other.recordTimeSec, recordTimeSec);
     }
 
     @Override

Modified: lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSort.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSort.java?rev=1468638&r1=1468637&r2=1468638&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSort.java
(original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSort.java
Tue Apr 16 22:09:49 2013
@@ -964,6 +964,33 @@ public class TestSort extends LuceneTest
     dir.close();
   }
   
+  /** Tests sorting on type double with +/- zero */
+  public void testDoubleSignedZero() throws IOException {
+    Directory dir = newDirectory();
+    RandomIndexWriter writer = new RandomIndexWriter(random(), dir);
+    Document doc = new Document();
+    doc.add(newStringField("value", "+0", Field.Store.YES));
+    writer.addDocument(doc);
+    doc = new Document();
+    doc.add(newStringField("value", "-0", Field.Store.YES));
+    writer.addDocument(doc);
+    doc = new Document();
+    IndexReader ir = writer.getReader();
+    writer.close();
+    
+    IndexSearcher searcher = newSearcher(ir);
+    Sort sort = new Sort(new SortField("value", SortField.Type.DOUBLE));
+
+    TopDocs td = searcher.search(new MatchAllDocsQuery(), 10, sort);
+    assertEquals(2, td.totalHits);
+    // numeric order
+    assertEquals("-0", searcher.doc(td.scoreDocs[0].doc).get("value"));
+    assertEquals("+0", searcher.doc(td.scoreDocs[1].doc).get("value"));
+
+    ir.close();
+    dir.close();
+  }
+  
   /** Tests sorting on type double with a missing value */
   public void testDoubleMissing() throws IOException {
     Directory dir = newDirectory();

Modified: lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSortDocValues.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSortDocValues.java?rev=1468638&r1=1468637&r2=1468638&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSortDocValues.java
(original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSortDocValues.java
Tue Apr 16 22:09:49 2013
@@ -604,6 +604,35 @@ public class TestSortDocValues extends L
     dir.close();
   }
   
+  /** Tests sorting on type double with +/- zero */
+  public void testDoubleSignedZero() throws IOException {
+    Directory dir = newDirectory();
+    RandomIndexWriter writer = new RandomIndexWriter(random(), dir);
+    Document doc = new Document();
+    doc.add(new DoubleDocValuesField("value", +0D));
+    doc.add(newStringField("value", "+0", Field.Store.YES));
+    writer.addDocument(doc);
+    doc = new Document();
+    doc.add(new DoubleDocValuesField("value", -0D));
+    doc.add(newStringField("value", "-0", Field.Store.YES));
+    writer.addDocument(doc);
+    doc = new Document();
+    IndexReader ir = writer.getReader();
+    writer.close();
+    
+    IndexSearcher searcher = newSearcher(ir);
+    Sort sort = new Sort(new SortField("value", SortField.Type.DOUBLE));
+
+    TopDocs td = searcher.search(new MatchAllDocsQuery(), 10, sort);
+    assertEquals(2, td.totalHits);
+    // numeric order
+    assertEquals("-0", searcher.doc(td.scoreDocs[0].doc).get("value"));
+    assertEquals("+0", searcher.doc(td.scoreDocs[1].doc).get("value"));
+
+    ir.close();
+    dir.close();
+  }
+  
   /** Tests sorting on type double in reverse */
   public void testDoubleReverse() throws IOException {
     Directory dir = newDirectory();

Modified: lucene/dev/branches/branch_4x/lucene/queries/src/java/org/apache/lucene/queries/function/ValueSource.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/queries/src/java/org/apache/lucene/queries/function/ValueSource.java?rev=1468638&r1=1468637&r2=1468638&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/queries/src/java/org/apache/lucene/queries/function/ValueSource.java
(original)
+++ lucene/dev/branches/branch_4x/lucene/queries/src/java/org/apache/lucene/queries/function/ValueSource.java
Tue Apr 16 22:09:49 2013
@@ -139,28 +139,13 @@ public abstract class ValueSource {
 
     @Override
     public int compare(int slot1, int slot2) {
-      final double v1 = values[slot1];
-      final double v2 = values[slot2];
-      if (v1 > v2) {
-        return 1;
-      } else if (v1 < v2) {
-        return -1;
-      } else {
-        return 0;
-      }
+      return Double.compare(values[slot1], values[slot2]);
 
     }
 
     @Override
     public int compareBottom(int doc) {
-      final double v2 = docVals.doubleVal(doc);
-      if (bottom > v2) {
-        return 1;
-      } else if (bottom < v2) {
-        return -1;
-      } else {
-        return 0;
-      }
+      return Double.compare(bottom, docVals.doubleVal(doc));
     }
 
     @Override
@@ -188,13 +173,7 @@ public abstract class ValueSource {
     public int compareDocToValue(int doc, Double valueObj) {
       final double value = valueObj;
       final double docValue = docVals.doubleVal(doc);
-      if (docValue < value) {
-        return -1;
-      } else if (docValue > value) {
-        return 1;
-      } else {
-        return 0;
-      }
+      return Double.compare(docValue, value);
     }
   }
 }

Modified: lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/schema/DocValuesTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/schema/DocValuesTest.java?rev=1468638&r1=1468637&r2=1468638&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/schema/DocValuesTest.java
(original)
+++ lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/schema/DocValuesTest.java
Tue Apr 16 22:09:49 2013
@@ -122,6 +122,26 @@ public class DocValuesTest extends SolrT
     assertQ(req("q", "*:*", "sort", "stringdv asc", "rows", "1", "fl", "id"),
         "//str[@name='id'][.='2']");
   }
+  
+  public void testDocValuesSorting2() {
+    assertU(adoc("id", "1", "doubledv", "12"));
+    assertU(adoc("id", "2", "doubledv", "50.567"));
+    assertU(adoc("id", "3", "doubledv", "+0"));
+    assertU(adoc("id", "4", "doubledv", "4.9E-324"));
+    assertU(adoc("id", "5", "doubledv", "-0"));
+    assertU(adoc("id", "6", "doubledv", "-5.123"));
+    assertU(adoc("id", "7", "doubledv", "1.7976931348623157E308"));
+    assertU(commit());
+    assertQ(req("fl", "id", "q", "*:*", "sort", "doubledv asc"),
+        "//result/doc[1]/str[@name='id'][.='6']",
+        "//result/doc[2]/str[@name='id'][.='5']",
+        "//result/doc[3]/str[@name='id'][.='3']",
+        "//result/doc[4]/str[@name='id'][.='4']",
+        "//result/doc[5]/str[@name='id'][.='1']",
+        "//result/doc[6]/str[@name='id'][.='2']",
+        "//result/doc[7]/str[@name='id'][.='7']"
+        );
+  }
 
   public void testDocValuesFaceting() {
     for (int i = 0; i < 50; ++i) {



Mime
View raw message