From fe0ca7336f12152e67c69e49701a6060e323bc37 Mon Sep 17 00:00:00 2001 From: "Chris M. Hostetter" Date: Tue, 26 Feb 2013 17:46:17 +0000 Subject: [PATCH] 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 | 3 ++ .../org/apache/solr/schema/CurrencyField.java | 32 ++++++++++++++--- .../apache/solr/schema/CurrencyFieldTest.java | 34 ++++++++++++++++--- 3 files changed, 61 insertions(+), 8 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index d41c0c8882d..e10e154701b 100644 --- a/solr/CHANGES.txt +++ b/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 ---------------------- diff --git a/solr/core/src/java/org/apache/solr/schema/CurrencyField.java b/solr/core/src/java/org/apache/solr/schema/CurrencyField.java index 32b7ce7e5b9..a270299bf37 100644 --- a/solr/core/src/java/org/apache/solr/schema/CurrencyField.java +++ b/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) { diff --git a/solr/core/src/test/org/apache/solr/schema/CurrencyFieldTest.java b/solr/core/src/test/org/apache/solr/schema/CurrencyFieldTest.java index 4c3af76a9cc..efe11640e37 100644 --- a/solr/core/src/test/org/apache/solr/schema/CurrencyFieldTest.java +++ b/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"));