From ee27de460faba5239c521cb7ecb387aaa86731a4 Mon Sep 17 00:00:00 2001 From: Martin Traverso Date: Wed, 22 Nov 2023 09:45:52 -0800 Subject: [PATCH] Fix incorrect comparison for negative zero --- .../main/java/io/trino/spi/type/DoubleType.java | 14 ++++++++++++-- .../src/main/java/io/trino/spi/type/RealType.java | 13 +++++++++++-- .../io/trino/spi/predicate/TestTupleDomain.java | 15 +++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/core/trino-spi/src/main/java/io/trino/spi/type/DoubleType.java b/core/trino-spi/src/main/java/io/trino/spi/type/DoubleType.java index 4a9175875e70..1a6a3c37016c 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/type/DoubleType.java +++ b/core/trino-spi/src/main/java/io/trino/spi/type/DoubleType.java @@ -242,7 +242,7 @@ private static boolean distinctFromOperator(double left, @IsNull boolean leftNul @ScalarOperator(COMPARISON_UNORDERED_LAST) private static long comparisonUnorderedLastOperator(double left, double right) { - return Double.compare(left, right); + return compare(left, right); } @ScalarOperator(COMPARISON_UNORDERED_FIRST) @@ -258,7 +258,8 @@ private static long comparisonUnorderedFirstOperator(double left, double right) if (Double.isNaN(right)) { return 1; } - return Double.compare(left, right); + + return compare(left, right); } @ScalarOperator(LESS_THAN) @@ -272,4 +273,13 @@ private static boolean lessThanOrEqualOperator(double left, double right) { return left <= right; } + + private static int compare(double left, double right) + { + if (left == right) { // Double.compare considers 0.0 and -0.0 different from each other + return 0; + } + + return Double.compare(left, right); + } } diff --git a/core/trino-spi/src/main/java/io/trino/spi/type/RealType.java b/core/trino-spi/src/main/java/io/trino/spi/type/RealType.java index da26b556cefd..f7b6eea4f7cf 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/type/RealType.java +++ b/core/trino-spi/src/main/java/io/trino/spi/type/RealType.java @@ -183,7 +183,7 @@ private static boolean distinctFromOperator(long left, @IsNull boolean leftNull, @ScalarOperator(COMPARISON_UNORDERED_LAST) private static long comparisonUnorderedLastOperator(long left, long right) { - return Float.compare(intBitsToFloat((int) left), intBitsToFloat((int) right)); + return compare(intBitsToFloat((int) left), intBitsToFloat((int) right)); } @ScalarOperator(COMPARISON_UNORDERED_FIRST) @@ -201,7 +201,7 @@ private static long comparisonUnorderedFirstOperator(long leftBits, long rightBi if (Float.isNaN(right)) { return 1; } - return Float.compare(left, right); + return compare(left, right); } @ScalarOperator(LESS_THAN) @@ -215,4 +215,13 @@ private static boolean lessThanOrEqualOperator(long left, long right) { return intBitsToFloat((int) left) <= intBitsToFloat((int) right); } + + private static int compare(float left, float right) + { + if (left == right) { // Float.compare considers 0.0 and -0.0 different from each other + return 0; + } + + return Float.compare(left, right); + } } diff --git a/core/trino-spi/src/test/java/io/trino/spi/predicate/TestTupleDomain.java b/core/trino-spi/src/test/java/io/trino/spi/predicate/TestTupleDomain.java index 613607f853d2..16ab08700e75 100644 --- a/core/trino-spi/src/test/java/io/trino/spi/predicate/TestTupleDomain.java +++ b/core/trino-spi/src/test/java/io/trino/spi/predicate/TestTupleDomain.java @@ -462,6 +462,16 @@ public void testContains() ImmutableMap.of( A, Domain.singleValue(BIGINT, 0L), B, Domain.none(VARCHAR)))).isTrue(); + + assertThat(contains( + ImmutableMap.of(A, Domain.singleValue(DOUBLE, 0.0)), + ImmutableMap.of(A, Domain.singleValue(DOUBLE, -0.0)))) + .isTrue(); + + assertThat(contains( + ImmutableMap.of(A, Domain.singleValue(DOUBLE, -0.0)), + ImmutableMap.of(A, Domain.singleValue(DOUBLE, 0.0)))) + .isTrue(); } @Test @@ -575,6 +585,11 @@ public void testEquals() ImmutableMap.of( A, Domain.singleValue(BIGINT, 0L), C, Domain.singleValue(DOUBLE, 0.0)))).isFalse(); + + assertThat(equals( + ImmutableMap.of(A, Domain.singleValue(DOUBLE, 0.0)), + ImmutableMap.of(A, Domain.singleValue(DOUBLE, -0.0)))) + .isTrue(); } @Test