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 authored Apr 20, 2021
1 parent 29e2ea7 commit 5315ed6
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 8 deletions.
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 @@ -88,7 +90,7 @@ setup:
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,6 +100,7 @@ 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":
Expand All @@ -107,7 +110,7 @@ setup:
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,6 +120,7 @@ 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":
Expand All @@ -128,10 +132,10 @@ 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}

---
Expand All @@ -144,8 +148,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
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,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 @@ -67,6 +67,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

0 comments on commit 5315ed6

Please sign in to comment.