Skip to content

[C++] Compute: Why compute::internal::BooleanKeyEncoder doesn't handle !valid scalar as not-null? #43733

@mapleFU

Description

@mapleFU

Describe the enhancement requested

I'm current learning the code of compute::RowEncoder. I found that in FixedWidthKeyEncoder, Scalar detects is_valid:

Status FixedWidthKeyEncoder::Encode(const ExecValue& data, int64_t batch_length,
                                    uint8_t** encoded_bytes) {
  if (data.is_array()) {
    // ...
  } else {
    const auto& scalar = data.scalar_as<arrow::internal::PrimitiveScalarBase>();
    if (scalar.is_valid) { // <-- is valid is checked here..
      const std::string_view data = scalar.view();
      DCHECK_EQ(data.size(), static_cast<size_t>(byte_width_));
      for (int64_t i = 0; i < batch_length; i++) {
        auto& encoded_ptr = *encoded_bytes++;
        *encoded_ptr++ = kValidByte;
        memcpy(encoded_ptr, data.data(), data.size());
        encoded_ptr += byte_width_;
      }
    } else {
      for (int64_t i = 0; i < batch_length; i++) {
        auto& encoded_ptr = *encoded_bytes++;
        *encoded_ptr++ = kNullByte;
        memset(encoded_ptr, 0, byte_width_);
        encoded_ptr += byte_width_;
      }
    }
  }
  return Status::OK();
}

However, in BooleanKeyEncoder, this is not handled:

Status BooleanKeyEncoder::Encode(const ExecValue& data, int64_t batch_length,
                                 uint8_t** encoded_bytes) {
  if (data.is_array()) {
    // ..
  } else {
    const auto& scalar = data.scalar_as<BooleanScalar>();
    bool value = scalar.is_valid && scalar.value;  <-- boolean is handled in this way
    for (int64_t i = 0; i < batch_length; i++) {
      auto& encoded_ptr = *encoded_bytes++;
      *encoded_ptr++ = kValidByte;
      *encoded_ptr++ = value;
    }
  }
  return Status::OK();
}

Is this expected?

Component(s)

C++

Metadata

Metadata

Assignees

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions