Skip to content

Commit

Permalink
Fix range queries on runtime double field (#71915)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Christoph Büscher committed Apr 20, 2021
1 parent 37dd7d8 commit 3186837
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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":{}}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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:
Expand Down

0 comments on commit 3186837

Please sign in to comment.