From 539b17c69353f6be5c54b6f7842915778b9c136e Mon Sep 17 00:00:00 2001
From: Michael Kolupaev <michael.kolupaev@clickhouse.com>
Date: Sat, 9 Nov 2024 00:00:42 +0000
Subject: [PATCH] Fix nulls/infinities

---
 src/Functions/negate.cpp | 48 +++++++++++++++++++++++++++-------------
 1 file changed, 33 insertions(+), 15 deletions(-)

diff --git a/src/Functions/negate.cpp b/src/Functions/negate.cpp
index e1f0390f9cb..91e64e98c6b 100644
--- a/src/Functions/negate.cpp
+++ b/src/Functions/negate.cpp
@@ -39,22 +39,40 @@ template <> struct FunctionUnaryArithmeticMonotonicity<NameNegate>
         ///  * then jumps up from -2^63 to 2^63-1, then
         ///  * monotonically decreases on [2^63+1, 2^64-1] (with overflow).
         /// Similarly for UInt128 and UInt256.
-        bool is_monotonic = true;
-        switch (left.getType())
-        {
-            case Field::Types::UInt64:
-                is_monotonic = (left.safeGet<UInt64>() > 1ul << 63) == (right.safeGet<UInt64>() > 1ul << 63);
-                break;
-            case Field::Types::UInt128:
-                is_monotonic = (left.safeGet<UInt128>() > UInt128(1) << 127) == (right.safeGet<UInt128>() > UInt128(1) << 127);
-                break;
-            case Field::Types::UInt256:
-                is_monotonic = (left.safeGet<UInt256>() > UInt256(1) << 255) == (right.safeGet<UInt256>() > UInt256(1) << 255);
-                break;
-            default:
-                break;
-        }
+        /// Note: we currently don't handle the corner case -UINT64_MIN == UINT64_MIN, and similar for floats and wide signed ints.
+        /// (This implementation seems overcomplicated and not very correct, maybe there's a better way to do it,
+        ///  maybe by using the actual IDataType instead of two field types.)
 
+        /// We don't know the data type, assume nonmonotonic.
+        if (left.isNull() && right.isNull())
+            return { .is_monotonic = false, .is_positive = false, .is_strict = true };
+
+        auto which_half_if_unsigned_or_infinity = [](const Field & f, int half_if_null, bool & is_unsigned) -> int
+        {
+            is_unsigned = true;
+            switch (f.getType())
+            {
+                case Field::Types::UInt64: return (f.safeGet<UInt64>() > 1ul << 63) ? +1 : -1;
+                case Field::Types::UInt128: return (f.safeGet<UInt128>() > UInt128(1) << 127) ? +1 : -1;
+                case Field::Types::UInt256: return (f.safeGet<UInt256>() > UInt256(1) << 255) ? +1 : -1;
+                default: break;
+            }
+            is_unsigned = false;
+            if (f.isPositiveInfinity())
+                return +1;
+            if (f.isNegativeInfinity())
+                return -1;
+            if (f.isNull())
+                return half_if_null;
+            return 0;
+        };
+        bool left_is_unsigned, right_is_unsigned;
+        int left_half = which_half_if_unsigned_or_infinity(left, -1, left_is_unsigned);
+        int right_half = which_half_if_unsigned_or_infinity(right, +1, right_is_unsigned);
+
+        bool is_monotonic = true;
+        if (left_is_unsigned || right_is_unsigned)
+            is_monotonic = left_half * right_half >= 0; /// either both values in the same half or unexpected signed-unsigned mix
         return { .is_monotonic = is_monotonic, .is_positive = false, .is_strict = true };
     }
 };