Skip to content

Commit c0a5d0a

Browse files
xternxtern
authored
IGNITE-26613 Sql. Use compare() in binary comparisons for floating point types (#7948)
1 parent f91e121 commit c0a5d0a

6 files changed

Lines changed: 408 additions & 35 deletions

File tree

‎modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItFloatingPointTest.java‎

Lines changed: 98 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,14 @@
1717

1818
package org.apache.ignite.internal.sql.engine;
1919

20-
import static java.util.stream.Collectors.toList;
2120
import static org.apache.ignite.internal.TestWrappers.unwrapIgniteImpl;
2221
import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsIndexScan;
2322
import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsIndexScanIgnoreBounds;
2423
import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsTableScan;
25-
import static org.hamcrest.MatcherAssert.assertThat;
2624

2725
import java.util.List;
2826
import org.apache.ignite.Ignite;
2927
import org.apache.ignite.internal.app.IgniteImpl;
30-
import org.hamcrest.Matchers;
3128
import org.junit.jupiter.api.BeforeAll;
3229
import org.junit.jupiter.api.BeforeEach;
3330
import org.junit.jupiter.api.Test;
@@ -205,15 +202,15 @@ void testEqualityComparison() {
205202
assertQuery("SELECT fn FROM test WHERE fn = '-Infinity'::FLOAT").returns(Float.NEGATIVE_INFINITY).check();
206203
assertQuery("SELECT f FROM test WHERE f = '+Infinity'::FLOAT").returns(Float.POSITIVE_INFINITY).check();
207204
assertQuery("SELECT fn FROM test WHERE fn = '+Infinity'::FLOAT").returns(Float.POSITIVE_INFINITY).check();
208-
assertQuery("SELECT f FROM test WHERE f = 'NaN'::FLOAT").returnNothing().check(); // NaN never equals
209-
assertQuery("SELECT fn FROM test WHERE fn = 'NaN'::FLOAT").returnNothing().check(); // NaN never equals
205+
assertQuery("SELECT f FROM test WHERE f = 'NaN'::FLOAT").returns(Float.NaN).check();
206+
assertQuery("SELECT fn FROM test WHERE fn = 'NaN'::FLOAT").returns(Float.NaN).check();
210207

211208
assertQuery("SELECT d FROM test WHERE d = '-Infinity'::DOUBLE").returns(Double.NEGATIVE_INFINITY).check();
212209
assertQuery("SELECT dn FROM test WHERE dn = '-Infinity'::DOUBLE").returns(Double.NEGATIVE_INFINITY).check();
213210
assertQuery("SELECT d FROM test WHERE d = '+Infinity'::DOUBLE").returns(Double.POSITIVE_INFINITY).check();
214211
assertQuery("SELECT dn FROM test WHERE dn = '+Infinity'::DOUBLE").returns(Double.POSITIVE_INFINITY).check();
215-
assertQuery("SELECT d FROM test WHERE d = 'NaN'::DOUBLE").returnNothing().check(); // NaN never equals
216-
assertQuery("SELECT dn FROM test WHERE dn = 'NaN'::DOUBLE").returnNothing().check(); // NaN never equals
212+
assertQuery("SELECT d FROM test WHERE d = 'NaN'::DOUBLE").returns(Double.NaN).check();
213+
assertQuery("SELECT dn FROM test WHERE dn = 'NaN'::DOUBLE").returns(Double.NaN).check();
217214
}
218215

219216
@Test
@@ -228,6 +225,7 @@ void testNonEqualityComparisonWithIndexScan() {
228225
.returns(-1.0f)
229226
.returns(1.0f)
230227
.returns(Float.POSITIVE_INFINITY)
228+
.returns(Float.NaN)
231229
.check();
232230
assertQuery("SELECT /*+ FORCE_INDEX(test_fn_idx) */ fn FROM test WHERE fn > ?")
233231
.withParam(Float.NEGATIVE_INFINITY)
@@ -238,6 +236,7 @@ void testNonEqualityComparisonWithIndexScan() {
238236
.returns(-1.0f)
239237
.returns(1.0f)
240238
.returns(Float.POSITIVE_INFINITY)
239+
.returns(Float.NaN)
241240
.check();
242241
assertQuery("SELECT /*+ FORCE_INDEX(test_f_idx) */ f FROM test WHERE f > ?")
243242
.withParam(Float.NaN)
@@ -255,6 +254,7 @@ void testNonEqualityComparisonWithIndexScan() {
255254
.returns(-0.0d).returns(0.0d)
256255
.returns(-1.0d).returns(1.0d)
257256
.returns(Double.POSITIVE_INFINITY)
257+
.returns(Double.NaN)
258258
.check();
259259
assertQuery("SELECT /*+ FORCE_INDEX(test_dn_idx) */ dn FROM test WHERE dn > '-Infinity'::DOUBLE")
260260
.matches(containsIndexScan("PUBLIC", "TEST", "TEST_DN_IDX"))
@@ -264,6 +264,7 @@ void testNonEqualityComparisonWithIndexScan() {
264264
.returns(-1.0d)
265265
.returns(1.0d)
266266
.returns(Double.POSITIVE_INFINITY)
267+
.returns(Double.NaN)
267268
.check();
268269
assertQuery("SELECT /*+ FORCE_INDEX(test_d_idx) */ d FROM test WHERE d > 'NaN'::DOUBLE")
269270
.matches(containsIndexScan("PUBLIC", "TEST", "TEST_D_IDX"))
@@ -280,43 +281,69 @@ void testNonEqualityComparisonWithIndexScan() {
280281
assertQuery("SELECT /*+ FORCE_INDEX(test_f_idx) */ f FROM test WHERE f < ?")
281282
.withParam(+0.0f)
282283
.matches(containsIndexScan("PUBLIC", "TEST", "TEST_F_IDX"))
284+
.returns(-0.0f)
283285
.returns(-1.0f)
284286
.returns(Float.NEGATIVE_INFINITY)
285287
.check();
286288
assertQuery("SELECT /*+ FORCE_INDEX(test_fn_idx) */ fn FROM test WHERE fn < ?")
287289
.withParam(+0.0f)
288290
.matches(containsIndexScan("PUBLIC", "TEST", "TEST_FN_IDX"))
291+
.returns(-0.0f)
289292
.returns(-1.0f)
290293
.returns(Float.NEGATIVE_INFINITY)
291294
.check();
292295
assertQuery("SELECT /*+ FORCE_INDEX(test_f_idx) */ f FROM test WHERE f < ?")
293296
.withParam(Float.NaN)
294297
.matches(containsIndexScan("PUBLIC", "TEST", "TEST_F_IDX"))
295-
.returnNothing()
298+
.returns(Float.NEGATIVE_INFINITY)
299+
.returns(-0.0f)
300+
.returns(0.0f)
301+
.returns(-1.0f)
302+
.returns(1.0f)
303+
.returns(Float.POSITIVE_INFINITY)
296304
.check();
297305
assertQuery("SELECT /*+ FORCE_INDEX(test_fn_idx) */ fn FROM test WHERE fn < ?")
298306
.withParam(Float.NaN)
299307
.matches(containsIndexScan("PUBLIC", "TEST", "TEST_FN_IDX"))
300-
.returnNothing()
308+
.returns(Float.NEGATIVE_INFINITY)
309+
.returns(-0.0f)
310+
.returns(0.0f)
311+
.returns(0.0f)
312+
.returns(-1.0f)
313+
.returns(1.0f)
314+
.returns(Float.POSITIVE_INFINITY)
301315
.check();
302316

303317
assertQuery("SELECT /*+ FORCE_INDEX(test_d_idx) */ d FROM test WHERE d < +0.0::DOUBLE")
304318
.matches(containsIndexScan("PUBLIC", "TEST", "TEST_D_IDX"))
319+
.returns(-0.0d)
305320
.returns(-1.0d)
306321
.returns(Double.NEGATIVE_INFINITY)
307322
.check();
308323
assertQuery("SELECT /*+ FORCE_INDEX(test_dn_idx) */ dn FROM test WHERE dn < +0.0::DOUBLE")
309324
.matches(containsIndexScan("PUBLIC", "TEST", "TEST_DN_IDX"))
325+
.returns(-0.0d)
310326
.returns(-1.0d)
311327
.returns(Double.NEGATIVE_INFINITY)
312328
.check();
313329
assertQuery("SELECT /*+ FORCE_INDEX(test_d_idx) */ d FROM test WHERE d < '-NaN'::DOUBLE")
314330
.matches(containsIndexScan("PUBLIC", "TEST", "TEST_D_IDX"))
315-
.returnNothing()
331+
.returns(Double.NEGATIVE_INFINITY)
332+
.returns(-0.0d)
333+
.returns(0.0d)
334+
.returns(-1.0d)
335+
.returns(1.0d)
336+
.returns(Double.POSITIVE_INFINITY)
316337
.check();
317338
assertQuery("SELECT /*+ FORCE_INDEX(test_dn_idx) */ dn FROM test WHERE dn < '-NaN'::DOUBLE")
318339
.matches(containsIndexScan("PUBLIC", "TEST", "TEST_DN_IDX"))
319-
.returnNothing()
340+
.returns(Double.NEGATIVE_INFINITY)
341+
.returns(-0.0d)
342+
.returns(0.0d)
343+
.returns(0.0d)
344+
.returns(-1.0d)
345+
.returns(1.0d)
346+
.returns(Double.POSITIVE_INFINITY)
320347
.check();
321348
}
322349
}
@@ -326,13 +353,18 @@ void testNonEqualityComparisonWithTableScan() {
326353
{ // Greater-than
327354
assertQuery("SELECT /*+ NO_INDEX */ f FROM test WHERE f > -0.0::FLOAT")
328355
.matches(containsTableScan("PUBLIC", "TEST"))
356+
.returns(0.0f)
329357
.returns(1.0f)
330358
.returns(Float.POSITIVE_INFINITY)
359+
.returns(Float.NaN)
331360
.check();
332-
assertQuery("SELECT /*+ NO_INDEX */ fn FROM test WHERE fn > -0.0::FLOAT")
361+
assertQuery("SELECT /*+ NO_INDEX */ id, fn FROM test WHERE fn > -0.0::FLOAT")
333362
.matches(containsTableScan("PUBLIC", "TEST"))
334-
.returns(1.0f)
335-
.returns(Float.POSITIVE_INFINITY)
363+
.returns(0, 0.0f)
364+
.returns(5, 0.0f)
365+
.returns(7, 1.0f)
366+
.returns(2, Float.POSITIVE_INFINITY)
367+
.returns(3, Float.NaN)
336368
.check();
337369
assertQuery("SELECT /*+ NO_INDEX */ f FROM test WHERE f > ?")
338370
.withParam(Float.NaN)
@@ -347,13 +379,18 @@ void testNonEqualityComparisonWithTableScan() {
347379

348380
assertQuery("SELECT /*+ NO_INDEX */ d FROM test WHERE d > -0.0::DOUBLE")
349381
.matches(containsTableScan("PUBLIC", "TEST"))
382+
.returns(0.0d)
350383
.returns(1.0d)
351384
.returns(Double.POSITIVE_INFINITY)
385+
.returns(Double.NaN)
352386
.check();
353-
assertQuery("SELECT /*+ NO_INDEX */ dn FROM test WHERE dn > -0.0::DOUBLE")
387+
assertQuery("SELECT /*+ NO_INDEX */ id, dn FROM test WHERE dn > -0.0::DOUBLE")
354388
.matches(containsTableScan("PUBLIC", "TEST"))
355-
.returns(1.0d)
356-
.returns(Double.POSITIVE_INFINITY)
389+
.returns(0, 0.0d)
390+
.returns(5, 0.0d)
391+
.returns(7, 1.0d)
392+
.returns(2, Double.POSITIVE_INFINITY)
393+
.returns(3, Double.NaN)
357394
.check();
358395
assertQuery("SELECT /*+ NO_INDEX */ d FROM test WHERE d > 'NaN'::DOUBLE")
359396
.matches(containsTableScan("PUBLIC", "TEST"))
@@ -368,42 +405,68 @@ void testNonEqualityComparisonWithTableScan() {
368405
{ // Lesser-than
369406
assertQuery("SELECT /*+ NO_INDEX */ f FROM test WHERE f < +0.0::FLOAT")
370407
.matches(containsTableScan("PUBLIC", "TEST"))
408+
.returns(-0.0f)
371409
.returns(-1.0f)
372410
.returns(Float.NEGATIVE_INFINITY)
373411
.check();
374412
assertQuery("SELECT /*+ NO_INDEX */ fn FROM test WHERE fn < +0.0::FLOAT")
375413
.matches(containsTableScan("PUBLIC", "TEST"))
414+
.returns(-0.0f)
376415
.returns(-1.0f)
377416
.returns(Float.NEGATIVE_INFINITY)
378417
.check();
379418
assertQuery("SELECT /*+ NO_INDEX */ f FROM test WHERE f < ?")
380419
.withParam(Float.NaN)
381420
.matches(containsTableScan("PUBLIC", "TEST"))
382-
.returnNothing()
421+
.returns(Float.NEGATIVE_INFINITY)
422+
.returns(-1.0f)
423+
.returns(-0.0f)
424+
.returns(0.0f)
425+
.returns(1.0f)
426+
.returns(Float.POSITIVE_INFINITY)
383427
.check();
384428
assertQuery("SELECT /*+ NO_INDEX */ fn FROM test WHERE fn < ?")
385429
.withParam(Float.NaN)
386430
.matches(containsTableScan("PUBLIC", "TEST"))
387-
.returnNothing()
431+
.returns(Float.NEGATIVE_INFINITY)
432+
.returns(-1.0f)
433+
.returns(-0.0f)
434+
.returns(0.0f)
435+
.returns(0.0f)
436+
.returns(1.0f)
437+
.returns(Float.POSITIVE_INFINITY)
388438
.check();
389439

390440
assertQuery("SELECT /*+ NO_INDEX */ d FROM test WHERE d < +0.0::DOUBLE")
391441
.matches(containsTableScan("PUBLIC", "TEST"))
442+
.returns(-0.0d)
392443
.returns(-1.0d)
393444
.returns(Double.NEGATIVE_INFINITY)
394445
.check();
395446
assertQuery("SELECT /*+ NO_INDEX */ dn FROM test WHERE dn < +0.0::DOUBLE")
396447
.matches(containsTableScan("PUBLIC", "TEST"))
448+
.returns(-0.0d)
397449
.returns(-1.0d)
398450
.returns(Double.NEGATIVE_INFINITY)
399451
.check();
400452
assertQuery("SELECT /*+ NO_INDEX */ d FROM test WHERE d < 'NaN'::DOUBLE")
401453
.matches(containsTableScan("PUBLIC", "TEST"))
402-
.returnNothing()
454+
.returns(Double.NEGATIVE_INFINITY)
455+
.returns(-1.0d)
456+
.returns(-0.0d)
457+
.returns(0.0d)
458+
.returns(1.0d)
459+
.returns(Double.POSITIVE_INFINITY)
403460
.check();
404461
assertQuery("SELECT /*+ NO_INDEX */ dn FROM test WHERE dn < 'NaN'::DOUBLE")
405462
.matches(containsTableScan("PUBLIC", "TEST"))
406-
.returnNothing()
463+
.returns(Double.NEGATIVE_INFINITY)
464+
.returns(-1.0d)
465+
.returns(-0.0d)
466+
.returns(0.0d)
467+
.returns(0.0d)
468+
.returns(1.0d)
469+
.returns(Double.POSITIVE_INFINITY)
407470
.check();
408471
}
409472
}
@@ -424,27 +487,32 @@ void testIsDistinctFrom() {
424487
.check();
425488

426489
assertQuery("SELECT f FROM test WHERE f IS DISTINCT FROM '-0.0'::FLOAT")
490+
.returns(0.0f) // 0.0f <> -0.0f
427491
.returns(1.0f).returns(-1.0f)
428492
.returns(Float.NEGATIVE_INFINITY).returns(Float.POSITIVE_INFINITY)
429493
.returns(Float.NaN)
430494
.returns((Object) null)
431495
.check();
432496
assertQuery("SELECT d FROM test WHERE d IS DISTINCT FROM '-0.0'::DOUBLE")
497+
.returns(0.0d) // 0.0d <> -0.0d
433498
.returns(1.0d).returns(-1.0d)
434499
.returns(Double.NEGATIVE_INFINITY).returns(Double.POSITIVE_INFINITY)
435500
.returns(Double.NaN)
436501
.returns((Object) null)
437502
.check();
438503

439-
List<Float> floats = sql("SELECT f FROM test WHERE f IS DISTINCT FROM 'NaN'::FLOAT")
440-
.stream().flatMap(List::stream).map(Float.class::cast).collect(toList());
441-
assertThat(floats, Matchers.hasSize(8));
442-
assertThat(floats, Matchers.hasItem(Float.NaN)); // NaN not equal to NaN
443-
444-
List<Double> doubles = sql("SELECT d FROM test WHERE d IS DISTINCT FROM 'NaN'::DOUBLE")
445-
.stream().flatMap(List::stream).map(Double.class::cast).collect(toList());
446-
assertThat(doubles, Matchers.hasSize(8));
447-
assertThat(doubles, Matchers.hasItem(Double.NaN)); // NaN not equal to NaN
504+
assertQuery("SELECT f FROM test WHERE f IS DISTINCT FROM 'NaN'::FLOAT")
505+
.returns(0.0f).returns(-0.0f)
506+
.returns(1.0f).returns(-1.0f)
507+
.returns(Float.NEGATIVE_INFINITY).returns(Float.POSITIVE_INFINITY)
508+
.returns((Object) null)
509+
.check();
510+
assertQuery("SELECT d FROM test WHERE d IS DISTINCT FROM 'NaN'::DOUBLE")
511+
.returns(0.0d).returns(-0.0d)
512+
.returns(1.0d).returns(-1.0d)
513+
.returns(Double.NEGATIVE_INFINITY).returns(Double.POSITIVE_INFINITY)
514+
.returns((Object) null)
515+
.check();
448516
}
449517

450518
@Test

‎modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteExpressions.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.apache.ignite.internal.sql.engine.exec.exp;
1919

2020
import java.lang.reflect.Type;
21+
import java.util.EnumSet;
2122
import org.apache.calcite.linq4j.tree.Expression;
2223
import org.apache.calcite.linq4j.tree.ExpressionType;
2324
import org.apache.calcite.linq4j.tree.Expressions;
@@ -29,6 +30,16 @@
2930

3031
/** Calcite liq4j expressions customized for Ignite. */
3132
public class IgniteExpressions {
33+
/** Comparison expression types that need special handling for floating-point types. */
34+
private static final EnumSet<ExpressionType> COMPARISON_OR_EQUALS_OPERATORS = EnumSet.of(
35+
ExpressionType.Equal,
36+
ExpressionType.NotEqual,
37+
ExpressionType.LessThan,
38+
ExpressionType.LessThanOrEqual,
39+
ExpressionType.GreaterThan,
40+
ExpressionType.GreaterThanOrEqual
41+
);
42+
3243
/** Make binary expression with arithmetic operations override. */
3344
public static Expression makeBinary(ExpressionType binaryType, Expression left, Expression right) {
3445
switch (binaryType) {
@@ -41,6 +52,14 @@ public static Expression makeBinary(ExpressionType binaryType, Expression left,
4152
case Divide:
4253
return divideExact(left, right);
4354
default:
55+
if (COMPARISON_OR_EQUALS_OPERATORS.contains(binaryType)) {
56+
Expression floatingPointCmp = compareFloatingPoint(binaryType, left, right);
57+
58+
if (floatingPointCmp != null) {
59+
return floatingPointCmp;
60+
}
61+
}
62+
4463
return Expressions.makeBinary(binaryType, left, right);
4564
}
4665
}
@@ -182,4 +201,25 @@ private static Type larger(Type type0, Type type1) {
182201
return Double.TYPE;
183202
}
184203
}
204+
205+
/**
206+
* Generates a comparison expression for floating-point types using {@link Float#compare(float, float)}
207+
* or {@link Double#compare(double, double)} instead of primitive comparison operators.
208+
*/
209+
private static @Nullable Expression compareFloatingPoint(ExpressionType binaryType, Expression left, Expression right) {
210+
Type leftType = left.getType();
211+
Type rightType = right.getType();
212+
213+
boolean isFloat = leftType == Float.TYPE || rightType == Float.TYPE;
214+
boolean isDouble = leftType == Double.TYPE || rightType == Double.TYPE;
215+
216+
if (!isFloat && !isDouble) {
217+
return null;
218+
}
219+
220+
Class<?> targetClass = isDouble ? Double.class : Float.class;
221+
Expression cmp = Expressions.call(targetClass, "compare", left, right);
222+
223+
return Expressions.makeBinary(binaryType, cmp, Expressions.constant(0));
224+
}
185225
}

‎modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/externalize/RelJson.java‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,19 @@ private Object toJson(RexNode node) {
420420
RexLiteral literal = (RexLiteral) node;
421421
Object value = literal.getValue3();
422422
map = map();
423-
map.put("literal", toJson(value));
423+
// Convert the approximate numeric negative zero (-0.0) value to a string to preserve the minus sign,
424+
// otherwise due to USE_BIG_DECIMAL_FOR_FLOATS jackson will create a BigDecimal from a double (-0.0)
425+
// and this will result in the loss of the minus sign.
426+
// Other special values +Infinity/-Infinity/NaN are serialized by jackson as strings.
427+
// The value in RexLiteral for approximate types is always of type Double (see RexLiteral#valueMatchesType).
428+
if (value instanceof Double
429+
&& isApproximateNumeric(node.getType())
430+
&& isNegativeZero((Double) value)) {
431+
map.put("literal", "-0.0");
432+
} else {
433+
map.put("literal", toJson(value));
434+
}
435+
424436
map.put("type", toJson(node.getType()));
425437

426438
return map;
@@ -1062,4 +1074,8 @@ private List<RexNode> toRexList(RelInput relInput, List<?> operands) {
10621074
}
10631075
return list;
10641076
}
1077+
1078+
private static boolean isNegativeZero(double d) {
1079+
return d == 0.0 && Double.doubleToRawLongBits(d) != 0;
1080+
}
10651081
}

0 commit comments

Comments
 (0)