feat: implement shiftRight function push down (pingcap#5156)

close pingcap#5100
Lloyd-Pottiger · Jul 19, 2022 · 25c2f82 · 25c2f82
1 parent 48558ba
commit 25c2f82
Show file tree

Hide file tree

Showing 6 changed files with 335 additions and 3 deletions.
diff --git a/dbms/src/DataTypes/NumberTraits.h b/dbms/src/DataTypes/NumberTraits.h
@@ -277,6 +277,7 @@ struct ResultOfAbs<Decimal<T>>
 };
 
 /** For bitwise operations, an integer is obtained with number of bits is equal to the maximum of the arguments.
+ * todo: note that MySQL handles only unsigned 64-bit integer argument and result values. We should refine the code.
  */
 template <typename A, typename B>
 struct ResultOfBit

diff --git a/dbms/src/Flash/Coprocessor/DAGExpressionAnalyzerHelper.cpp b/dbms/src/Flash/Coprocessor/DAGExpressionAnalyzerHelper.cpp
@@ -450,6 +450,7 @@ DAGExpressionAnalyzerHelper::FunctionBuilderMap DAGExpressionAnalyzerHelper::fun
  {"bitOr", DAGExpressionAnalyzerHelper::buildBitwiseFunction},
  {"bitXor", DAGExpressionAnalyzerHelper::buildBitwiseFunction},
  {"bitNot", DAGExpressionAnalyzerHelper::buildBitwiseFunction},
+ {"bitShiftRight", DAGExpressionAnalyzerHelper::buildBitwiseFunction},
  {"leftUTF8", DAGExpressionAnalyzerHelper::buildLeftUTF8Function},
  {"date_add", DAGExpressionAnalyzerHelper::buildDateAddOrSubFunction<DateAdd>},
  {"date_sub", DAGExpressionAnalyzerHelper::buildDateAddOrSubFunction<DateSub>},

diff --git a/dbms/src/Flash/Coprocessor/DAGUtils.cpp b/dbms/src/Flash/Coprocessor/DAGUtils.cpp
@@ -332,7 +332,7 @@ const std::unordered_map<tipb::ScalarFuncSig, String> scalar_func_map({
  {tipb::ScalarFuncSig::DecimalIsFalseWithNull, "isFalseWithNull"},
 
  //{tipb::ScalarFuncSig::LeftShift, "cast"},
- //{tipb::ScalarFuncSig::RightShift, "cast"},
+ {tipb::ScalarFuncSig::RightShift, "bitShiftRight"},
 
  //{tipb::ScalarFuncSig::BitCount, "cast"},
  //{tipb::ScalarFuncSig::GetParamString, "cast"},

diff --git a/dbms/src/Functions/bitShiftRight.cpp b/dbms/src/Functions/bitShiftRight.cpp
@@ -13,6 +13,9 @@
 // limitations under the License.
 
 #include <Functions/FunctionBinaryArithmetic.h>
+#include <common/types.h>
+
+#include <limits>
 
 namespace DB
 {
@@ -29,7 +32,18 @@ struct BitShiftRightImpl<A, B, false>
  template <typename Result = ResultType>
  static Result apply(A a, B b)
  {
- return static_cast<Result>(a) >> static_cast<Result>(b);
+ // It is an undefined behavior for shift operation in c++ that the right operand is negative or greater than
+ // or equal to the number of digits of the bits in the (promoted) left operand.
+ // See https://en.cppreference.com/w/cpp/language/operator_arithmetic for details.
+ if (static_cast<Result>(b) >= std::numeric_limits<decltype(static_cast<Result>(a))>::digits)
+ {
+ return static_cast<Result>(0);
+ }
+ // Note that we do not consider the case that the right operand is negative,
+ // since other types will all be cast to uint64 before shift operation
+ // according to DAGExpressionAnalyzerHelper::buildBitwiseFunction.
+ // Therefore, we simply suppress clang-tidy checking here.
+ return static_cast<Result>(a) >> static_cast<Result>(b); // NOLINT(clang-analyzer-core.UndefinedBinaryOperatorResult)
  }
  template <typename Result = ResultType>
  static Result apply(A, B, UInt8 &)
@@ -87,4 +101,4 @@ void registerFunctionBitShiftRight(FunctionFactory & factory)
  factory.registerFunction<FunctionBitShiftRight>();
 }
 
-} // namespace DB
+} // namespace DB