search

apache / arrow

Apache Arrow is the universal columnar format and multi-language toolbox for fast data interchange and in-memory analytics
https://arrow.apache.org/
Apache License 2.0
14.63k stars 3.56k forks source link

GH-44795: [C++] Use arrow::util::span on arrow::util::bitmap_builders_utilities instead of std::vector #44796

Open raulcd opened 1 day ago

raulcd commented 1 day ago

Rationale for this change

arrow::util::span (a backport of C++20 std::span) is more generally applicable than std::vector, so any public API currently accepting a vector const-ref argument should instead accept a span argument.

What changes are included in this PR?

arrow::util::BytesToBits accepts arrow::util::span instead of std::vector

Are these changes tested?

Yes, existing C++ tests via CI

Are there any user-facing changes?

Yes, from Result<std::shared_ptr<Buffer>> BytesToBits(const std::vector<uint8_t>&, MemoryPool* pool) to Result<std::shared_ptr<Buffer>> BytesToBits(util::span<const uint8_t> bytes, MemoryPool* pool)

github-actions[bot] commented 1 day ago

:warning: GitHub issue #44795 has been automatically assigned in GitHub to PR creator.

raulcd commented 1 day ago

The macOS CI failures are unrelated, I opened: https://github.com/apache/arrow/issues/44797 @pitrou is something like this what you had in mind? I did it for a single API method just to validate I understood what you had in mind. I can open subtasks or aggregate some more on a single PR.

pitrou commented 1 day ago

@raulcd Yes, this is the kind of change I had in mind. See comments.