This is an automated email from the ASF dual-hosted git repository.
apitrou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new c49c47d ARROW-8367: [C++] Deprecate Buffer::FromString(...,
MemoryPool*)
c49c47d is described below
commit c49c47d3d9ea3acda7beaf4b59be082c944906f6
Author: Antoine Pitrou <[email protected]>
AuthorDate: Wed Apr 8 14:10:14 2020 +0200
ARROW-8367: [C++] Deprecate Buffer::FromString(..., MemoryPool*)
Closes #6869 from pitrou/ARROW-8367-deprecate-buffer-fromstring-pool
Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/arrow/buffer.cc | 15 ++++++++-------
cpp/src/arrow/buffer.h | 22 +++++++---------------
cpp/src/arrow/buffer_test.cc | 2 +-
cpp/src/arrow/filesystem/mockfs.cc | 3 +--
cpp/src/arrow/flight/flight_test.cc | 7 +++----
cpp/src/arrow/flight/internal.cc | 6 ++++--
6 files changed, 24 insertions(+), 31 deletions(-)
diff --git a/cpp/src/arrow/buffer.cc b/cpp/src/arrow/buffer.cc
index edb4796..52229b8 100644
--- a/cpp/src/arrow/buffer.cc
+++ b/cpp/src/arrow/buffer.cc
@@ -67,21 +67,22 @@ bool Buffer::Equals(const Buffer& other) const {
!memcmp(data_, other.data_,
static_cast<size_t>(size_))));
}
-Result<std::shared_ptr<Buffer>> Buffer::FromString(const std::string& data,
- MemoryPool* pool) {
+static Status BufferFromString(const std::string& data, MemoryPool* pool,
+ std::shared_ptr<Buffer>* out) {
auto size = static_cast<int64_t>(data.size());
ARROW_ASSIGN_OR_RAISE(auto buf, AllocateBuffer(size, pool));
std::copy(data.c_str(), data.c_str() + size, buf->mutable_data());
- return std::move(buf);
+ *out = std::move(buf);
+ return Status::OK();
}
Status Buffer::FromString(const std::string& data, MemoryPool* pool,
std::shared_ptr<Buffer>* out) {
- return FromString(data, pool).Value(out);
+ return BufferFromString(data, pool, out);
}
Status Buffer::FromString(const std::string& data, std::shared_ptr<Buffer>*
out) {
- return FromString(data).Value(out);
+ return BufferFromString(data, default_memory_pool(), out);
}
std::string Buffer::ToString() const {
@@ -127,7 +128,7 @@ Result<std::shared_ptr<Buffer>> Buffer::ViewOrCopy(
class StlStringBuffer : public Buffer {
public:
- explicit StlStringBuffer(std::string&& data)
+ explicit StlStringBuffer(std::string data)
: Buffer(nullptr, 0), input_(std::move(data)) {
data_ = reinterpret_cast<const uint8_t*>(input_.c_str());
size_ = static_cast<int64_t>(input_.size());
@@ -138,7 +139,7 @@ class StlStringBuffer : public Buffer {
std::string input_;
};
-std::shared_ptr<Buffer> Buffer::FromString(std::string&& data) {
+std::shared_ptr<Buffer> Buffer::FromString(std::string data) {
return std::make_shared<StlStringBuffer>(std::move(data));
}
diff --git a/cpp/src/arrow/buffer.h b/cpp/src/arrow/buffer.h
index 807b16e..6838029 100644
--- a/cpp/src/arrow/buffer.h
+++ b/cpp/src/arrow/buffer.h
@@ -145,28 +145,20 @@ class ARROW_EXPORT Buffer {
}
}
- /// \brief Construct a new buffer that owns its memory from a std::string
- ///
- /// The string data is copied into the newly-allocated buffer memory.
+ /// \brief Construct an immutable buffer that takes ownership of the contents
+ /// of an std::string (without copying it).
///
- /// \param[in] data a std::string object
- /// \param[in] pool a memory pool
- static Result<std::shared_ptr<Buffer>> FromString(
- const std::string& data, MemoryPool* pool ARROW_MEMORY_POOL_DEFAULT);
+ /// \param[in] data a string to own
+ /// \return a new Buffer instance
+ static std::shared_ptr<Buffer> FromString(std::string data);
- ARROW_DEPRECATED("Use Result-returning version")
+ ARROW_DEPRECATED("Use Buffer-returning version")
static Status FromString(const std::string& data, MemoryPool* pool,
std::shared_ptr<Buffer>* out);
- ARROW_DEPRECATED("Use Result-returning version")
+ ARROW_DEPRECATED("Use Buffer-returning version")
static Status FromString(const std::string& data, std::shared_ptr<Buffer>*
out);
- /// \brief Construct an immutable buffer that takes ownership of the contents
- /// of an std::string
- /// \param[in] data an rvalue-reference of a string
- /// \return a new Buffer instance
- static std::shared_ptr<Buffer> FromString(std::string&& data);
-
/// \brief Create buffer referencing typed memory with some length without
/// copying
/// \param[in] data the typed memory as C array
diff --git a/cpp/src/arrow/buffer_test.cc b/cpp/src/arrow/buffer_test.cc
index 460da3d..9bbef94 100644
--- a/cpp/src/arrow/buffer_test.cc
+++ b/cpp/src/arrow/buffer_test.cc
@@ -366,7 +366,7 @@ TEST(TestBuffer, FromStdStringWithMemory) {
{
std::string temp = "hello, world";
- ASSERT_OK_AND_ASSIGN(buf, Buffer::FromString(temp));
+ buf = Buffer::FromString(temp);
AssertIsCPUBuffer(*buf);
ASSERT_EQ(0, memcmp(buf->data(), temp.c_str(), temp.size()));
ASSERT_EQ(static_cast<int64_t>(temp.size()), buf->size());
diff --git a/cpp/src/arrow/filesystem/mockfs.cc
b/cpp/src/arrow/filesystem/mockfs.cc
index 521ac66..f5ae5e4 100644
--- a/cpp/src/arrow/filesystem/mockfs.cc
+++ b/cpp/src/arrow/filesystem/mockfs.cc
@@ -371,8 +371,7 @@ class MockFileSystem::Impl {
if (!entry->is_file()) {
return NotAFile(path);
}
- ARROW_ASSIGN_OR_RAISE(auto buffer,
Buffer::FromString(entry->as_file().data));
- return std::make_shared<io::BufferReader>(buffer);
+ return
std::make_shared<io::BufferReader>(Buffer::FromString(entry->as_file().data));
}
};
diff --git a/cpp/src/arrow/flight/flight_test.cc
b/cpp/src/arrow/flight/flight_test.cc
index de8b1e3..3342925 100644
--- a/cpp/src/arrow/flight/flight_test.cc
+++ b/cpp/src/arrow/flight/flight_test.cc
@@ -390,7 +390,7 @@ class TestFlightClient : public ::testing::Test {
class AuthTestServer : public FlightServerBase {
Status DoAction(const ServerCallContext& context, const Action& action,
std::unique_ptr<ResultStream>* result) override {
- ARROW_ASSIGN_OR_RAISE(auto buf,
Buffer::FromString(context.peer_identity()));
+ auto buf = Buffer::FromString(context.peer_identity());
*result = std::unique_ptr<ResultStream>(new
SimpleResultStream({Result{buf}}));
return Status::OK();
}
@@ -717,8 +717,7 @@ class ReportContextTestServer : public FlightServerBase {
if (middleware == nullptr || middleware->name() !=
"TracingServerMiddleware") {
buf = Buffer::FromString("");
} else {
- ARROW_ASSIGN_OR_RAISE(
- buf, Buffer::FromString(((const
TracingServerMiddleware*)middleware)->span_id));
+ buf = Buffer::FromString(((const
TracingServerMiddleware*)middleware)->span_id);
}
*result = std::unique_ptr<ResultStream>(new
SimpleResultStream({Result{buf}}));
return Status::OK();
@@ -1003,7 +1002,7 @@ TEST_F(TestFlightClient, DoAction) {
action.type = "action1";
const std::string action1_value = "action1-content";
- action.body = *Buffer::FromString(action1_value);
+ action.body = Buffer::FromString(action1_value);
ASSERT_OK(client_->DoAction(action, &stream));
for (int i = 0; i < 3; ++i) {
diff --git a/cpp/src/arrow/flight/internal.cc b/cpp/src/arrow/flight/internal.cc
index bd5afa7..3cea1c9 100644
--- a/cpp/src/arrow/flight/internal.cc
+++ b/cpp/src/arrow/flight/internal.cc
@@ -305,7 +305,8 @@ Status ToProto(const ActionType& type, pb::ActionType*
pb_type) {
Status FromProto(const pb::Action& pb_action, Action* action) {
action->type = pb_action.type();
- return Buffer::FromString(pb_action.body()).Value(&action->body);
+ action->body = Buffer::FromString(pb_action.body());
+ return Status::OK();
}
Status ToProto(const Action& action, pb::Action* pb_action) {
@@ -321,7 +322,8 @@ Status ToProto(const Action& action, pb::Action* pb_action)
{
Status FromProto(const pb::Result& pb_result, Result* result) {
// ARROW-3250; can avoid copy. Can also write custom deserializer if it
// becomes an issue
- return Buffer::FromString(pb_result.body()).Value(&result->body);
+ result->body = Buffer::FromString(pb_result.body());
+ return Status::OK();
}
Status ToProto(const Result& result, pb::Result* pb_result) {
