Skip to content

Commit ef19bc7

Browse files
jpountzjpountz
committed
Make -0 compare less than +0 consistently. (#22173)
Our `float`/`double` fields generally assume that `-0` compares less than `+0`, except when bounds are exclusive: an exclusive lower bound on `-0` excludes `+0` and an exclusive upper bound on `+0` excludes `-0`. Closes #22167
1 parent 38fc4c0 commit ef19bc7

File tree

3 files changed

+101
-7
lines changed

3 files changed

+101
-7
lines changed

‎core/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java‎

Lines changed: 79 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,30 @@ Query termsQuery(String field, List<Object> values) {
201201
return HalfFloatPoint.newSetQuery(field, v);
202202
}
203203

204+
private float nextDown(float f) {
205+
// HalfFloatPoint.nextDown considers that -0 is the same as +0
206+
// while point ranges are consistent with Float.compare, so
207+
// they consider that -0 < +0, so we explicitly make sure
208+
// that nextDown(+0) returns -0
209+
if (Float.floatToIntBits(f) == Float.floatToIntBits(0f)) {
210+
return -0f;
211+
} else {
212+
return HalfFloatPoint.nextDown(f);
213+
}
214+
}
215+
216+
private float nextUp(float f) {
217+
// HalfFloatPoint.nextUp considers that -0 is the same as +0
218+
// while point ranges are consistent with Float.compare, so
219+
// they consider that -0 < +0, so we explicitly make sure
220+
// that nextUp(-0) returns +0
221+
if (Float.floatToIntBits(f) == Float.floatToIntBits(-0f)) {
222+
return +0f;
223+
} else {
224+
return HalfFloatPoint.nextUp(f);
225+
}
226+
}
227+
204228
@Override
205229
Query rangeQuery(String field, Object lowerTerm, Object upperTerm,
206230
boolean includeLower, boolean includeUpper) {
@@ -209,16 +233,16 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm,
209233
if (lowerTerm != null) {
210234
l = parse(lowerTerm);
211235
if (includeLower) {
212-
l = Math.nextDown(l);
236+
l = nextDown(l);
213237
}
214238
l = HalfFloatPoint.nextUp(l);
215239
}
216240
if (upperTerm != null) {
217241
u = parse(upperTerm);
218242
if (includeUpper) {
219-
u = Math.nextUp(u);
243+
u = nextUp(u);
220244
}
221-
u = HalfFloatPoint.nextDown(u);
245+
u = nextDown(u);
222246
}
223247
return HalfFloatPoint.newRangeQuery(field, l, u);
224248
}
@@ -291,6 +315,30 @@ Query termsQuery(String field, List<Object> values) {
291315
return FloatPoint.newSetQuery(field, v);
292316
}
293317

318+
private float nextDown(float f) {
319+
// Math.nextDown considers that -0 is the same as +0
320+
// while point ranges are consistent with Float.compare, so
321+
// they consider that -0 < +0, so we explicitly make sure
322+
// that nextDown(+0) returns -0
323+
if (Float.floatToIntBits(f) == Float.floatToIntBits(0f)) {
324+
return -0f;
325+
} else {
326+
return Math.nextDown(f);
327+
}
328+
}
329+
330+
private float nextUp(float f) {
331+
// Math.nextUp considers that -0 is the same as +0
332+
// while point ranges are consistent with Float.compare, so
333+
// they consider that -0 < +0, so we explicitly make sure
334+
// that nextUp(-0) returns +0
335+
if (Float.floatToIntBits(f) == Float.floatToIntBits(-0f)) {
336+
return +0f;
337+
} else {
338+
return Math.nextUp(f);
339+
}
340+
}
341+
294342
@Override
295343
Query rangeQuery(String field, Object lowerTerm, Object upperTerm,
296344
boolean includeLower, boolean includeUpper) {
@@ -299,13 +347,13 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm,
299347
if (lowerTerm != null) {
300348
l = parse(lowerTerm);
301349
if (includeLower == false) {
302-
l = Math.nextUp(l);
350+
l = nextUp(l);
303351
}
304352
}
305353
if (upperTerm != null) {
306354
u = parse(upperTerm);
307355
if (includeUpper == false) {
308-
u = Math.nextDown(u);
356+
u = nextDown(u);
309357
}
310358
}
311359
return FloatPoint.newRangeQuery(field, l, u);
@@ -379,6 +427,30 @@ Query termsQuery(String field, List<Object> values) {
379427
return DoublePoint.newSetQuery(field, v);
380428
}
381429

430+
private double nextDown(double d) {
431+
// Math.nextDown considers that -0 is the same as +0
432+
// while point ranges are consistent with Double.compare, so
433+
// they consider that -0 < +0, so we explicitly make sure
434+
// that nextDown(+0) returns -0
435+
if (Double.doubleToLongBits(d) == Double.doubleToLongBits(0d)) {
436+
return -0d;
437+
} else {
438+
return Math.nextDown(d);
439+
}
440+
}
441+
442+
private double nextUp(double d) {
443+
// Math.nextUp considers that -0 is the same as +0
444+
// while point ranges are consistent with Double.compare, so
445+
// they consider that -0 < +0, so we explicitly make sure
446+
// that nextUp(-0) returns +0
447+
if (Double.doubleToLongBits(d) == Double.doubleToLongBits(-0d)) {
448+
return +0d;
449+
} else {
450+
return Math.nextUp(d);
451+
}
452+
}
453+
382454
@Override
383455
Query rangeQuery(String field, Object lowerTerm, Object upperTerm,
384456
boolean includeLower, boolean includeUpper) {
@@ -387,13 +459,13 @@ Query rangeQuery(String field, Object lowerTerm, Object upperTerm,
387459
if (lowerTerm != null) {
388460
l = parse(lowerTerm);
389461
if (includeLower == false) {
390-
l = Math.nextUp(l);
462+
l = nextUp(l);
391463
}
392464
}
393465
if (upperTerm != null) {
394466
u = parse(upperTerm);
395467
if (includeUpper == false) {
396-
u = Math.nextDown(u);
468+
u = nextDown(u);
397469
}
398470
}
399471
return DoublePoint.newRangeQuery(field, l, u);

‎core/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,4 +149,20 @@ public void testHalfFloatRange() throws IOException {
149149
}
150150
IOUtils.close(reader, dir);
151151
}
152+
153+
public void testNegativeZero() {
154+
assertEquals(
155+
NumberType.DOUBLE.rangeQuery("field", null, -0d, true, true),
156+
NumberType.DOUBLE.rangeQuery("field", null, +0d, true, false));
157+
assertEquals(
158+
NumberType.FLOAT.rangeQuery("field", null, -0f, true, true),
159+
NumberType.FLOAT.rangeQuery("field", null, +0f, true, false));
160+
assertEquals(
161+
NumberType.HALF_FLOAT.rangeQuery("field", null, -0f, true, true),
162+
NumberType.HALF_FLOAT.rangeQuery("field", null, +0f, true, false));
163+
164+
assertFalse(NumberType.DOUBLE.termQuery("field", -0d).equals(NumberType.DOUBLE.termQuery("field", +0d)));
165+
assertFalse(NumberType.FLOAT.termQuery("field", -0f).equals(NumberType.FLOAT.termQuery("field", +0f)));
166+
assertFalse(NumberType.HALF_FLOAT.termQuery("field", -0f).equals(NumberType.HALF_FLOAT.termQuery("field", +0f)));
167+
}
152168
}

‎docs/reference/mapping/types/numeric.asciidoc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ PUT my_index
3939
--------------------------------------------------
4040
// CONSOLE
4141

42+
NOTE: The `double`, `float` and `half_float` types consider that `-0.0` and
43+
`+0.0` are different values. As a consequence, doing a `term` query on
44+
`-0.0` will not match `+0.0` and vice-versa. Same is true for range queries:
45+
if the upper bound is `-0.0` then `+0.0` will not match, and if the lower
46+
bound is `+0.0` then `-0.0` will not match.
47+
4248
==== Which type should I use?
4349

4450
As far as integer types (`byte`, `short`, `integer` and `long`) are concerned,

0 commit comments

Comments
 (0)