-
Notifications
You must be signed in to change notification settings - Fork 1.5k
PARQUET-360: Handle all map key types with cat tool's json dump #259
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
PARQUET-360: Handle all map key types with cat tool's json dump #259
Conversation
50052ea to
e3c2669
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should use the BinaryNode that is used elsewhere to convert a Binary or byte[] to JSON. That will base64-encode the data so it is still binary rather than creating an array node.
6789d24 to
e4cd545
Compare
|
@rdblue updated, thanks. |
|
Sorry, I should have mentioned this earlier: can you also add a test or two that exercises the new keyToString function? |
d92f70e to
995de88
Compare
|
@rdblue yeah totally forgot to add them. updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you forgot to add a license header to this file and that's causing Travis CI to fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
995de88 to
d047502
Compare
|
Just tried to merge, but the format is wrong. Could you remove the square brackets from this PR title? Thanks! |
|
@rdblue updated the title. |
|
looks good to me. |
When dumping a parquet map with `parquet-cat --json` it throws a class cast exception as it doesn't properly handle all map key types. ``` java.lang.ClassCastException: [B cannot be cast to java.lang.String at org.apache.parquet.tools.read.SimpleMapRecord.toJsonObject(SimpleMapRecord.java:34) at org.apache.parquet.tools.read.SimpleRecord.toJsonValue(SimpleRecord.java:119) at org.apache.parquet.tools.read.SimpleRecord.toJsonObject(SimpleRecord.java:112) at org.apache.parquet.tools.read.SimpleRecord.prettyPrintJson(SimpleRecord.java:106) at org.apache.parquet.tools.command.CatCommand.execute(CatCommand.java:76) at org.apache.parquet.tools.Main.main(Main.java:222) [B cannot be cast to java.lang.String ``` Author: Nezih Yigitbasi <nyigitbasi@netflix.com> Closes apache#259 from nezihyigitbasi/parquet-cat-json and squashes the following commits: d047502 [Nezih Yigitbasi] Add unit test e4cd545 [Nezih Yigitbasi] Get rid of deprecated methods bdc8fdf [Nezih Yigitbasi] Handle all map key types with cat tool's json dump
When dumping a parquet map with
parquet-cat --jsonit throws a class cast exception as it doesn't properly handle all map key types.