zeroshade commented on code in PR #1549:
URL: https://github.com/apache/arrow-go/pull/1549#discussion_r4566177510
##########
arrow/flight/record_batch_reader.go:
##########
@@ -56,28 +56,37 @@ func (d *dataMessageReader) Message() (*ipc.Message, error)
{
err error
)
- if d.peeked != nil {
- fd = d.peeked
- d.peeked = nil
- } else {
- fd, err = d.rdr.Recv()
- }
+ for {
+ if d.peeked != nil {
+ fd = d.peeked
+ d.peeked = nil
+ } else {
+ fd, err = d.rdr.Recv()
+ }
- if err != nil {
- if d.msg != nil {
- // clear the previous message in the error case
- d.msg.Release()
- d.msg = nil
+ if err != nil {
+ if d.msg != nil {
+ // clear the previous message in the error case
+ d.msg.Release()
+ d.msg = nil
+ }
+ d.lastAppMetadata = nil
+ d.descr = nil
+ return nil, err
+ }
+
+ if fd.FlightDescriptor != nil {
+ d.descr = fd.FlightDescriptor
+ }
+ // a message with neither an IPC header nor a body only carries
+ // app metadata (see Writer.WriteMetadata); there is no IPC
message
+ // to decode, so skip it
+ if len(fd.DataHeader) > 0 || len(fd.DataBody) > 0 {
Review Comment:
Optional: a message with a body but no header still reaches
`ipc.NewMessage`, and mid-stream that returns the same cryptic `arrow/ipc:
unknown error while reading: runtime error: index out of range [3] with length
0`. Since this loop already inspects both fields, it could return a clear error
here instead, something like "FlightData has a body but no IPC header". That's
existing behaviour, so it can also be a follow-up.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
