I'm not sure if this is the right place to respond, but just a couple of comments:
1) Our latest patch for Jira 1914 ( https://issues.apache.org/jira/browse/PIG-1914) supports storing Maps with complex schemas. I think a lot of the tests will also be useful for this patch. 2) I don't think we should serialize tuples as json lists. While it preserves order (which is nice) I suspect most users would rather have the direct mapping of pig alias to json key and to have their tuples as json objects with key/value pairs instead of as a list which requires referencing fields by position. -- Jeremy Karn / Lead Developer MORTAR DATA / 519 277 4391 / www.mortardata.com On Mon, Feb 18, 2013 at 12:51 AM, Prashant Kommireddi <prash1...@gmail.com>wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9481/#review16700 > ----------------------------------------------------------- > > > > src/org/apache/pig/builtin/ToJson.java > <https://reviews.apache.org/r/9481/#comment35417> > > Hey Russell, you must have missed the apache header? > > > > src/org/apache/pig/builtin/ToJson.java > <https://reviews.apache.org/r/9481/#comment35418> > > Is this object used anywhere? You can may be get rid of this if not. > > > > src/org/apache/pig/builtin/ToJson.java > <https://reviews.apache.org/r/9481/#comment35419> > > This could probably be constructed lazily only once. Something like > > ByteArrayOutputStream baos; > > if(baos == null) { > baos = new ByteArrayOutputStream(BUF_SIZE); > } > > > > src/org/apache/pig/builtin/ToJson.java > <https://reviews.apache.org/r/9481/#comment35420> > > This can also be lazily created (once only)? > > > > test/org/apache/pig/test/TestToJson.java > <https://reviews.apache.org/r/9481/#comment35421> > > Looks like the Apache header is in here twice? You have one at the > top, this one could be removed. > > > - Prashant Kommireddi > > > On Feb. 17, 2013, 9:47 p.m., Russell Jurney wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > https://reviews.apache.org/r/9481/ > > ----------------------------------------------------------- > > > > (Updated Feb. 17, 2013, 9:47 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-2641PIG-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/JsonStorage.java 56086ff > > 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 > > > > >
