mapleFU commented on code in PR #41012:
URL: https://github.com/apache/arrow/pull/41012#discussion_r1604554818


##########
cpp/src/arrow/compute/kernels/scalar_compare.cc:
##########
@@ -385,6 +385,55 @@ struct VarArgsCompareFunction : ScalarFunction {
   }
 };
 
+class DecimalTypesCompareMatcher : public TypeMatcher {
+ public:
+  explicit DecimalTypesCompareMatcher(std::shared_ptr<TypeMatcher> 
decimal_type_matcher)
+      : decimal_type_matcher(std::move(decimal_type_matcher)) {}
+
+  bool Matches(const DataType& type) const override {
+    return decimal_type_matcher->Matches(type);
+  }
+
+  bool Matches(const std::vector<TypeHolder>& types) const override {
+    DCHECK_EQ(types.size(), 2);
+    if (!is_decimal(*types[0]) || !is_decimal(*types[1])) {
+      return true;
+    }
+
+    // Below match logic should only be executed when types are both decimal
+    //
+    const auto& left_type = checked_cast<const DecimalType&>(*types[0]);
+    const auto& right_type = checked_cast<const DecimalType&>(*types[1]);
+    assert(is_decimal(left_type) && is_decimal(right_type));
+
+    // check the decimal types' scales according kAdd promotion rule
+    const int32_t s1 = left_type.scale();
+    const int32_t s2 = right_type.scale();

Review Comment:
   So same scale with different precision can be matched here?



##########
cpp/src/arrow/compute/kernel.cc:
##########
@@ -505,9 +510,14 @@ bool KernelSignature::Equals(const KernelSignature& other) 
const {
 }
 
 bool KernelSignature::MatchesInputs(const std::vector<TypeHolder>& types) 
const {
+  auto is_match_combination_types = [&](const InputType& in_type) {
+    return in_type.kind() == InputType::USE_TYPE_MATCHER ? 
in_type.Matches(types) : true;
+  };
+
   if (is_varargs_) {
     for (size_t i = 0; i < types.size(); ++i) {
-      if (!in_types_[std::min(i, in_types_.size() - 1)].Matches(*types[i])) {
+      auto in_type = in_types_[std::min(i, in_types_.size() - 1)];

Review Comment:
   ```suggestion
         const auto& in_type = in_types_[std::min(i, in_types_.size() - 1)];
   ```



##########
cpp/src/arrow/compute/kernels/scalar_compare.cc:
##########
@@ -385,6 +385,55 @@ struct VarArgsCompareFunction : ScalarFunction {
   }
 };
 
+class DecimalTypesCompareMatcher : public TypeMatcher {
+ public:
+  explicit DecimalTypesCompareMatcher(std::shared_ptr<TypeMatcher> 
decimal_type_matcher)
+      : decimal_type_matcher(std::move(decimal_type_matcher)) {}
+
+  bool Matches(const DataType& type) const override {
+    return decimal_type_matcher->Matches(type);
+  }
+
+  bool Matches(const std::vector<TypeHolder>& types) const override {
+    DCHECK_EQ(types.size(), 2);
+    if (!is_decimal(*types[0]) || !is_decimal(*types[1])) {
+      return true;
+    }
+
+    // Below match logic should only be executed when types are both decimal
+    //
+    const auto& left_type = checked_cast<const DecimalType&>(*types[0]);
+    const auto& right_type = checked_cast<const DecimalType&>(*types[1]);
+    assert(is_decimal(left_type) && is_decimal(right_type));

Review Comment:
   Is this duplicate?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to