From d95558e401d699aa97e9f3ad27f4fce5022f9485 Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 1 Nov 2017 09:32:32 +0100 Subject: [PATCH 1/2] #27189 Fixed rounding of bounds in scaled float comparison --- .../index/mapper/ScaledFloatFieldMapper.java | 8 ++++---- .../index/mapper/ScaledFloatFieldTypeTests.java | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java index 0ff3acdea05fa..96ec29e2aa695 100644 --- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java +++ b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java @@ -256,19 +256,19 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower failIfNotIndexed(); Long lo = null; if (lowerTerm != null) { - double dValue = parse(lowerTerm); + double dValue = parse(lowerTerm) * scalingFactor; if (includeLower == false) { dValue = Math.nextUp(dValue); } - lo = Math.round(Math.ceil(dValue * scalingFactor)); + lo = Math.round(Math.ceil(dValue)); } Long hi = null; if (upperTerm != null) { - double dValue = parse(upperTerm); + double dValue = parse(upperTerm) * scalingFactor; if (includeUpper == false) { dValue = Math.nextDown(dValue); } - hi = Math.round(Math.floor(dValue * scalingFactor)); + hi = Math.round(Math.floor(dValue)); } Query query = NumberFieldMapper.NumberType.LONG.rangeQuery(name(), lo, hi, true, true, hasDocValues()); if (boost() != 1f) { diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldTypeTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldTypeTests.java index 811bac82bbce9..29818407cafb7 100644 --- a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldTypeTests.java +++ b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldTypeTests.java @@ -124,6 +124,22 @@ public void testRangeQuery() throws IOException { IOUtils.close(reader, dir); } + public void testRoundsUpperBoundCorrectly() { + ScaledFloatFieldMapper.ScaledFloatFieldType ft = new ScaledFloatFieldMapper.ScaledFloatFieldType(); + ft.setName("scaled_float"); + ft.setScalingFactor(100.0); + Query scaledFloatQ = ft.rangeQuery(null, 0.1, true, false, null); + assertEquals("scaled_float:[-9223372036854775808 TO 9]", scaledFloatQ.toString()); + } + + public void testRoundsLowerBoundCorrectly() { + ScaledFloatFieldMapper.ScaledFloatFieldType ft = new ScaledFloatFieldMapper.ScaledFloatFieldType(); + ft.setName("scaled_float"); + ft.setScalingFactor(100.0); + Query scaledFloatQ = ft.rangeQuery(-0.1, null, false, true, null); + assertEquals("scaled_float:[-9 TO 9223372036854775807]", scaledFloatQ.toString()); + } + public void testValueForSearch() { ScaledFloatFieldMapper.ScaledFloatFieldType ft = new ScaledFloatFieldMapper.ScaledFloatFieldType(); ft.setName("scaled_float"); From a20f366c76ac414def43b2a07524781128ccf257 Mon Sep 17 00:00:00 2001 From: Armin Date: Fri, 3 Nov 2017 15:59:35 +0100 Subject: [PATCH 2/2] #27189 more assertions from CR --- .../mapper/ScaledFloatFieldTypeTests.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldTypeTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldTypeTests.java index 29818407cafb7..83039ebd88319 100644 --- a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldTypeTests.java +++ b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldTypeTests.java @@ -130,6 +130,16 @@ public void testRoundsUpperBoundCorrectly() { ft.setScalingFactor(100.0); Query scaledFloatQ = ft.rangeQuery(null, 0.1, true, false, null); assertEquals("scaled_float:[-9223372036854775808 TO 9]", scaledFloatQ.toString()); + scaledFloatQ = ft.rangeQuery(null, 0.1, true, true, null); + assertEquals("scaled_float:[-9223372036854775808 TO 10]", scaledFloatQ.toString()); + scaledFloatQ = ft.rangeQuery(null, 0.095, true, false, null); + assertEquals("scaled_float:[-9223372036854775808 TO 9]", scaledFloatQ.toString()); + scaledFloatQ = ft.rangeQuery(null, 0.095, true, true, null); + assertEquals("scaled_float:[-9223372036854775808 TO 9]", scaledFloatQ.toString()); + scaledFloatQ = ft.rangeQuery(null, 0.105, true, false, null); + assertEquals("scaled_float:[-9223372036854775808 TO 10]", scaledFloatQ.toString()); + scaledFloatQ = ft.rangeQuery(null, 0.105, true, true, null); + assertEquals("scaled_float:[-9223372036854775808 TO 10]", scaledFloatQ.toString()); } public void testRoundsLowerBoundCorrectly() { @@ -138,6 +148,16 @@ public void testRoundsLowerBoundCorrectly() { ft.setScalingFactor(100.0); Query scaledFloatQ = ft.rangeQuery(-0.1, null, false, true, null); assertEquals("scaled_float:[-9 TO 9223372036854775807]", scaledFloatQ.toString()); + scaledFloatQ = ft.rangeQuery(-0.1, null, true, true, null); + assertEquals("scaled_float:[-10 TO 9223372036854775807]", scaledFloatQ.toString()); + scaledFloatQ = ft.rangeQuery(-0.095, null, false, true, null); + assertEquals("scaled_float:[-9 TO 9223372036854775807]", scaledFloatQ.toString()); + scaledFloatQ = ft.rangeQuery(-0.095, null, true, true, null); + assertEquals("scaled_float:[-9 TO 9223372036854775807]", scaledFloatQ.toString()); + scaledFloatQ = ft.rangeQuery(-0.105, null, false, true, null); + assertEquals("scaled_float:[-10 TO 9223372036854775807]", scaledFloatQ.toString()); + scaledFloatQ = ft.rangeQuery(-0.105, null, true, true, null); + assertEquals("scaled_float:[-10 TO 9223372036854775807]", scaledFloatQ.toString()); } public void testValueForSearch() {