- 
                Notifications
    You must be signed in to change notification settings 
- Fork 3.9k
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