[
https://issues.apache.org/jira/browse/ARROW-2388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16425403#comment-16425403
]
ASF GitHub Bot commented on ARROW-2388:
---------------------------------------
xhochy closed pull request #1833: ARROW-2388: [C++] Use valid_bytes API for
StringBuilder::Append
URL: https://github.com/apache/arrow/pull/1833
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/cpp/src/arrow/array-test.cc b/cpp/src/arrow/array-test.cc
index 308bbcd8a..0b4342a53 100644
--- a/cpp/src/arrow/array-test.cc
+++ b/cpp/src/arrow/array-test.cc
@@ -991,13 +991,13 @@ TEST_F(TestStringBuilder, TestScalarAppend) {
TEST_F(TestStringBuilder, TestAppendVector) {
vector<string> strings = {"", "bb", "a", "", "ccc"};
- vector<uint8_t> is_null = {0, 0, 0, 1, 0};
+ vector<uint8_t> valid_bytes = {1, 1, 1, 0, 1};
int N = static_cast<int>(strings.size());
int reps = 1000;
for (int j = 0; j < reps; ++j) {
- ASSERT_OK(builder_->Append(strings, is_null.data()));
+ ASSERT_OK(builder_->Append(strings, valid_bytes.data()));
}
Done();
@@ -1008,9 +1008,7 @@ TEST_F(TestStringBuilder, TestAppendVector) {
int32_t length;
int32_t pos = 0;
for (int i = 0; i < N * reps; ++i) {
- if (is_null[i % N]) {
- ASSERT_TRUE(result_->IsNull(i));
- } else {
+ if (valid_bytes[i % N]) {
ASSERT_FALSE(result_->IsNull(i));
result_->GetValue(i, &length);
ASSERT_EQ(pos, result_->value_offset(i));
@@ -1018,6 +1016,8 @@ TEST_F(TestStringBuilder, TestAppendVector) {
ASSERT_EQ(strings[i % N], result_->GetString(i));
pos += length;
+ } else {
+ ASSERT_TRUE(result_->IsNull(i));
}
}
}
diff --git a/cpp/src/arrow/builder.cc b/cpp/src/arrow/builder.cc
index ec486566f..a502e1fc2 100644
--- a/cpp/src/arrow/builder.cc
+++ b/cpp/src/arrow/builder.cc
@@ -1386,7 +1386,7 @@ const uint8_t* BinaryBuilder::GetValue(int64_t i,
int32_t* out_length) const {
StringBuilder::StringBuilder(MemoryPool* pool) : BinaryBuilder(utf8(), pool) {}
Status StringBuilder::Append(const std::vector<std::string>& values,
- uint8_t* null_bytes) {
+ const uint8_t* valid_bytes) {
std::size_t total_length = std::accumulate(
values.begin(), values.end(), 0ULL,
[](uint64_t sum, const std::string& str) { return sum + str.size(); });
@@ -1394,16 +1394,22 @@ Status StringBuilder::Append(const
std::vector<std::string>& values,
RETURN_NOT_OK(value_data_builder_.Reserve(total_length));
RETURN_NOT_OK(offsets_builder_.Reserve(values.size()));
- for (std::size_t i = 0; i < values.size(); ++i) {
- RETURN_NOT_OK(AppendNextOffset());
- if (null_bytes[i]) {
- UnsafeAppendToBitmap(false);
- } else {
+ if (valid_bytes) {
+ for (std::size_t i = 0; i < values.size(); ++i) {
+ RETURN_NOT_OK(AppendNextOffset());
+ if (valid_bytes[i]) {
+ RETURN_NOT_OK(value_data_builder_.Append(
+ reinterpret_cast<const uint8_t*>(values[i].data()),
values[i].size()));
+ }
+ }
+ } else {
+ for (std::size_t i = 0; i < values.size(); ++i) {
+ RETURN_NOT_OK(AppendNextOffset());
RETURN_NOT_OK(value_data_builder_.Append(
reinterpret_cast<const uint8_t*>(values[i].data()),
values[i].size()));
- UnsafeAppendToBitmap(true);
}
}
+ UnsafeAppendToBitmap(valid_bytes, values.size());
return Status::OK();
}
diff --git a/cpp/src/arrow/builder.h b/cpp/src/arrow/builder.h
index cdcee80be..32cfdd408 100644
--- a/cpp/src/arrow/builder.h
+++ b/cpp/src/arrow/builder.h
@@ -718,7 +718,8 @@ class ARROW_EXPORT StringBuilder : public BinaryBuilder {
using BinaryBuilder::Append;
- Status Append(const std::vector<std::string>& values, uint8_t* null_bytes);
+ Status Append(const std::vector<std::string>& values,
+ const uint8_t* valid_bytes = NULLPTR);
};
// ----------------------------------------------------------------------
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> [C++] Arrow::StringBuilder::Append() uses null_bytes not valid_bytes
> --------------------------------------------------------------------
>
> Key: ARROW-2388
> URL: https://issues.apache.org/jira/browse/ARROW-2388
> Project: Apache Arrow
> Issue Type: Improvement
> Components: C++
> Affects Versions: 0.10.0
> Reporter: Kouhei Sutou
> Assignee: Kouhei Sutou
> Priority: Minor
> Labels: pull-request-available
> Fix For: 0.10.0
>
>
> Append of other builders use valid_bytes not null_bytes.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
