From 3186837139b9c6b6d23c3200870651f10d3343b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20B=C3=BCscher?= Date: Tue, 20 Apr 2021 16:44:45 +0200 Subject: [PATCH] Fix range queries on runtime double field (#71915) DoubleScriptFieldRangeQuery which is used on runtime fields of type "double" currently uses simple double type comparison for checking its upper and lower bounds. Unfortunately it seems that -0.0 == 0.0, but when we want to exclude a 0.0 bound via "lt" the generated range query uses -0.0 as its upper bound which erroneously includes the 0.0 value. We can use `Double.compare` instead which seems to handle this edge case well. Closes #71786 --- .../query/DoubleScriptFieldRangeQuery.java | 2 +- .../DoubleScriptFieldRangeQueryTests.java | 17 +++++++ .../test/runtime_fields/30_double.yml | 51 ++++++++++++++++--- 3 files changed, 62 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldRangeQuery.java b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldRangeQuery.java index b90a7a2424f19..2d9b8789b5d10 100644 --- a/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldRangeQuery.java +++ b/x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldRangeQuery.java @@ -32,7 +32,7 @@ public DoubleScriptFieldRangeQuery( @Override protected boolean matches(double[] values, int count) { for (int i = 0; i < count; i++) { - if (lowerValue <= values[i] && values[i] <= upperValue) { + if (Double.compare(lowerValue, values[i]) <= 0 && Double.compare(values[i], upperValue) <= 0) { return true; } } diff --git a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldRangeQueryTests.java b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldRangeQueryTests.java index 5143766f58bcd..84713c3cbb4ca 100644 --- a/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldRangeQueryTests.java +++ b/x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/DoubleScriptFieldRangeQueryTests.java @@ -66,6 +66,23 @@ public void testMatches() { assertTrue(query.matches(new double[] { 2, 5 }, 2)); assertTrue(query.matches(new double[] { 5, 2 }, 2)); assertFalse(query.matches(new double[] { 5, 2 }, 1)); + + // test some special cases around 0.0 + query = new DoubleScriptFieldRangeQuery(randomScript(), leafFactory, "test", Double.NEGATIVE_INFINITY, -0.0); + assertTrue(query.matches(new double[] { -0.0 }, 1)); + assertFalse(query.matches(new double[] { 0.0 }, 1)); + + query = new DoubleScriptFieldRangeQuery(randomScript(), leafFactory, "test", Double.NEGATIVE_INFINITY, 0.0); + assertTrue(query.matches(new double[] { -0.0 }, 1)); + assertTrue(query.matches(new double[] { 0.0 }, 1)); + + query = new DoubleScriptFieldRangeQuery(randomScript(), leafFactory, "test", -0.0, Double.POSITIVE_INFINITY); + assertTrue(query.matches(new double[] { -0.0 }, 1)); + assertTrue(query.matches(new double[] { 0.0 }, 1)); + + query = new DoubleScriptFieldRangeQuery(randomScript(), leafFactory, "test", 0.0, Double.POSITIVE_INFINITY); + assertFalse(query.matches(new double[] { -0.0 }, 1)); + assertTrue(query.matches(new double[] { 0.0 }, 1)); } @Override diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/runtime_fields/30_double.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/runtime_fields/30_double.yml index 67ae10100fd89..9a3abab2af6d6 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/runtime_fields/30_double.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/runtime_fields/30_double.yml @@ -53,6 +53,8 @@ setup: index: sensor refresh: true body: | + {"index":{}} + {"timestamp": 1516897304000, "temperature": 202, "voltage": 0.0, "node": "c"} {"index":{}} {"timestamp": 1516729294000, "temperature": 200, "voltage": 5.2, "node": "a"} {"index":{}} @@ -82,13 +84,16 @@ setup: --- "fetch fields": + - skip: + version: " - 7.12.0" + reason: bugfix for 0.0 value was added in 7.12.1 - do: search: index: sensor body: sort: timestamp fields: [voltage_percent, voltage_percent_from_source, voltage_sqrts] - - match: {hits.total.value: 6} + - match: {hits.total.value: 7} - match: {hits.hits.0.fields.voltage_percent: [0.6896551724137931] } - match: {hits.hits.0.fields.voltage_percent_from_source: [0.6896551724137931] } # Scripts that scripts that emit multiple values are supported and their results are sorted @@ -98,16 +103,20 @@ setup: - match: {hits.hits.3.fields.voltage_percent: [0.8793103448275862] } - match: {hits.hits.4.fields.voltage_percent: [1.0] } - match: {hits.hits.5.fields.voltage_percent: [0.896551724137931] } + - match: {hits.hits.6.fields.voltage_percent: [0.0] } --- "docvalue_fields": + - skip: + version: " - 7.12.0" + reason: bugfix for 0.0 value was added in 7.12.1 - do: search: index: sensor body: sort: timestamp docvalue_fields: [voltage_percent, voltage_percent_from_source, voltage_sqrts] - - match: {hits.total.value: 6} + - match: {hits.total.value: 7} - match: {hits.hits.0.fields.voltage_percent: [0.6896551724137931] } - match: {hits.hits.0.fields.voltage_percent_from_source: [0.6896551724137931] } # Scripts that scripts that emit multiple values are supported and their results are sorted @@ -117,9 +126,13 @@ setup: - match: {hits.hits.3.fields.voltage_percent: [0.8793103448275862] } - match: {hits.hits.4.fields.voltage_percent: [1.0] } - match: {hits.hits.5.fields.voltage_percent: [0.896551724137931] } + - match: {hits.hits.6.fields.voltage_percent: [0.0] } --- "terms agg": + - skip: + version: " - 7.12.0" + reason: bugfix for 0.0 value was added in 7.12.1 - do: search: index: sensor @@ -128,14 +141,17 @@ setup: v10: terms: field: voltage_percent - - match: {hits.total.value: 6} - - match: {aggregations.v10.buckets.0.key: 0.6896551724137931} + - match: {hits.total.value: 7} + - match: {aggregations.v10.buckets.0.key: 0.0} - match: {aggregations.v10.buckets.0.doc_count: 1} - - match: {aggregations.v10.buckets.1.key: 0.7241379310344828} + - match: {aggregations.v10.buckets.1.key: 0.6896551724137931} - match: {aggregations.v10.buckets.1.doc_count: 1} --- "range query": + - skip: + version: " - 7.12.0" + reason: bugfix for 0.0 value was added in 7.12.1 - do: search: index: sensor @@ -144,8 +160,29 @@ setup: range: voltage_percent: lt: .7 - - match: {hits.total.value: 1} - - match: {hits.hits.0._source.voltage: 4.0} + - match: {hits.total.value: 2} + - match: {hits.hits.0._source.voltage: 0.0} + - match: {hits.hits.1._source.voltage: 4.0} + + - do: + search: + index: sensor + body: + query: + range: + voltage_percent: + lt: 0.0 + - match: {hits.total.value: 0} + + - do: + search: + index: sensor + body: + query: + range: + voltage_percent: + gt: 0.0 + - match: {hits.total.value: 6} - do: search: