SOLR-2198: handle null values in collapse field

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1038295 13f79535-47bb-0310-9956-ffa450edef68

solr/src/java/org/apache/solr/schema/FieldType.java

@@ -512,7 +512,8 @@ public abstract class FieldType extends FieldProperties {
    */
   @Deprecated
   public ValueSource getValueSource(SchemaField field) {
-    return new OrdFieldSource(field.name);
+    // return new OrdFieldSource(field.name);
+    return new StrFieldSource(field.name);
   }
 
   /**

solr/src/java/org/apache/solr/schema/SortableDoubleField.java

@@ -107,6 +107,8 @@ class SortableDoubleFieldSource extends FieldCacheSource {
     final double def = defVal;
 
     return new StringIndexDocValues(this, reader, field) {
+      private final BytesRef spare = new BytesRef();
+
       protected String toTerm(String readableValue) {
         return NumberUtils.double2sortableStr(readableValue);
       }
@@ -125,7 +127,7 @@ class SortableDoubleFieldSource extends FieldCacheSource {
 
       public double doubleVal(int doc) {
         int ord=termsIndex.getOrd(doc);
-        return ord==0 ? def  : NumberUtils.SortableStr2double(termsIndex.lookup(ord, new BytesRef()));
+        return ord==0 ? def  : NumberUtils.SortableStr2double(termsIndex.lookup(ord, spare));
       }
 
       public String strVal(int doc) {
@@ -148,7 +150,14 @@ class SortableDoubleFieldSource extends FieldCacheSource {
 
           @Override
           public void fillValue(int doc) {
-            mval.value = doubleVal(doc);
+            int ord=termsIndex.getOrd(doc);
+            if (ord == 0) {
+              mval.value = def;
+              mval.exists = false;
+            } else {
+              mval.value = NumberUtils.SortableStr2double(termsIndex.lookup(ord, spare));
+              mval.exists = true;
+            }
           }
         };
       }

solr/src/java/org/apache/solr/schema/SortableFloatField.java

@@ -107,13 +107,15 @@ class SortableFloatFieldSource extends FieldCacheSource {
     final float def = defVal;
 
     return new StringIndexDocValues(this, reader, field) {
+      private final BytesRef spare = new BytesRef();
+
       protected String toTerm(String readableValue) {
         return NumberUtils.float2sortableStr(readableValue);
       }
 
       public float floatVal(int doc) {
         int ord=termsIndex.getOrd(doc);
-        return ord==0 ? def  : NumberUtils.SortableStr2float(termsIndex.lookup(ord, new BytesRef()));
+        return ord==0 ? def  : NumberUtils.SortableStr2float(termsIndex.lookup(ord, spare));
       }
 
       public int intVal(int doc) {
@@ -148,7 +150,14 @@ class SortableFloatFieldSource extends FieldCacheSource {
 
           @Override
           public void fillValue(int doc) {
-            mval.value = floatVal(doc);
+            int ord=termsIndex.getOrd(doc);
+            if (ord == 0) {
+              mval.value = def;
+              mval.exists = false;
+            } else {
+              mval.value = NumberUtils.SortableStr2float(termsIndex.lookup(ord, spare));
+              mval.exists = true;
+            }
           }
         };
       }

solr/src/java/org/apache/solr/schema/SortableIntField.java

@@ -111,6 +111,8 @@ class SortableIntFieldSource extends FieldCacheSource {
     final int def = defVal;
 
     return new StringIndexDocValues(this, reader, field) {
+      private final BytesRef spare = new BytesRef();
+
       protected String toTerm(String readableValue) {
         return NumberUtils.int2sortableStr(readableValue);
       }
@@ -121,7 +123,7 @@ class SortableIntFieldSource extends FieldCacheSource {
 
       public int intVal(int doc) {
         int ord=termsIndex.getOrd(doc);
-        return ord==0 ? def  : NumberUtils.SortableStr2int(termsIndex.lookup(ord, new BytesRef()),0,3);
+        return ord==0 ? def  : NumberUtils.SortableStr2int(termsIndex.lookup(ord, spare),0,3);
       }
 
       public long longVal(int doc) {
@@ -152,7 +154,14 @@ class SortableIntFieldSource extends FieldCacheSource {
 
           @Override
           public void fillValue(int doc) {
-            mval.value = intVal(doc);
+            int ord=termsIndex.getOrd(doc);
+            if (ord == 0) {
+              mval.value = def;
+              mval.exists = false;
+            } else {
+              mval.value = NumberUtils.SortableStr2int(termsIndex.lookup(ord, spare),0,3);
+              mval.exists = true;
+            }
           }
         };
       }

solr/src/java/org/apache/solr/schema/SortableLongField.java

@@ -108,6 +108,8 @@ class SortableLongFieldSource extends FieldCacheSource {
     final long def = defVal;
 
     return new StringIndexDocValues(this, reader, field) {
+      private final BytesRef spare = new BytesRef();
+
       protected String toTerm(String readableValue) {
         return NumberUtils.long2sortableStr(readableValue);
       }
@@ -122,7 +124,7 @@ class SortableLongFieldSource extends FieldCacheSource {
 
       public long longVal(int doc) {
         int ord=termsIndex.getOrd(doc);
-        return ord==0 ? def  : NumberUtils.SortableStr2long(termsIndex.lookup(ord, new BytesRef()),0,5);
+        return ord==0 ? def  : NumberUtils.SortableStr2long(termsIndex.lookup(ord, spare),0,5);
       }
 
       public double doubleVal(int doc) {
@@ -149,7 +151,14 @@ class SortableLongFieldSource extends FieldCacheSource {
 
           @Override
           public void fillValue(int doc) {
-            mval.value = longVal(doc);
+            int ord=termsIndex.getOrd(doc);
+            if (ord == 0) {
+              mval.value = def;
+              mval.exists = false;
+            } else {
+              mval.value = NumberUtils.SortableStr2long(termsIndex.lookup(ord, spare),0,5);
+              mval.exists = true;
+            }
           }
         };
       }

solr/src/java/org/apache/solr/search/MutableValue.java

@@ -18,7 +18,7 @@ package org.apache.solr.search;
 
 /** @lucene.internal */
 public abstract class MutableValue implements Comparable {
-  protected boolean exists = true;
+  public boolean exists = true;
 
   public abstract void copy(MutableValue source);
   public abstract MutableValue duplicate();
@@ -47,7 +47,7 @@ public abstract class MutableValue implements Comparable {
   public boolean equals(Object other) {
     Class c1 = this.getClass();
     Class c2 = other.getClass();
-    return (c1 == c2) ? this.equalsSameType(other) : false;
+    return (c1 == c2) && this.equalsSameType(other);
   }
 
   public abstract int hashCode();

solr/src/java/org/apache/solr/search/MutableValueDate.java

@@ -21,13 +21,14 @@ import java.util.Date;
 public class MutableValueDate extends MutableValueLong {
   @Override
   public Object toObject() {
-    return new Date(value);
+    return exists ? new Date(value) : null;
   }
 
   @Override
   public MutableValue duplicate() {
     MutableValueDate v = new MutableValueDate();
     v.value = this.value;
+    v.exists = this.exists;
     return v;
   }  
 }

solr/src/java/org/apache/solr/search/MutableValueDouble.java

@@ -21,29 +21,38 @@ public class MutableValueDouble extends MutableValue {
 
   @Override
   public Object toObject() {
-    return value;
+    return exists ? value : null;
   }
 
   @Override
   public void copy(MutableValue source) {
-    value = ((MutableValueDouble)source).value;
+    MutableValueDouble s = (MutableValueDouble) source;
+    value = s.value;
+    exists = s.exists;
   }
 
   @Override
   public MutableValue duplicate() {
     MutableValueDouble v = new MutableValueDouble();
     v.value = this.value;
+    v.exists = this.exists;
     return v;
   }
 
   @Override
   public boolean equalsSameType(Object other) {
-    return value == ((MutableValueDouble)other).value;
+    MutableValueDouble b = (MutableValueDouble)other;
+    return value == b.value && exists == b.exists;
   }
 
   @Override
   public int compareSameType(Object other) {
-    return Double.compare(value, ((MutableValueDouble)other).value);  // handles NaN
+    MutableValueDouble b = (MutableValueDouble)other;
+    int c = Double.compare(value, b.value);
+    if (c != 0) return c;
+    if (!exists) return -1;
+    if (!b.exists) return 1;
+    return 0;
   }
 
   @Override

solr/src/java/org/apache/solr/search/MutableValueFloat.java

@@ -21,29 +21,37 @@ public class MutableValueFloat extends MutableValue {
 
   @Override
   public Object toObject() {
-    return value;
+    return exists ? value : null;
   }
 
   @Override
   public void copy(MutableValue source) {
-    value = ((MutableValueFloat)source).value;
+    MutableValueFloat s = (MutableValueFloat) source;
+    value = s.value;
+    exists = s.exists;
   }
 
   @Override
   public MutableValue duplicate() {
     MutableValueFloat v = new MutableValueFloat();
     v.value = this.value;
+    v.exists = this.exists;
     return v;
   }
 
   @Override
   public boolean equalsSameType(Object other) {
-    return value == ((MutableValueFloat)other).value;
+    MutableValueFloat b = (MutableValueFloat)other;
+    return value == b.value && exists == b.exists;
   }
 
   @Override
   public int compareSameType(Object other) {
-    return Float.compare(value, ((MutableValueFloat)other).value);  // handles NaN
+    MutableValueFloat b = (MutableValueFloat)other;
+    int c = Float.compare(value, b.value);
+    if (c != 0) return c;
+    if (exists == b.exists) return 0;
+    return exists ? 1 : -1;
   }
 
   @Override

solr/src/java/org/apache/solr/search/MutableValueInt.java

@@ -21,38 +21,46 @@ public class MutableValueInt extends MutableValue {
   
   @Override
   public Object toObject() {
-    return value;
+    return exists ? value : null;
   }
 
   @Override
   public void copy(MutableValue source) {
-    value = ((MutableValueInt)source).value;
+    MutableValueInt s = (MutableValueInt) source;
+    value = s.value;
+    exists = s.exists;
   }
 
   @Override
   public MutableValue duplicate() {
     MutableValueInt v = new MutableValueInt();
     v.value = this.value;
+    v.exists = this.exists;
     return v;
   }
 
   @Override
   public boolean equalsSameType(Object other) {
-    return value == ((MutableValueInt)other).value;
+    MutableValueInt b = (MutableValueInt)other;
+    return value == b.value && exists == b.exists;
   }
 
   @Override
   public int compareSameType(Object other) {
-    int a = value;
+    MutableValueInt b = (MutableValueInt)other;
-    int b = ((MutableValueInt)other).value;
+    int ai = value;
-    return (int)((((long)a) - ((long)b)) >> 32);  // any shift >= 32 should do.
+    int bi = b.value;
-
+    int c = (int)((((long)ai) - ((long)bi)) >> 32);  // any shift >= 32 should do.
+    if (c!=0) return c;
     /* is there any pattern that the compiler would recognize as a single native CMP instruction? */
     /***
     if (a<b) return -1;
     else if (a>b) return 1;
     else return 0;
     ***/
+
+    if (exists == b.exists) return 0;
+    return exists ? 1 : -1;
   }
 
 

solr/src/java/org/apache/solr/search/MutableValueLong.java

@@ -21,32 +21,38 @@ public class MutableValueLong extends MutableValue {
 
   @Override
   public Object toObject() {
-    return value;
+    return exists ? value : null;
   }
 
   @Override
   public void copy(MutableValue source) {
-    value = ((MutableValueLong)source).value;
+    MutableValueLong s = (MutableValueLong) source;
+    exists = s.exists;
+    value = s.value;
   }
 
   @Override
   public MutableValue duplicate() {
     MutableValueLong v = new MutableValueLong();
     v.value = this.value;
+    v.exists = this.exists;
     return v;
   }
 
   @Override
   public boolean equalsSameType(Object other) {
-    return value == ((MutableValueLong)other).value;
+    MutableValueLong b = (MutableValueLong)other;
+    return value == b.value && exists == b.exists;
   }
 
   @Override
   public int compareSameType(Object other) {
-    long b = ((MutableValueLong)other).value;
+    MutableValueLong b = (MutableValueLong)other;
-    if (value<b) return -1;
+    long bv = b.value;
-    else if (value>b) return 1;
+    if (value<bv) return -1;
-    else return 0;
+    if (value>bv) return 1;
+    if (exists == b.exists) return 0;
+    return exists ? 1 : -1;
   }
 
 

solr/src/java/org/apache/solr/search/MutableValueStr.java

@@ -24,29 +24,37 @@ public class MutableValueStr extends MutableValue {
 
   @Override
   public Object toObject() {
-    return ByteUtils.UTF8toUTF16(value);
+    return exists ? ByteUtils.UTF8toUTF16(value) : null;
   }
 
   @Override
   public void copy(MutableValue source) {
-    value.copy(((MutableValueStr)source).value);
+    MutableValueStr s = (MutableValueStr) source;
+    exists = s.exists;
+    value.copy(s.value);
   }
 
   @Override
   public MutableValue duplicate() {
     MutableValueStr v = new MutableValueStr();
-    v.value = new BytesRef(value);
+    v.value.copy(value);
+    v.exists = this.exists;
     return v;
   }
 
   @Override
   public boolean equalsSameType(Object other) {
-    return value.equals(((MutableValueStr)other).value);
+    MutableValueStr b = (MutableValueStr)other;
+    return value.equals(b.value) && exists == b.exists;
   }
 
   @Override
   public int compareSameType(Object other) {
-    return value.compareTo(((MutableValueStr)other).value);
+    MutableValueStr b = (MutableValueStr)other;
+    int c = value.compareTo(b.value);
+    if (c != 0) return c;
+    if (exists == b.exists) return 0;
+    return exists ? 1 : -1;
   }
 
 

solr/src/java/org/apache/solr/search/function/DoubleFieldSource.java

@@ -18,6 +18,7 @@
 package org.apache.solr.search.function;
 
 import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.util.Bits;
 import org.apache.lucene.search.FieldCache;
 import org.apache.lucene.search.cache.DoubleValuesCreator;
 import org.apache.lucene.search.cache.FloatValuesCreator;
@@ -50,6 +51,7 @@ public class DoubleFieldSource extends NumericFieldCacheSource<DoubleValues> {
   public DocValues getValues(Map context, IndexReader reader) throws IOException {
     final DoubleValues vals = cache.getDoubles(reader, field, creator);
     final double[] arr = vals.values;
+	final Bits valid = vals.valid;
     
     return new DocValues() {
       public float floatVal(int doc) {
@@ -148,6 +150,7 @@ public class DoubleFieldSource extends NumericFieldCacheSource<DoubleValues> {
           @Override
           public void fillValue(int doc) {
             mval.value = doubleArr[doc];
+            mval.exists = valid.get(doc);
           }
         };
       }

solr/src/java/org/apache/solr/search/function/FloatFieldSource.java

@@ -21,6 +21,7 @@ import java.io.IOException;
 import java.util.Map;
 
 import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.util.Bits;
 import org.apache.lucene.search.cache.FloatValuesCreator;
 import org.apache.lucene.search.cache.CachedArray.FloatValues;
 import org.apache.solr.search.MutableValue;
@@ -47,6 +48,7 @@ public class FloatFieldSource extends NumericFieldCacheSource<FloatValues> {
   public DocValues getValues(Map context, IndexReader reader) throws IOException {
     final FloatValues vals = cache.getFloats(reader, field, creator);
     final float[] arr = vals.values;
+	final Bits valid = vals.valid;
     
     return new DocValues() {
       public float floatVal(int doc) {
@@ -87,6 +89,7 @@ public class FloatFieldSource extends NumericFieldCacheSource<FloatValues> {
           @Override
           public void fillValue(int doc) {
             mval.value = floatArr[doc];
+            mval.exists = valid.get(doc);
           }
         };
       }

solr/src/java/org/apache/solr/search/function/IntFieldSource.java

@@ -18,6 +18,7 @@
 package org.apache.solr.search.function;
 
 import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.util.Bits;
 import org.apache.solr.search.MutableValueInt;
 import org.apache.solr.search.MutableValue;
 import org.apache.lucene.search.FieldCache;
@@ -51,6 +52,7 @@ public class IntFieldSource extends NumericFieldCacheSource<IntValues> {
   public DocValues getValues(Map context, IndexReader reader) throws IOException {
     final IntValues vals = cache.getInts(reader, field, creator);
     final int[] arr = vals.values;
+	final Bits valid = vals.valid;
     
     return new DocValues() {
       final MutableValueInt val = new MutableValueInt();
@@ -127,6 +129,7 @@ public class IntFieldSource extends NumericFieldCacheSource<IntValues> {
           @Override
           public void fillValue(int doc) {
             mval.value = intArr[doc];
+            mval.exists = valid.get(doc);
           }
         };
       }

solr/src/java/org/apache/solr/search/function/LongFieldSource.java

@@ -18,6 +18,7 @@
 package org.apache.solr.search.function;
 
 import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.util.Bits;
 import org.apache.lucene.search.cache.LongValuesCreator;
 import org.apache.lucene.search.cache.CachedArray.LongValues;
 import org.apache.solr.search.MutableValue;
@@ -52,6 +53,7 @@ public class LongFieldSource extends NumericFieldCacheSource<LongValues> {
   public DocValues getValues(Map context, IndexReader reader) throws IOException {
     final LongValues vals = cache.getLongs(reader, field, creator);
     final long[] arr = vals.values;
+	final Bits valid = vals.valid;
     
     return new DocValues() {
       public float floatVal(int doc) {
@@ -126,6 +128,7 @@ public class LongFieldSource extends NumericFieldCacheSource<LongValues> {
           @Override
           public void fillValue(int doc) {
             mval.value = longArr[doc];
+            mval.exists = valid.get(doc);
           }
         };
       }

solr/src/java/org/apache/solr/search/function/OrdFieldSource.java

@@ -18,6 +18,9 @@
 package org.apache.solr.search.function;
 
 import org.apache.lucene.index.IndexReader;
+import org.apache.solr.search.MutableValue;
+import org.apache.solr.search.MutableValueInt;
+import org.apache.solr.util.NumberUtils;
 
 import java.io.IOException;
 import java.util.Map;
@@ -89,6 +92,24 @@ public class OrdFieldSource extends ValueSource {
 
       public String toString(int doc) {
         return description() + '=' + intVal(doc);
+      }
+
+            @Override
+      public ValueFiller getValueFiller() {
+        return new ValueFiller() {
+          private final MutableValueInt mval = new MutableValueInt();
+
+          @Override
+          public MutableValue getValue() {
+            return mval;
+          }
+
+          @Override
+          public void fillValue(int doc) {
+            mval.value = termsIndex.getOrd(doc);
+            mval.exists = mval.value!=0;
+          }
+        };
       }
     };
   }

solr/src/java/org/apache/solr/search/function/StringIndexDocValues.java

@@ -104,7 +104,9 @@ public abstract class StringIndexDocValues extends DocValues {
 
       @Override
       public void fillValue(int doc) {
-        mval.value = termsIndex.getTerm(doc, val.value);
+        int ord = termsIndex.getOrd(doc);
+        mval.exists = ord != 0;
+        mval.value = termsIndex.lookup(ord, mval.value);
       }
     };
   }

solr/src/test/org/apache/solr/SolrTestCaseJ4.java

@@ -907,16 +907,16 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
       if (which == fieldTypes.size()) {
         // sort by score
         sortSpec.append("score").append(asc ? " asc" : " desc");
-        comparators.add(createComparator("score", asc, false, false));
+        comparators.add(createComparator("score", asc, false, false, false));
       } else if (which == fieldTypes.size() + 1) {
         // sort by docid
         sortSpec.append("_docid_").append(asc ? " asc" : " desc");
-        comparators.add(createComparator("_docid_", asc, false, false));
+        comparators.add(createComparator("_docid_", asc, false, false, false));
       } else {
         String field = fieldTypes.get(which).fname;
         sortSpec.append(field).append(asc ? " asc" : " desc");
         SchemaField sf = schema.getField(field);
-        comparators.add(createComparator(field, asc, sf.sortMissingLast(), sf.sortMissingFirst()));
+        comparators.add(createComparator(field, asc, sf.sortMissingLast(), sf.sortMissingFirst(), !(sf.sortMissingLast()||sf.sortMissingFirst()) ));
       }
     }
 
@@ -924,13 +924,13 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
 
     if (comparators.size() == 0) {
       // default sort is by score desc
-      comparators.add(createComparator("score", false, false, false));      
+      comparators.add(createComparator("score", false, false, false, false));      
     }
 
     return createComparator(comparators);
   }
 
-  public static Comparator<Doc> createComparator(final String field, final boolean asc, final boolean sortMissingLast, final boolean sortMissingFirst) {
+  public static Comparator<Doc> createComparator(final String field, final boolean asc, final boolean sortMissingLast, final boolean sortMissingFirst, final boolean sortMissingAsZero) {
     final int mul = asc ? 1 : -1;
 
     if (field.equals("_docid_")) {
@@ -943,15 +943,31 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
     }
 
     if (field.equals("score")) {
-      return createComparator("score_f", asc, sortMissingLast, sortMissingFirst);
+      return createComparator("score_f", asc, sortMissingLast, sortMissingFirst, sortMissingAsZero);
     }
 
     return new Comparator<Doc>() {
+      private Comparable zeroVal(Comparable template) {
+        if (template == null) return null;
+        if (template instanceof String) return null;  // fast-path for string
+        if (template instanceof Integer) return 0;
+        if (template instanceof Long) return (long)0;
+        if (template instanceof Float) return (float)0;
+        if (template instanceof Double) return (double)0;
+        if (template instanceof Short) return (short)0;
+        if (template instanceof Byte) return (byte)0;
+        if (template instanceof Character) return (char)0;
+        return null;
+      }
+
       @Override
       public int compare(Doc o1, Doc o2) {
         Comparable v1 = o1.getFirstValue(field);
         Comparable v2 = o2.getFirstValue(field);
 
+        v1 = v1 == null ? zeroVal(v2) : v1;
+        v2 = v2 == null ? zeroVal(v1) : v2;
+
         int c = 0;
         if (v1 == v2) {
           c = 0;

solr/src/test/org/apache/solr/TestGroupingSearch.java

@@ -326,14 +326,14 @@ public class TestGroupingSearch extends SolrTestCaseJ4 {
     while (--indexIter >= 0) {
 
       int indexSize = random.nextInt(25 * RANDOM_MULTIPLIER);
-
+//indexSize=2;
       List<FldType> types = new ArrayList<FldType>();
       types.add(new FldType("id",ONE_ONE, new SVal('A','Z',4,4)));
       types.add(new FldType("score_f",ONE_ONE, new FVal(1,100)));  // field used to score
-      types.add(new FldType("foo_i",ONE_ONE, new IRange(0,indexSize)));
+      types.add(new FldType("foo_i",ZERO_ONE, new IRange(0,indexSize)));
-      types.add(new FldType("foo_s",ONE_ONE, new SVal('a','z',1,2)));
+      types.add(new FldType("foo_s",ZERO_ONE, new SVal('a','z',1,2)));
-      types.add(new FldType("small_s",ONE_ONE, new SVal('a',(char)('c'+indexSize/10),1,1)));
+      types.add(new FldType("small_s",ZERO_ONE, new SVal('a',(char)('c'+indexSize/10),1,1)));
-      types.add(new FldType("small_i",ONE_ONE, new IRange(0,5+indexSize/10)));
+      types.add(new FldType("small_i",ZERO_ONE, new IRange(0,5+indexSize/10)));
 
       clearIndex();
       Map<Comparable, Doc> model = indexDocs(types, null, indexSize);
@@ -403,9 +403,9 @@ public class TestGroupingSearch extends SolrTestCaseJ4 {
          // Test specific case
         if (false) {
           groupField="small_i";
-          sortComparator=createComparator(Arrays.asList(createComparator("small_s", true, true, false)));
+          sortComparator=createComparator(Arrays.asList(createComparator("small_s", true, true, false, true)));
           sortStr = "small_s asc";
-          groupComparator = createComparator(Arrays.asList(createComparator("small_s", true, true, false)));
+          groupComparator = createComparator(Arrays.asList(createComparator("small_s", true, true, false, false)));
           groupSortStr = "small_s asc";
           rows=1; start=0; group_offset=1; group_limit=1;
         }