lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hoss...@apache.org
Subject svn commit: r1701090 - in /lucene/dev/branches/branch_5x: ./ solr/ solr/core/ solr/core/src/java/org/apache/solr/schema/ solr/core/src/test/org/apache/solr/search/function/
Date Thu, 03 Sep 2015 18:08:04 GMT
Author: hossman
Date: Thu Sep  3 18:08:04 2015
New Revision: 1701090

URL: http://svn.apache.org/r1701090
Log:
SOLR-8001: Fixed bugs in field(foo,min) and field(foo,max) when some docs have no values (merge
r1701081)

Modified:
    lucene/dev/branches/branch_5x/   (props changed)
    lucene/dev/branches/branch_5x/solr/   (props changed)
    lucene/dev/branches/branch_5x/solr/CHANGES.txt   (contents, props changed)
    lucene/dev/branches/branch_5x/solr/core/   (props changed)
    lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieDoubleField.java
    lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieFloatField.java
    lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieIntField.java
    lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieLongField.java
    lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/search/function/TestMinMaxOnMultiValuedField.java

Modified: lucene/dev/branches/branch_5x/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/CHANGES.txt?rev=1701090&r1=1701089&r2=1701090&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/CHANGES.txt (original)
+++ lucene/dev/branches/branch_5x/solr/CHANGES.txt Thu Sep  3 18:08:04 2015
@@ -104,6 +104,9 @@ Bug Fixes
 
 * SOLR-7984: wrong and misleading error message 'no default request handler is registered'
(noble, hossman)
 
+* SOLR-8001: Fixed bugs in field(foo,min) and field(foo,max) when some docs have no values
+  (David Smiley, hossman)
+
 Optimizations
 ----------------------
 

Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieDoubleField.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieDoubleField.java?rev=1701090&r1=1701089&r2=1701090&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieDoubleField.java
(original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieDoubleField.java
Thu Sep  3 18:08:04 2015
@@ -70,6 +70,11 @@ public class TrieDoubleField extends Tri
           @Override
           public double doubleVal(int doc) {
             BytesRef bytes = view.get(doc);
+            if (0 == bytes.length) {
+              // the only way this should be possible is for non existent value
+              assert !exists(doc) : "zero bytes for doc, but exists is true";
+              return 0D;
+            }
             return  NumericUtils.sortableLongToDouble(NumericUtils.prefixCodedToLong(bytes));
           }
 
@@ -90,8 +95,12 @@ public class TrieDoubleField extends Tri
               
               @Override
               public void fillValue(int doc) {
-                mval.exists = exists(doc);
-                mval.value = mval.exists ? doubleVal(doc) : 0.0D;
+                // micro optimized (eliminate at least one redudnent ord check) 
+                //mval.exists = exists(doc);
+                //mval.value = mval.exists ? doubleVal(doc) : 0.0D;
+                BytesRef bytes = view.get(doc);
+                mval.exists = (0 == bytes.length);
+                mval.value = mval.exists ? NumericUtils.sortableLongToDouble(NumericUtils.prefixCodedToLong(bytes))
: 0D;
               }
             };
           }

Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieFloatField.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieFloatField.java?rev=1701090&r1=1701089&r2=1701090&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieFloatField.java
(original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieFloatField.java
Thu Sep  3 18:08:04 2015
@@ -78,6 +78,11 @@ public class TrieFloatField extends Trie
           @Override
           public float floatVal(int doc) {
             BytesRef bytes = view.get(doc);
+            if (0 == bytes.length) {
+              // the only way this should be possible is for non existent value
+              assert !exists(doc) : "zero bytes for doc, but exists is true";
+              return 0F;
+            }
             return  NumericUtils.sortableIntToFloat(NumericUtils.prefixCodedToInt(bytes));
           }
 
@@ -98,8 +103,13 @@ public class TrieFloatField extends Trie
               
               @Override
               public void fillValue(int doc) {
-                mval.exists = exists(doc);
-                mval.value = mval.exists ? floatVal(doc) : 0.0F;
+                // micro optimized (eliminate at least one redudnent ord check) 
+                //mval.exists = exists(doc);
+                //mval.value = mval.exists ? floatVal(doc) : 0.0F;
+                //
+                BytesRef bytes = view.get(doc);
+                mval.exists = (0 == bytes.length);
+                mval.value = mval.exists ? NumericUtils.sortableIntToFloat(NumericUtils.prefixCodedToInt(bytes))
: 0F;
               }
             };
           }

Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieIntField.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieIntField.java?rev=1701090&r1=1701089&r2=1701090&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieIntField.java
(original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieIntField.java
Thu Sep  3 18:08:04 2015
@@ -77,6 +77,11 @@ public class TrieIntField extends TrieFi
           @Override
           public int intVal(int doc) {
             BytesRef bytes = view.get(doc);
+            if (0 == bytes.length) {
+              // the only way this should be possible is for non existent value
+              assert !exists(doc) : "zero bytes for doc, but exists is true";
+              return 0;
+            }
             return NumericUtils.prefixCodedToInt(bytes);
           }
 
@@ -97,8 +102,13 @@ public class TrieIntField extends TrieFi
               
               @Override
               public void fillValue(int doc) {
-                mval.exists = exists(doc);
-                mval.value = mval.exists ? intVal(doc) : 0;
+                // micro optimized (eliminate at least one redudnent ord check) 
+                //mval.exists = exists(doc);
+                //mval.value = mval.exists ? intVal(doc) : 0;
+                //
+                BytesRef bytes = view.get(doc);
+                mval.exists = (0 == bytes.length);
+                mval.value = mval.exists ? NumericUtils.prefixCodedToInt(bytes) : 0;
               }
             };
           }

Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieLongField.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieLongField.java?rev=1701090&r1=1701089&r2=1701090&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieLongField.java
(original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/schema/TrieLongField.java
Thu Sep  3 18:08:04 2015
@@ -64,6 +64,11 @@ public class TrieLongField extends TrieF
           @Override
           public long longVal(int doc) {
             BytesRef bytes = view.get(doc);
+            if (0 == bytes.length) {
+              // the only way this should be possible is for non existent value
+              assert !exists(doc) : "zero bytes for doc, but exists is true";
+              return 0L;
+            }
             return NumericUtils.prefixCodedToLong(bytes);
           }
 
@@ -84,8 +89,13 @@ public class TrieLongField extends TrieF
               
               @Override
               public void fillValue(int doc) {
-                mval.exists = exists(doc);
-                mval.value = mval.exists ? longVal(doc) : 0;
+                // micro optimized (eliminate at least one redudnent ord check) 
+                //mval.exists = exists(doc);
+                //mval.value = mval.exists ? longVal(doc) : 0;
+                //
+                BytesRef bytes = view.get(doc);
+                mval.exists = (0 == bytes.length);
+                mval.value = mval.exists ? NumericUtils.prefixCodedToLong(bytes) : 0L;
               }
             };
           }

Modified: lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/search/function/TestMinMaxOnMultiValuedField.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/search/function/TestMinMaxOnMultiValuedField.java?rev=1701090&r1=1701089&r2=1701090&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/search/function/TestMinMaxOnMultiValuedField.java
(original)
+++ lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/search/function/TestMinMaxOnMultiValuedField.java
Thu Sep  3 18:08:04 2015
@@ -261,6 +261,8 @@ public class TestMinMaxOnMultiValuedFiel
     // extreme's of the data type
     testSimpleValues(fieldname, int.class, Integer.MIN_VALUE, 42, -550);
     testSimpleValues(fieldname, int.class, Integer.MAX_VALUE, 0, Integer.MIN_VALUE);
+
+    testSimpleSort(fieldname, -42, 666);
   }
   
   /** @see #testSimpleValues */
@@ -275,6 +277,8 @@ public class TestMinMaxOnMultiValuedFiel
     // extreme's of the data type
     testSimpleValues(fieldname, long.class, Long.MIN_VALUE, 42L, -550L);
     testSimpleValues(fieldname, long.class, Long.MAX_VALUE, 0L, Long.MIN_VALUE);
+    
+    testSimpleSort(fieldname, -42, 666);
   }
   
   /** @see #testSimpleValues */
@@ -289,6 +293,8 @@ public class TestMinMaxOnMultiValuedFiel
     // extreme's of the data type
     testSimpleValues(fieldname, float.class, Float.NEGATIVE_INFINITY, 42.5F, -550.4F);
     testSimpleValues(fieldname, float.class, Float.POSITIVE_INFINITY, 0.0F, Float.NEGATIVE_INFINITY);
+    
+    testSimpleSort(fieldname, -4.2, 6.66);
   }
   
   /** @see #testSimpleValues */
@@ -303,11 +309,14 @@ public class TestMinMaxOnMultiValuedFiel
     // extreme's of the data type
     testSimpleValues(fieldname, double.class, Double.NEGATIVE_INFINITY, 42.5D, -550.4D);
     testSimpleValues(fieldname, double.class, Double.POSITIVE_INFINITY, 0.0D, Double.NEGATIVE_INFINITY);
+    
+    testSimpleSort(fieldname, -4.2, 6.66);
   }
   
   /** Tests a single doc with a few explicit values, as well as testing exists with and w/o
values */
   protected void testSimpleValues(final String fieldname, final Class clazz, final Comparable...
vals) {
-
+    clearIndex();
+    
     assert 0 < vals.length;
     
     Comparable min = vals[0];
@@ -327,7 +336,8 @@ public class TestMinMaxOnMultiValuedFiel
     assertU(adoc(doc1));
     assertU(adoc(sdoc("id", "2"))); // fieldname doesn't exist
     assertU(commit());
-    
+
+    // doc with values
     assertQ(fieldname,
             req("q","id:1",
                 "fl","exists_val_min:exists(field("+fieldname+",min))",
@@ -341,6 +351,7 @@ public class TestMinMaxOnMultiValuedFiel
             ,"//"+type+"[@name='val_max']='"+max+"'"
             );
 
+    // doc w/o values
     assertQ(fieldname,
             req("q","id:2",
                 "fl","exists_val_min:exists(field("+fieldname+",min))",
@@ -353,6 +364,87 @@ public class TestMinMaxOnMultiValuedFiel
             ,"count(//"+type+"[@name='val_min'])=0"
             ,"count(//"+type+"[@name='val_max'])=0"
             );
+
+    // sanity check no sort error when there are missing values
+    for (String dir : new String[] { "asc", "desc" }) {
+      for (String mm : new String[] { "min", "max" }) {
+        for (String func : new String[] { "field("+fieldname+","+mm+")",
+                                          "def(field("+fieldname+","+mm+"),42)",
+                                          "sum(32,field("+fieldname+","+mm+"))"  }) {
+          assertQ(fieldname,
+                  req("q","*:*", 
+                      "fl", "id",
+                      "sort", func + " " + dir)
+                  ,"//*[@numFound='2']"
+                  // no assumptions about order for now, see bug: SOLR-8005
+                  ,"//float[@name='id']='1.0'"
+                  ,"//float[@name='id']='2.0'"
+                  );
+        }
+      }
+    }
+  }
+
+  /** 
+   * Tests sort order of min/max realtive to other docs w/o any values.
+   * @param fieldname The field to test
+   * @param negative a "negative" value for this field (ie: in a function context, is less
then the "0")
+   * @param positive a "positive" value for this field (ie: in a function context, is more
then the "0")
+   */
+  protected void testSimpleSort(final String fieldname,
+                                final Comparable negative, final Comparable positive) {
+    clearIndex();
+
+    int numDocsExpected = 1;
+    for (int i = 1; i < 4; i++) { // pos docids
+      if (random().nextBoolean()) {
+        assertU(adoc(sdoc("id",i))); // fieldname doesn't exist
+        numDocsExpected++;
+      }
+    }
+    
+    assertU(adoc(sdoc("id", "0",
+                      fieldname, negative,
+                      fieldname, positive)));
+    
+    for (int i = 1; i < 4; i++) { // neg docids
+      if (random().nextBoolean()) {
+        assertU(adoc(sdoc("id",-i))); // fieldname doesn't exist
+        numDocsExpected++;
+      }
+    }
+    assertU(commit());
+
+    // need to wrap with "def" until SOLR-8005 is resolved
+    assertDocWithValsIsFirst(numDocsExpected, "def(field("+fieldname+",min),0) asc");
+    assertDocWithValsIsLast(numDocsExpected,  "def(field("+fieldname+",min),0) desc");
+    
+    assertDocWithValsIsFirst(numDocsExpected, "def(field("+fieldname+",max),0) desc");
+    assertDocWithValsIsLast(numDocsExpected,  "def(field("+fieldname+",max),0) asc");
+
+    // def wrapper shouldn't be needed since it's already part of another function
+    assertDocWithValsIsFirst(numDocsExpected, "sum(32,field("+fieldname+",max)) desc");
+    assertDocWithValsIsLast(numDocsExpected,  "sum(32,field("+fieldname+",max)) asc");
+    
+    assertDocWithValsIsFirst(numDocsExpected, "sum(32,field("+fieldname+",min)) asc");
+    assertDocWithValsIsLast(numDocsExpected,  "sum(32,field("+fieldname+",min)) desc");
   }
 
+  /** helper for testSimpleSort */
+  private static void assertDocWithValsIsFirst(final int numDocs, final String sort) {
+    assertQ(sort,
+            req("q","*:*", "rows", ""+numDocs, "sort", sort)
+            ,"//result[@numFound='"+numDocs+"']"
+            ,"//result/doc[1]/float[@name='id']='0.0'"
+            );
+  }
+  /** helper for testSimpleSort */
+  private static void assertDocWithValsIsLast(final int numDocs, final String sort) {
+    assertQ(sort,
+            req("q","*:*", "rows", ""+numDocs, "sort", sort)
+            ,"//result[@numFound='"+numDocs+"']"
+            ,"//result/doc["+numDocs+"]/float[@name='id']='0.0'"
+            );
+  }
+  
 }



Mime
View raw message