SOLR-4504: Fixed CurrencyField range queries to correctly exclude documents w/o values

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

solr/CHANGES.txt

@@ -173,6 +173,9 @@ Bug Fixes
   AnalysisSPILoader when doing concurrent core loads in multicore
   Solr configs.  (Uwe Schindler, Hossman)
 
+* SOLR-4504: Fixed CurrencyField range queries to correctly exclude 
+  documents w/o values (hossman)
+
 Optimizations
 ----------------------
 

solr/core/src/java/org/apache/solr/schema/CurrencyField.java

@@ -24,6 +24,9 @@ import org.apache.lucene.queries.function.FunctionValues;
 import org.apache.lucene.queries.function.ValueSource;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.SortField;
+import org.apache.lucene.search.Filter;
+import org.apache.lucene.search.FieldValueFilter;
+import org.apache.lucene.queries.ChainedFilter;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrException.ErrorCode;
 import org.apache.solr.response.TextResponseWriter;
@@ -240,10 +243,19 @@ public class CurrencyField extends FieldType implements SchemaAware, ResourceLoa
   public Query getRangeQuery(QParser parser, SchemaField field, final CurrencyValue p1, final CurrencyValue p2, final boolean minInclusive, final boolean maxInclusive) {
     String currencyCode = (p1 != null) ? p1.getCurrencyCode() :
                           (p2 != null) ? p2.getCurrencyCode() : defaultCurrency;
-    final CurrencyValueSource vs = new CurrencyValueSource(field, currencyCode, parser);
 
-    return new SolrConstantScoreQuery(new ValueSourceRangeFilter(vs,
-            p1 == null ? null : p1.getAmount() + "" , p2 == null ? null : p2.getAmount() + "", minInclusive, maxInclusive));
+    // ValueSourceRangeFilter doesn't check exists(), so we have to
+    final Filter docsWithValues = new FieldValueFilter(getAmountField(field).getName());
+    final Filter vsRangeFilter = new ValueSourceRangeFilter
+      (new CurrencyValueSource(field, currencyCode, parser),
+       p1 == null ? null : p1.getAmount() + "", 
+       p2 == null ? null : p2.getAmount() + "",
+       minInclusive, maxInclusive);
+    final Filter docsInRange = new ChainedFilter
+      (new Filter [] { docsWithValues, vsRangeFilter }, ChainedFilter.AND);
+
+    return new SolrConstantScoreQuery(docsInRange);
+    
   }
 
   @Override
@@ -315,8 +327,21 @@ public class CurrencyField extends FieldType implements SchemaAware, ResourceLoa
           }
         }
 
+        @Override
+        public boolean exists(int doc) {
+          return amounts.exists(doc);
+        }
+        
         @Override
         public long longVal(int doc) {
+          long amount = amounts.longVal(doc);
+          // bail fast using whatever ammounts defaults to if no value
+          // (if we don't do this early, currencyOrd may be < 0, 
+          // causing index bounds exception
+          if ( ! exists(doc) ) {
+            return amount;
+          }
+
           if (!initializedCache) {
             for (int i = 0; i < fractionDigitCache.length; i++) {
               fractionDigitCache[i] = -1;
@@ -325,7 +350,6 @@ public class CurrencyField extends FieldType implements SchemaAware, ResourceLoa
             initializedCache = true;
           }
 
-          long amount = amounts.longVal(doc);
           int currencyOrd = currencies.ordVal(doc);
 
           if (currencyOrd == targetCurrencyOrd) {

solr/core/src/test/org/apache/solr/schema/CurrencyFieldTest.java

@@ -109,9 +109,28 @@ public class CurrencyFieldTest extends SolrTestCaseJ4 {
 
   @Test
   public void testCurrencyRangeSearch() throws Exception {
+    clearIndex();
+    final int emptyDocs = atLeast(50); // times 2
+    final int negDocs = atLeast(5);
+    
+    assertU(adoc("id", "0", "amount", "0,USD")); // 0
+    // lots of docs w/o values
+    for (int i = 100; i <= 100 + emptyDocs; i++) {
+      assertU(adoc("id", "" + i));
+    }
+    // docs with values in ranges we'll query
     for (int i = 1; i <= 10; i++) {
       assertU(adoc("id", "" + i, "amount", i + ",USD"));
     }
+    // more docs w/o values
+    for (int i = 500; i <= 500 + emptyDocs; i++) {
+      assertU(adoc("id", "" + i));
+    }
+    // some negative values
+    for (int i = -100; i > -100 - negDocs; i--) {
+      assertU(adoc("id", "" + i, "amount", i + ",USD"));
+    }
+    assertU(adoc("id", "40", "amount", "0,USD")); // 0
 
     assertU(commit());
 
@@ -145,22 +164,22 @@ public class CurrencyFieldTest extends SolrTestCaseJ4 {
     // Open ended ranges without currency
     assertQ(req("fl", "*,score", "q",
             "amount:[* TO *]"),
-            "//*[@numFound='10']");
+            "//*[@numFound='" + (2 + 10 + negDocs) + "']");
     
     // Open ended ranges with currency
     assertQ(req("fl", "*,score", "q",
             "amount:[*,EUR TO *,EUR]"),
-            "//*[@numFound='10']");
+            "//*[@numFound='" + (2 + 10 + negDocs) + "']");
 
     // Open ended start range without currency
     assertQ(req("fl", "*,score", "q",
             "amount:[* TO 5,USD]"),
-            "//*[@numFound='5']");
+            "//*[@numFound='" + (2 + 5 + negDocs) + "']");
 
     // Open ended start range with currency (currency for the * won't matter)
     assertQ(req("fl", "*,score", "q",
             "amount:[*,USD TO 5,USD]"),
-            "//*[@numFound='5']");
+            "//*[@numFound='" + (2 + 5 + negDocs) + "']");
 
     // Open ended end range
     assertQ(req("fl", "*,score", "q",
@@ -170,6 +189,7 @@ public class CurrencyFieldTest extends SolrTestCaseJ4 {
 
   @Test
   public void testCurrencyPointQuery() throws Exception {
+    clearIndex();
     assertU(adoc("id", "" + 1, "amount", "10.00,USD"));
     assertU(adoc("id", "" + 2, "amount", "15.00,EUR"));
     assertU(commit());
@@ -184,6 +204,8 @@ public class CurrencyFieldTest extends SolrTestCaseJ4 {
 
   @Ignore
   public void testPerformance() throws Exception {
+    clearIndex();
+
     Random r = random();
     int initDocs = 200000;
 
@@ -225,6 +247,8 @@ public class CurrencyFieldTest extends SolrTestCaseJ4 {
 
   @Test
   public void testCurrencySort() throws Exception {
+    clearIndex();
+
     assertU(adoc("id", "" + 1, "amount", "10.00,USD"));
     assertU(adoc("id", "" + 2, "amount", "15.00,EUR"));
     assertU(adoc("id", "" + 3, "amount", "7.00,EUR"));
@@ -238,6 +262,8 @@ public class CurrencyFieldTest extends SolrTestCaseJ4 {
 
   @Test
   public void testMockFieldType() throws Exception {
+    clearIndex();
+
     assertU(adoc("id", "1", "mock_amount", "1.00,USD"));
     assertU(adoc("id", "2", "mock_amount", "1.00,EUR"));
     assertU(adoc("id", "3", "mock_amount", "1.00,NOK"));