mapleFU commented on code in PR #46730:
URL: https://github.com/apache/arrow/pull/46730#discussion_r2146963301
##########
cpp/src/arrow/array/builder_binary.h:
##########
@@ -645,6 +664,35 @@ class ARROW_EXPORT BinaryViewBuilder : public ArrayBuilder
{
UnsafeAppend(value.data(), static_cast<int64_t>(value.size()));
}
+ Result<int32_t> AppendBuffer(const uint8_t* value, const int64_t length) {
+ if (ARROW_PREDICT_FALSE(length <= TypeClass::kInlineSize)) {
+ return Status::Invalid(
+ "The size of buffer to append should be larger than kInlineSize");
+ }
+ ARROW_RETURN_NOT_OK(data_heap_builder_.FinishLastBlock());
+ ARROW_ASSIGN_OR_RAISE(auto v,
+ data_heap_builder_.Append</*Safe=*/true>(value,
length));
+ return v.ref.buffer_index;
+ }
+
+ Result<int32_t> AppendBuffer(const char* value, const int64_t length) {
+ return AppendBuffer(reinterpret_cast<const uint8_t*>(value), length);
+ }
+
+ Result<int32_t> AppendBuffer(const std::string& value) {
Review Comment:
Can `std::string_view` being used?
##########
cpp/src/arrow/array/builder_binary.h:
##########
@@ -645,6 +664,35 @@ class ARROW_EXPORT BinaryViewBuilder : public ArrayBuilder
{
UnsafeAppend(value.data(), static_cast<int64_t>(value.size()));
}
+ Result<int32_t> AppendBuffer(const uint8_t* value, const int64_t length) {
+ if (ARROW_PREDICT_FALSE(length <= TypeClass::kInlineSize)) {
+ return Status::Invalid(
+ "The size of buffer to append should be larger than kInlineSize");
+ }
+ ARROW_RETURN_NOT_OK(data_heap_builder_.FinishLastBlock());
+ ARROW_ASSIGN_OR_RAISE(auto v,
+ data_heap_builder_.Append</*Safe=*/true>(value,
length));
+ return v.ref.buffer_index;
+ }
+
+ Result<int32_t> AppendBuffer(const char* value, const int64_t length) {
+ return AppendBuffer(reinterpret_cast<const uint8_t*>(value), length);
+ }
+
+ Result<int32_t> AppendBuffer(const std::string& value) {
+ return AppendBuffer(value.data(), static_cast<int64_t>(value.size()));
+ }
+
+ Status AppendViewFromBuffer(const int32_t buffer_idx, const int32_t start,
+ const int32_t length) {
+ ARROW_RETURN_NOT_OK(Reserve(1));
+ UnsafeAppendToBitmap(true);
+ ARROW_ASSIGN_OR_RAISE(
+ auto v, data_heap_builder_.GetViewFromBuffer<true>(buffer_idx, start,
length));
Review Comment:
should we also adding a unsafe Append version?
##########
cpp/src/arrow/array/builder_binary.h:
##########
@@ -645,6 +664,35 @@ class ARROW_EXPORT BinaryViewBuilder : public ArrayBuilder
{
UnsafeAppend(value.data(), static_cast<int64_t>(value.size()));
}
+ Result<int32_t> AppendBuffer(const uint8_t* value, const int64_t length) {
+ if (ARROW_PREDICT_FALSE(length <= TypeClass::kInlineSize)) {
+ return Status::Invalid(
+ "The size of buffer to append should be larger than kInlineSize");
+ }
Review Comment:
I'm curious that would this api be a bit strict for outside user 🤔? Should
we add comment doc string for this, and add sample usage:
```
if (buffer.size() <= 12) {
// Append directly
for (...) {}
} else {
auto buffer_idx = AppendBuffer();
for (..) {
AppendViewFromBuffer(buffer_idx, ...);
}
}
--
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-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
