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] incompatible with java client for empty record batches #26866

Closed
asfimport opened this issue Dec 16, 2020 · 3 comments
Closed

Comments

@asfimport
Copy link
Collaborator

asfimport commented Dec 16, 2020

An error has been found when one sends an empty record batch from C# server and tries to read it with the java client.

From investigation the java client requires the protobuf tags to be sent in the message even though it is empty. Java code can be seen here:

https://github.com/apache/arrow/blob/master/java/flight/flight-core/src/main/java/org/apache/arrow/flight/ArrowMessage.java

Line 257-301 (the error is that it wont accept a null body for a record batch)

Normal functionality of gRPC is to exclude the entire tag if an object is empty, example code from generated csharp:

if (DataBody.Length != 0)

{ output.WriteRawTag(194, 62); output.WriteBytes(DataBody); }

To fix this so the csharp version is compatible with the java client requires a non empty flight data body must be sent or at least the tag of the body.

Reporter: Alexander / @Ulimo

Related issues:

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

@asfimport
Copy link
Collaborator Author

Alexander / @Ulimo:
@eerhardt since you reviewed the flight code, this is something I discovered. Wondered if you could give some input if you have time.

In my opinion this should probably be solved in the java client, but I were thinking of making a "ugly" fix for the csharp code so its atleast compatible with java client as it is right now and it can be removed later. This is because the java client has quite a lot more custom code and a change there will probably take some time.

I will send in a PR atleast I think and we can see if its something that should be included or not.

@asfimport
Copy link
Collaborator Author

David Li / @lidavidm:
Thanks for catching this. The C++ implementation has a similar issue. I've filed ARROW-10962 to fix this on the Java side (and ARROW-10960 should fix this on the C++ side).

@CurtHagenlocher
Copy link
Contributor

Closing this issue as the problem seems to have been identified as being on the Java (and C++) sides.

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

2 participants