-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-7979: [C++] Add experimental buffer compression to IPC write path. Add "field" selection to read path. Migrate some APIs to Result<T>. Read/write Message metadata #6638
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.
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
Changes from all commits
a78b47e
a18f105
3bc293d
d51ff24
ff3cbde
7c98ee8
c4d5454
96b8770
10be7d5
36a68b7
c714f2b
4a60848
9c54534
a8a37c3
ac22b0e
9ff78b9
3c9176d
23f27d6
3e8a341
67ad393
bffef74
b9f47c5
db5a9a8
e20343e
ce08470
e7a4943
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -265,7 +265,7 @@ class GrpcStreamReader : public FlightStreamReader { | |
|
|
||
| private: | ||
| friend class GrpcIpcMessageReader; | ||
| std::unique_ptr<ipc::RecordBatchReader> batch_reader_; | ||
| std::shared_ptr<ipc::RecordBatchReader> batch_reader_; | ||
| std::shared_ptr<Buffer> last_app_metadata_; | ||
| std::shared_ptr<ClientRpc> rpc_; | ||
| }; | ||
|
|
@@ -327,8 +327,8 @@ Status GrpcStreamReader::Open(std::unique_ptr<ClientRpc> rpc, | |
| out->get()->rpc_ = std::move(rpc); | ||
| std::unique_ptr<GrpcIpcMessageReader> message_reader( | ||
| new GrpcIpcMessageReader(out->get(), out->get()->rpc_, std::move(stream))); | ||
| return ipc::RecordBatchStreamReader::Open(std::move(message_reader), | ||
| &(*out)->batch_reader_); | ||
| return (ipc::RecordBatchStreamReader::Open(std::move(message_reader)) | ||
| .Value(&(*out)->batch_reader_)); | ||
| } | ||
|
|
||
| std::shared_ptr<Schema> GrpcStreamReader::schema() const { | ||
|
|
@@ -385,9 +385,6 @@ class GrpcStreamWriter : public FlightStreamWriter { | |
| } | ||
| return Status::OK(); | ||
| } | ||
| void set_memory_pool(MemoryPool* pool) override { | ||
| batch_writer_->set_memory_pool(pool); | ||
| } | ||
|
||
| Status Close() override { return batch_writer_->Close(); } | ||
|
|
||
| private: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: avoid using
Result::Valueunless wrapping a result returning API into a status returning APIThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is using
Result::Valuebad? Not obvious from reading the docstringsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not bad, just less preferred due to mixing argout Status return and Result usage