Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-44795: [C++] Use arrow::util::span<T> on BooleanBuilder APIs instead of std::vector<T>& #44813

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

GH-44795: [C++] Use arrow::util::span<T> on BooleanBuilder APIs instead of std::vector<T>& #44813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3ec464b
GH-44795: [C++] Use arrow::util::span<T> on BooleanBuilder APIs inste…
raulcd Nov 22, 2024
1facf30
Revert changes from util::span<bool> to std::vector<bool>&
raulcd Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cpp/src/arrow/array/builder_primitive.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,12 @@ Status BooleanBuilder::AppendValues(const uint8_t* values, int64_t length,
return Status::OK();
}

Status BooleanBuilder::AppendValues(const std::vector<uint8_t>& values,
Status BooleanBuilder::AppendValues(const util::span<uint8_t> values,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, const std::vector<T>& should be replaced with util::span<const T>, not const util::span<T> (because a span's constness doesn't propagate to the elements it points to, unlike vector).

const std::vector<bool>& is_valid) {
return AppendValues(values.data(), static_cast<int64_t>(values.size()), is_valid);
}

Status BooleanBuilder::AppendValues(const std::vector<uint8_t>& values) {
Status BooleanBuilder::AppendValues(const util::span<uint8_t> values) {
return AppendValues(values.data(), static_cast<int64_t>(values.size()));
}

Expand Down
18 changes: 9 additions & 9 deletions cpp/src/arrow/array/builder_primitive.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,34 +443,34 @@ class ARROW_EXPORT BooleanBuilder
/// \brief Append a sequence of elements in one shot
/// \param[in] values a contiguous C array of values
/// \param[in] length the number of values to append
/// \param[in] is_valid an std::vector<bool> indicating valid (1) or null
/// \param[in] is_valid a std::vector<bool> indicating valid (1) or null
/// (0). Equal in length to values
/// \return Status
Status AppendValues(const uint8_t* values, int64_t length,
const std::vector<bool>& is_valid);

/// \brief Append a sequence of elements in one shot
/// \param[in] values a std::vector of bytes
/// \param[in] is_valid an std::vector<bool> indicating valid (1) or null
/// \param[in] values an arrow::util::span of bytes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can just say "a span of bytes" IMHO.

/// \param[in] is_valid a std::vector<bool> indicating valid (1) or null
/// (0). Equal in length to values
/// \return Status
Status AppendValues(const std::vector<uint8_t>& values,
Status AppendValues(const util::span<uint8_t> values,
const std::vector<bool>& is_valid);

/// \brief Append a sequence of elements in one shot
/// \param[in] values a std::vector of bytes
/// \param[in] values an arrow::util::span of bytes
/// \return Status
Status AppendValues(const std::vector<uint8_t>& values);
Status AppendValues(const util::span<uint8_t> values);

/// \brief Append a sequence of elements in one shot
/// \param[in] values an std::vector<bool> indicating true (1) or false
/// \param[in] is_valid an std::vector<bool> indicating valid (1) or null
/// \param[in] values a std::vector<bool> indicating true (1) or false
/// \param[in] is_valid a std::vector<bool> indicating valid (1) or null
/// (0). Equal in length to values
/// \return Status
Status AppendValues(const std::vector<bool>& values, const std::vector<bool>& is_valid);

/// \brief Append a sequence of elements in one shot
/// \param[in] values an std::vector<bool> indicating true (1) or false
/// \param[in] values a std::vector<bool> indicating true (1) or false
/// \return Status
Status AppendValues(const std::vector<bool>& values);

Expand Down
Loading