Skip to content
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

[C++][FlightRPC] Missing protobuf data_body should result in default value of empty bytes, not null #26883

Closed
asfimport opened this issue Dec 18, 2020 · 1 comment

Comments

@asfimport
Copy link
Collaborator

asfimport commented Dec 18, 2020

Problem

ProtoBuf proto3 specifies that if a message does not contain a particular singular element, the field should get the default value. 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 the protobuf in wireshark shown in the cpp-client-empty-data-body.png attachment.

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 the protobuf in wireshark shown in the rust-client-missing-data-body.png attachment.

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 called in ReadNext of the record batch stream reader.

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.

Reporter: Carol Nichols / @carols10cents
Assignee: Carol Nichols / @carols10cents

Related issues:

Original Issue Attachments:

PRs and other links:

Note: This issue was originally created as ARROW-10960. Please see the migration documentation for further details.

@asfimport
Copy link
Collaborator Author

David Li / @lidavidm:
Issue resolved by pull request 8962
#8962

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant