ARROW-9362: [Java] Support reading/writing V5 MetadataVersion #7685
Conversation
|
Does Java already implement the required IPC union layout and semantics? Otherwise perhaps we should defer this PR until the union work is done. |
|
We should wait for #7290, yes. (Is anyone reviewing it?) Also, this now checks the metadata version against the schema before writing. |
lidavidm
changed the title
rymurr
left a comment
rymurr
left a comment
There was a problem hiding this comment.
LGTM. Awesome to get this in!
I like the DFS, I suppose the recursive algorithm wont cause a problem for any normal schema.
BryanCutler
left a comment
BryanCutler
left a comment
There was a problem hiding this comment.
LGTM. I mostly looked at the vector ipc and tests, and just had a minor question.
There was a problem hiding this comment.
doesn't this need an IpcOption to set MetadataVersion.V4?
There was a problem hiding this comment.
Good catch, thank you. I've added the IpcOption.
|
So is this ok to merge before #7290 ? |
This should be OK. It doesn't enable unions in the integration tests and prevents reading/writing unions with the old version. (Of course, someone could build master and start writing files with V5 metadata and the wrong union layout?) |
wesm
changed the title
This also enables Flight to write differing metadata versions. Not implemented: any checks for unions in read/write based on metadata version. I refactored TestFileWriter very heavily as I wanted to ensure IpcOptions was thoroughly tested and didn't want to duplicate code. I hope this doesn't make the change too hard to review. Closes apache#7685 from lidavidm/arrow-9362 Authored-by: David Li <li.davidm96@gmail.com> Signed-off-by: Wes McKinney <wesm@apache.org>
4 participants
This also enables Flight to write differing metadata versions.
Not implemented: any checks for unions in read/write based on metadata version.
I refactored TestFileWriter very heavily as I wanted to ensure IpcOptions was thoroughly tested and didn't want to duplicate code. I hope this doesn't make the change too hard to review.