Fix logic right shift for negative numbers (#10575)

Summary:
`>>` operator may perform an arithmetic shift right for signed integers, depending on the compiler, which gives wrong result when the input is negative. To ensure a logical shift, we cast it to uint64_t.

Pull Request resolved: #10575

Reviewed By: kgpai

Differential Revision: D60261588

Pulled By: zacw7

fbshipit-source-id: a86cb5b01fb4a8daf696eca5c6f0edbff273e019

velox/functions/prestosql/Bitwise.h

@@ -167,24 +167,33 @@ struct BitwiseRightShiftArithmeticFunction {
 
 template <typename T>
 struct BitwiseLogicalShiftRightFunction {
-  FOLLY_ALWAYS_INLINE bool
+  FOLLY_ALWAYS_INLINE void
 #if defined(__clang__)
       __attribute__((no_sanitize("integer")))
 #endif
       call(int64_t& result, int64_t number, int64_t shift, int64_t bits) {
     // Presto defines this only for bigint, thus we will define this only for
     // int64_t.
     if (bits == 64) {
+      if (number < 0) {
+        // >> operator may perform an arithmetic shift right for signed
+        // integers, depending on the compiler, which gives wrong result when
+        // the input is negative. To ensure a logical shift, we cast it to
+        // uint64_t.
+        uint64_t unsignedNumber = static_cast<uint64_t>(number) >> shift;
+        result = static_cast<int64_t>(unsignedNumber);
+        return;
+      }
       result = number >> shift;
-      return true;
+      return;
     }
 
     VELOX_USER_CHECK(
         !(bits <= 1 || bits > 64), "Bits must be between 2 and 64");
     VELOX_USER_CHECK_GE(shift, 0, "Shift must be non-negative");
 
     result = (number & ((1LL << bits) - 1)) >> shift;
-    return true;
+    return;
   }
 };
 

velox/functions/prestosql/tests/BitwiseTest.cpp

@@ -369,9 +369,10 @@ TEST_F(BitwiseTest, logicalShiftRight) {
   EXPECT_EQ(evalFunc(-1, 32, 32), 0);
   EXPECT_EQ(evalFunc(-1, 30, 32), 3);
   EXPECT_EQ(evalFunc(kMin64, 10, 32), 0);
-  EXPECT_EQ(evalFunc(kMin64, kMax64, 64), -1);
+  EXPECT_EQ(evalFunc(kMin64, kMax64, 64), 1);
   EXPECT_EQ(evalFunc(kMax64, kMin64, 64), kMax64);
   EXPECT_EQ(evalFunc(7, 0, 64), 7);
+  EXPECT_EQ(evalFunc(-8, 2, 64), 4611686018427387902);
 
   VELOX_ASSERT_THROW(evalFunc(3, -1, 3), "Shift must be non-negative");
   VELOX_ASSERT_THROW(evalFunc(3, 1, 1), "Bits must be between 2 and 64");