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#] ArrowStreamReader doesn't consume whole stream and doesn't implement sync read #21497

Closed
asfimport opened this issue Mar 22, 2019 · 3 comments
Assignees
Labels
Component: C# Priority: Blocker Marks a blocker for the release Type: bug
Milestone

Comments

@asfimport
Copy link

There are 2 major issues with the ArrowStreamReader that are blocking me from using it.

  1. When it reads a batch from a .NET Stream that doesn't return the whole chunk of memory in one "Read" call (like a socket/network stream), it only calls Read once, and then continues on. This is an issue because it has "garbage" at the end of its buffer (which was never written to by the stream), and when attempting to read the next batch, it is in the middle of the previous batch from the .NET Stream. This causes all sorts of issues because it assumes the next 4 bytes are the message length, which it obviously isn't. See the reading code for where it only calls Read once - it should be in a loop.
  2. ArrowStreamReader has a synchronous ReadNextRecordBatch() method - but it throws NotImplementedException. This is necessary when a caller isn't in an async method, they can't/shouldn't call the async API.

Reporter: Eric Erhardt / @eerhardt
Assignee: Eric Erhardt / @eerhardt

PRs and other links:

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

@asfimport
Copy link
Author

Eric Erhardt / @eerhardt:
@wesm, @xhochy, @kou - this is a pretty major issue in the C# library that is blocking usage of the Arrow C# library over a network stream. I have a PR up to fix it that has been approved by Stephen Toub. I'm hoping this PR can be merged for the 0.13 release, as it is super important for our usages.

@asfimport
Copy link
Author

Kouhei Sutou / @kou:
Issue resolved by pull request 4017
#4017

@asfimport
Copy link
Author

Kouhei Sutou / @kou:
Thanks! I've merged.

@asfimport asfimport added this to the 0.13.0 milestone Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: C# Priority: Blocker Marks a blocker for the release Type: bug
Projects
None yet
Development

No branches or pull requests

2 participants