From 9e4aaf97809582d2232276d190a4e18197a79923 Mon Sep 17 00:00:00 2001 From: Tomas Fernandez Lobbe Date: Thu, 13 Jul 2017 12:27:36 -0700 Subject: [PATCH] SOLR-11043: Fix facet.range.method=dv and interval facets on single-valued float fields with negative values --- solr/CHANGES.txt | 3 + .../apache/solr/request/IntervalFacets.java | 8 +-- .../apache/solr/request/SimpleFacetsTest.java | 33 ++++++++-- .../solr/request/TestIntervalFaceting.java | 63 ++++++++++++++++++- .../apache/solr/schema/TestPointFields.java | 1 - 5 files changed, 92 insertions(+), 16 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index d6dfe520be4..f1e975e67d5 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -337,6 +337,9 @@ Bug Fixes * SOLR-11045: The new replica created by MoveReplica will have to have same name and coreName as the old one in case of HDFS (Cao Manh Dat) +* SOLR-11043: Fix facet.range.method=dv and interval facets on negative single-valued float fields. + (Tomás Fernández Löbbe, Steve Rowe) + Optimizations ---------------------- diff --git a/solr/core/src/java/org/apache/solr/request/IntervalFacets.java b/solr/core/src/java/org/apache/solr/request/IntervalFacets.java index 9a23a839cd9..1f7cd42f2af 100644 --- a/solr/core/src/java/org/apache/solr/request/IntervalFacets.java +++ b/solr/core/src/java/org/apache/solr/request/IntervalFacets.java @@ -216,9 +216,7 @@ public class IntervalFacets implements Iterable { longs = new FilterNumericDocValues(DocValues.getNumeric(ctx.reader(), fieldName)) { @Override public long longValue() throws IOException { - long bits = super.longValue(); - if (bits < 0) bits ^= 0x7fffffffffffffffL; - return bits; + return NumericUtils.sortableFloatBits((int)super.longValue()); } }; break; @@ -227,9 +225,7 @@ public class IntervalFacets implements Iterable { longs = new FilterNumericDocValues(DocValues.getNumeric(ctx.reader(), fieldName)) { @Override public long longValue() throws IOException { - long bits = super.longValue(); - if (bits < 0) bits ^= 0x7fffffffffffffffL; - return bits; + return NumericUtils.sortableDoubleBits(super.longValue()); } }; break; diff --git a/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java b/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java index 35a66c3e17b..86c57ff7a36 100644 --- a/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java +++ b/solr/core/src/test/org/apache/solr/request/SimpleFacetsTest.java @@ -183,6 +183,27 @@ public class SimpleFacetsTest extends SolrTestCaseJ4 { add_doc("id", "2004", "hotel_s1", "b", "airport_s1", "ams", "duration_i1", "5"); } + public void testDvMethodNegativeFloatRangeFacet() throws Exception { + String field = "negative_num_f1_dv"; + assertTrue(h.getCore().getLatestSchema().getField(field).hasDocValues()); + + final String[] commonParams = { + "q", "*:*", "facet", "true", "facet.range.start", "-2", "facet.range.end", "0", "facet.range.gap", "2" + }; + final String countAssertion + = "//lst[@name='facet_counts']/lst[@name='facet_ranges']/lst[@name='%s']/lst[@name='counts']/int[@name='-2.0'][.='1']"; + + assertU(adoc("id", "10001", field, "-1.0")); + assertU(commit()); + + assertQ(req(commonParams, "facet.range", field, "facet.range.method", "filter"), + String.format(countAssertion, field) + ); + assertQ(req(commonParams, "facet.range", field, "facet.range.method", "dv"), + String.format(countAssertion, field) + ); + } + public void testDefaultsAndAppends() throws Exception { // all defaults @@ -3385,8 +3406,8 @@ public class SimpleFacetsTest extends SolrTestCaseJ4 { ModifiableSolrParams params = new ModifiableSolrParams(); Integer[] values = new Integer[2]; do { - values[0] = random().nextInt(3000); - values[1] = random().nextInt(3000); + values[0] = random().nextInt(3000) * (random().nextBoolean()?-1:1); + values[1] = random().nextInt(3000) * (random().nextBoolean()?-1:1); } while (values[0].equals(values[1])); Arrays.sort(values); long gapNum = Math.max(1, random().nextInt(3000)); @@ -3404,8 +3425,8 @@ public class SimpleFacetsTest extends SolrTestCaseJ4 { ModifiableSolrParams params = new ModifiableSolrParams(); Float[] values = new Float[2]; do { - values[0] = random().nextFloat() * 3000; - values[1] = random().nextFloat() * 3000; + values[0] = random().nextFloat() * 3000 * (random().nextBoolean()?-1:1); + values[1] = random().nextFloat() * 3000 * (random().nextBoolean()?-1:1); } while (values[0].equals(values[1])); Arrays.sort(values); float gapNum = Math.max(1, random().nextFloat() * 3000); @@ -3425,8 +3446,8 @@ public class SimpleFacetsTest extends SolrTestCaseJ4 { ModifiableSolrParams params = new ModifiableSolrParams(); Date[] dates = new Date[2]; do { - dates[0] = new Date((long)(random().nextDouble()*(new Date().getTime()))); - dates[1] = new Date((long)(random().nextDouble()*(new Date().getTime()))); + dates[0] = new Date((long)(random().nextDouble()*(new Date().getTime()) * (random().nextBoolean()?-1:1))); + dates[1] = new Date((long)(random().nextDouble()*(new Date().getTime()) * (random().nextBoolean()?-1:1))); } while (dates[0].equals(dates[1])); Arrays.sort(dates); long dateDiff = (dates[1].getTime() - dates[0].getTime())/1000; diff --git a/solr/core/src/test/org/apache/solr/request/TestIntervalFaceting.java b/solr/core/src/test/org/apache/solr/request/TestIntervalFaceting.java index df0031f1c09..042eb2eece1 100644 --- a/solr/core/src/test/org/apache/solr/request/TestIntervalFaceting.java +++ b/solr/core/src/test/org/apache/solr/request/TestIntervalFaceting.java @@ -673,10 +673,10 @@ public class TestIntervalFaceting extends SolrTestCaseJ4 { @Test public void testFloatFields() { - doTestFloat("test_f_dv"); + doTestFloat("test_f_dv", false); } - private void doTestFloat(String field) { + private void doTestFloat(String field, boolean testDouble) { assertU(adoc("id", "1", field, "0")); assertU(adoc("id", "2", field, "1")); assertU(adoc("id", "3", field, "2")); @@ -705,11 +705,68 @@ public class TestIntervalFaceting extends SolrTestCaseJ4 { assertIntervalQuery(field, "(1,1)", "0"); assertIntervalQuery(field, "(4,7)", "4"); assertIntervalQuery(field, "(123,*)", "1"); + + clearIndex(); + assertU(adoc("id", "16", field, "-1.3")); + assertU(adoc("id", "17", field, "0.0")); + assertU(adoc("id", "18", field, "-0.0")); + assertU(adoc("id", "19", field, String.valueOf(Float.MIN_VALUE))); + assertU(adoc("id", "20", field, String.valueOf(Float.MAX_VALUE))); + assertU(adoc("id", "21", field, String.valueOf(Float.NEGATIVE_INFINITY))); + assertU(adoc("id", "22", field, String.valueOf(Float.POSITIVE_INFINITY))); + assertU(commit()); + + assertIntervalQuery(field, "[*,*]", "7"); + assertIntervalQuery(field, "(*,*)", "7"); + assertIntervalQuery(field, "(-1,1)", "3"); + assertIntervalQuery(field, "(-2,1)", "4"); + assertIntervalQuery(field, "(-1.3,0)", "1"); + assertIntervalQuery(field, "[-1.3,0)", "2"); + assertIntervalQuery(field, "[-1.3,0]", "3"); + assertIntervalQuery(field, "(" + Float.NEGATIVE_INFINITY + ",0)", "2"); + assertIntervalQuery(field, "(* ,0)", "3"); + assertIntervalQuery(field, "[" + Float.NEGATIVE_INFINITY + ",0)", "3"); + assertIntervalQuery(field, "(0, " + Float.MIN_VALUE + ")", "0"); + assertIntervalQuery(field, "(0, " + Float.MIN_VALUE + "]", "1"); + assertIntervalQuery(field, "(0, " + Float.MAX_VALUE + ")", "1"); + assertIntervalQuery(field, "(0, " + Float.MAX_VALUE + "]", "2"); + assertIntervalQuery(field, "(0, " + Float.POSITIVE_INFINITY + ")", "2"); + assertIntervalQuery(field, "(0, " + Float.POSITIVE_INFINITY + "]", "3"); + + if (testDouble) { + clearIndex(); + assertU(adoc("id", "16", field, "-1.3")); + assertU(adoc("id", "17", field, "0.0")); + assertU(adoc("id", "18", field, "-0.0")); + assertU(adoc("id", "19", field, String.valueOf(Double.MIN_VALUE))); + assertU(adoc("id", "20", field, String.valueOf(Double.MAX_VALUE))); + assertU(adoc("id", "21", field, String.valueOf(Double.NEGATIVE_INFINITY))); + assertU(adoc("id", "22", field, String.valueOf(Double.POSITIVE_INFINITY))); + assertU(commit()); + + assertIntervalQuery(field, "[*,*]", "7"); + assertIntervalQuery(field, "(*,*)", "7"); + assertIntervalQuery(field, "(-1,1)", "3"); + assertIntervalQuery(field, "(-2,1)", "4"); + assertIntervalQuery(field, "(-1.3,0)", "1"); + assertIntervalQuery(field, "[-1.3,0)", "2"); + assertIntervalQuery(field, "[-1.3,0]", "3"); + assertIntervalQuery(field, "(" + Double.NEGATIVE_INFINITY + ",0)", "2"); + assertIntervalQuery(field, "(* ,0)", "3"); + assertIntervalQuery(field, "[" + Double.NEGATIVE_INFINITY + ",0)", "3"); + assertIntervalQuery(field, "(0, " + Double.MIN_VALUE + ")", "0"); + assertIntervalQuery(field, "(0, " + Double.MIN_VALUE + "]", "1"); + assertIntervalQuery(field, "(0, " + Double.MAX_VALUE + ")", "1"); + assertIntervalQuery(field, "(0, " + Double.MAX_VALUE + "]", "2"); + assertIntervalQuery(field, "(0, " + Double.POSITIVE_INFINITY + ")", "2"); + assertIntervalQuery(field, "(0, " + Double.POSITIVE_INFINITY + "]", "3"); + } + } @Test public void testDoubleFields() { - doTestFloat("test_d_dv"); + doTestFloat("test_d_dv", true); } @Test diff --git a/solr/core/src/test/org/apache/solr/schema/TestPointFields.java b/solr/core/src/test/org/apache/solr/schema/TestPointFields.java index 4c8ff69b09d..d8e90394a51 100644 --- a/solr/core/src/test/org/apache/solr/schema/TestPointFields.java +++ b/solr/core/src/test/org/apache/solr/schema/TestPointFields.java @@ -904,7 +904,6 @@ public class TestPointFields extends SolrTestCaseJ4 { } @Test - @AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-11043") public void testFloatPointFieldRangeFacet() throws Exception { String docValuesField = "number_p_f_dv"; String nonDocValuesField = "number_p_f";