[GitHub] [arrow] pitrou commented on a change in pull request #10538: ARROW-12955: [C++] Add additional type support for if_else kernel

[GitBox <gi...@apache.org>]

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



##########
File path: cpp/src/arrow/compute/kernels/scalar_if_else.cc
##########
@@ -353,6 +353,332 @@ struct IfElseFunctor<Type, enable_if_number<Type>> {
   }
 };
 
+template <typename Type>
+struct IfElseFunctor<Type, enable_if_base_binary<Type>> {
+  using OffsetType = typename TypeTraits<Type>::OffsetType::c_type;
+  using ArrayType = typename TypeTraits<Type>::ArrayType;
+
+  // A - Array
+  // S - Scalar
+
+  //  AAA
+  static Status Call(KernelContext* ctx, const ArrayData& cond, const ArrayData& left,
+                     const ArrayData& right, ArrayData* out) {
+    const uint8_t* cond_data = cond.buffers[1]->data();
+    BitBlockCounter bit_counter(cond_data, cond.offset, cond.length);
+
+    const auto* left_offsets = left.GetValues<OffsetType>(1);
+    const uint8_t* left_data = left.buffers[2]->data();
+    const auto* right_offsets = right.GetValues<OffsetType>(1);
+    const uint8_t* right_data = right.buffers[2]->data();
+
+    // reserve an additional space
+    ARROW_ASSIGN_OR_RAISE(auto out_offset_buf,
+                          ctx->Allocate((cond.length + 1) * sizeof(OffsetType)));
+    auto* out_offsets = reinterpret_cast<OffsetType*>(out_offset_buf->mutable_data());
+    out_offsets[0] = 0;
+
+    // allocate data buffer conservatively
+    auto data_buff_alloc =
+        static_cast<int64_t>((left_offsets[left.length] - left_offsets[0]) +
+                             (right_offsets[right.length] - right_offsets[0]));
+    ARROW_ASSIGN_OR_RAISE(std::shared_ptr<ResizableBuffer> out_data_buf,
+                          ctx->Allocate(data_buff_alloc));
+    uint8_t* out_data = out_data_buf->mutable_data();
+
+    int64_t offset = cond.offset;
+    OffsetType total_bytes_written = 0;
+    while (offset < cond.offset + cond.length) {

Review comment:
       It seems like this is copying data even if the output would be null. That doesn't sound like a good idea (if the filter bitmap has 99% nulls, you want those 99% null string slots in the output to take 0 data space).




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org