----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9481/#review16691 -----------------------------------------------------------
src/org/apache/pig/builtin/ToJson.java <https://reviews.apache.org/r/9481/#comment35397> Is this stageful? Is there any reason it shouldn't be static? src/org/apache/pig/builtin/ToJson.java <https://reviews.apache.org/r/9481/#comment35398> What if the schema is null? src/org/apache/pig/builtin/ToJson.java <https://reviews.apache.org/r/9481/#comment35399> This should be lazily initialized once src/org/apache/pig/builtin/ToJson.java <https://reviews.apache.org/r/9481/#comment35400> you can just annotate this class with @OutputSchema("chararray") and get rid of this method src/org/apache/pig/builtin/ToJson.java <https://reviews.apache.org/r/9481/#comment35402> I'm still unsure why you need a ResourceSchema here...FieldSchema has alias info etc src/org/apache/pig/builtin/ToJson.java <https://reviews.apache.org/r/9481/#comment35401> give variables meaningful names :) src/org/apache/pig/builtin/ToJson.java <https://reviews.apache.org/r/9481/#comment35403> Maps can actually have complex values (i.e. a tuple of stuff, which is itself nested). You will need to check for that based on the Schema src/org/apache/pig/builtin/ToJson.java <https://reviews.apache.org/r/9481/#comment35405> Issue with this, from json spec: "An object is an unordered set of name/value pairs." This means that there is no guarantee that we will deserialize tuples in the same order we serialized them, since order is not necessarily maintained. You could instead serialize bags and tuples as lists, and just have a convention for the name i.e. alias_tuple, and alias_bag. src/org/apache/pig/builtin/ToJson.java <https://reviews.apache.org/r/9481/#comment35404> you should just test this on initialization, you can go through and make sure the Schema is valid test/org/apache/pig/test/TestToJson.java <https://reviews.apache.org/r/9481/#comment35406> add a test for map with a complex key, and add some more nested stuff. test/org/apache/pig/test/TestToJson.java <https://reviews.apache.org/r/9481/#comment35407> you can do a try {} finally {} and add a removeOutput, and then get rid of it in setUp and tearDown - Jonathan Coveney On Feb. 16, 2013, 11:56 p.m., Russell Jurney wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9481/ > ----------------------------------------------------------- > > (Updated Feb. 16, 2013, 11:56 p.m.) > > > Review request for pig, Ashutosh Chauhan, Dmitriy Ryaboy, Alan Gates, > Jonathan Coveney, and Bill Graham. > > > Description > ------- > > This patch is for JIRA, https://issues.apache.org/jira/browse/PIG-2641 > PIG-2641 - which adds a ToJson builtin based on the serialization in > JsonStorage. > > > This addresses bug PIG-2641. > https://issues.apache.org/jira/browse/PIG-2641 > > > Diffs > ----- > > src/org/apache/pig/builtin/ToJson.java PRE-CREATION > test/org/apache/pig/test/TestToJson.java PRE-CREATION > test/org/apache/pig/test/data/jsonStorage1.tojson PRE-CREATION > > Diff: https://reviews.apache.org/r/9481/diff/ > > > Testing > ------- > > This works for me locally, and there is a working unit test. > > > Thanks, > > Russell Jurney > >
