lidavidm commented on a change in pull request #10538:
URL: https://github.com/apache/arrow/pull/10538#discussion_r668693928



##########
File path: cpp/src/arrow/compute/kernels/scalar_if_else.cc
##########
@@ -770,6 +770,144 @@ struct IfElseFunctor<Type, enable_if_base_binary<Type>> {
   }
 };
 
+template <typename Type>
+struct IfElseFunctor<Type, enable_if_fixed_size_binary<Type>> {
+  // A - Array, S - Scalar, X = Array/Scalar
+
+  // SXX
+  static Status Call(KernelContext* ctx, const BooleanScalar& cond, const 
Datum& left,
+                     const Datum& right, Datum* out) {
+    auto byte_width =
+        
std::static_pointer_cast<FixedSizeBinaryType>(left.type())->byte_width();

Review comment:
       maybe `checked_cast<const FixedSizeBinaryType&>(*left.type())` or 
`checked_pointer_cast<FixedSizeBinaryType>(left.type())` (will be dynamic cast 
in debug, static cast in release)

##########
File path: cpp/src/arrow/compute/kernels/scalar_if_else.cc
##########
@@ -893,6 +1031,21 @@ void AddBinaryIfElseKernels(const 
std::shared_ptr<IfElseFunction>& scalar_functi
   }
 }
 
+void AddFSBinaryIfElseKernel(const std::shared_ptr<IfElseFunction>& 
scalar_function) {
+  // cond array needs to be boolean always
+  ScalarKernel kernel(
+      {boolean(), InputType(Type::FIXED_SIZE_BINARY), 
InputType(Type::FIXED_SIZE_BINARY)},
+      OutputType([](KernelContext*, const std::vector<ValueDescr>& descrs) {
+        return ValueDescr(descrs[1].type, ValueDescr::ANY);
+      }),

Review comment:
       Hmm, do we need to check that the two branches have the same type width?

##########
File path: cpp/src/arrow/compute/kernels/scalar_if_else.cc
##########
@@ -770,6 +770,144 @@ struct IfElseFunctor<Type, enable_if_base_binary<Type>> {
   }
 };
 
+template <typename Type>
+struct IfElseFunctor<Type, enable_if_fixed_size_binary<Type>> {
+  // A - Array, S - Scalar, X = Array/Scalar
+
+  // SXX
+  static Status Call(KernelContext* ctx, const BooleanScalar& cond, const 
Datum& left,
+                     const Datum& right, Datum* out) {
+    auto byte_width =
+        
std::static_pointer_cast<FixedSizeBinaryType>(left.type())->byte_width();
+    return RunIfElseScalar(
+        cond, left, right, out,
+        /*CopyArrayData*/
+        [&](const ArrayData& valid_array, ArrayData* out_array) {
+          std::memcpy(
+              out_array->buffers[1]->mutable_data() + out_array->offset * 
byte_width,
+              valid_array.buffers[1]->mutable_data() + valid_array.offset * 
byte_width,

Review comment:
       nit: just `data()` here?




-- 
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