[GitHub] [arrow] carols10cents opened a new pull request #8962: ARROW-10960: [C++] [Flight] Default to empty buffer instead of null

[GitBox]

carols10cents opened a new pull request #8962:
URL: https://github.com/apache/arrow/pull/8962


   (repeating context from the Jira issue)
   
   # Problem
   
   ProtoBuf `proto3` specifies that [if a message does not contain a particular 
singular element, the field should get the default 
value](https://developers.google.com/protocol-buffers/docs/proto3#default). 
However, when the C++ `flight-test-integration-server` gets a `DoPut` request 
with a `FlightData` message for a record batch containing no items, and the 
`FlightData` is missing the `data_body` field, the server responds with an 
error "Expected body in IPC message of type record batch".
   
   ## What happens
   
   If I run the C++ `flight-test-integration-server` and the C++ 
`flight-test-integration-client` with the `generated_null_trivial` test case, 
the test passes and I see this in wireshark:
   
   <img width="712" alt="cpp-client-empty-data-body" 
src="https://user-images.githubusercontent.com/193874/102633180-13168780-411e-11eb-87a0-375271a39f83.png";>
   
   Note the `data_body` field is present but has no value.
   
   If I run the Rust `flight-test-integration-client` that I'm working on 
developing, it does not send the `data_body` field at all if there are no bytes 
to send. I see this in wireshark:
   
   <img width="715" alt="rust-client-missing-data-body" 
src="https://user-images.githubusercontent.com/193874/102633213-1e69b300-411e-11eb-9f38-7d5e2759fbb6.png";>
   
   Note the `data_body` field is not present.
   
   The C++ server then returns the error message `Expected body in IPC message 
of type record batch`, which comes from [this check for message 
body](https://github.com/apache/arrow/blob/519e9da4fc1698f686525f4226295f3680a3f3db/cpp/src/arrow/ipc/reader.cc#L92)
 called in [`ReadNext` of the record batch stream 
reader](https://github.com/apache/arrow/blob/519e9da4fc1698f686525f4226295f3680a3f3db/cpp/src/arrow/ipc/reader.cc#L787).
   
   ## What I expect to happen
   
   Instead of returning an error message because of a null pointer, the Message 
should get the default value of empty bytes.
   
   @shepmaster and I worked on this fix together, but I'm not at all confident 
this is the exact right fix. It's hard for me to trace through the code from a 
`FlightData` `data_body` to an IPC `Message` `body`, but this does fix the 
problem. I'm also not sure what other cases this change might affect. Feedback 
much appreciated!


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org