-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-460: [C++] JSON read/write for dictionaries #750
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
Conversation
Change-Id: Ib91476b24b1bfe11be7607d9bc0a324a6d99ca2d
Change-Id: Id87c9f768b26e0c5548f69f077e370cdefcbc464
Change-Id: I4a341f9fdd6b2fb12311908a30b26c85e76a8781
Change-Id: I30a98d0245971bc156d227206f02215cd027b4a0
Change-Id: I6d1146de9131d323d3ac4c66f74750dad4a82f95
|
Since |
|
We don't use the typeLayout at all on read; we use the |
Change-Id: I07aa680108570f2ea9cb462376e540fa0038b991
|
OK, dictionaries are now in the top level. here's an example json: https://gist.github.com/wesm/5100e41173a3b5e53437b7a887e4383a#file-gistfile1-json |
|
As you can see in the JSON, the typeLayout I am writing is the index type |
|
+1. Merging this and we can reconcile JSON format discrepancies when the Java version is complete and we add the integration tests themselves |
|
Sorry @wesm , I got mixed up between |
|
I'll post my PR soon, there is still a couple things that need to be worked out, but it can read/write JSON so we can start testing. |
|
The typeLayout right now is deterministic based on the other type metadata, so we don't need it. I can change the typeLayout to be the dictionary type layout if it causes a problem |
cc @BryanCutler; this ended up being rather tedious.
As one uncertainty: I wasn't sure what to write for the
typeLayoutfield for dictionary fields. We don't use this at all in C++, so let me know what you need in Java (if anything)