niyue commented on pull request #11486: URL: https://github.com/apache/arrow/pull/11486#issuecomment-954339392
@westonpace > I would like a unit test showing that a RecordBatchFileReader truly does not read the entire file Sure. Let me see how I can add more tests for it. > Maybe the ArrayLoader can move into message.cc I considered this approach as well, and I found this will introduce more changes to the existing reader/message APIs and I am not quite sure if this is desirable. It seems to me the current implementation places all concepts about arrow's structures in reader.cc while all flatbuffers structures are kept in the lower layer message.cc file. `ArrayLoader` involves quite a lot arrow structures, and I am not familiar with some of them, so I try to follow current organization to make it work so far. > This approach currently does not work for the asynchronous version of the reader... This can be handled in a follow-up. If you want to do that then please create a JIRA ticket for that work so we don't lose track. I didn't realize this previously since in my project I only use the sync version of the reader. I will look into it later. Since ARROW-12683 is not specific to sync version of the reader, if this PR is accepted, I think probably we can close ARROW-12683 and I will create a JIRA issue to track the async version of the reader enhancement as follow-up. What do you think? -- 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]
