> On March 1, 2013, 2:54 p.m., Jonathan Coveney wrote: > > contrib/piggybank/java/src/main/java/org/apache/pig/piggybank/storage/LTSVLoader.java, > > line 265 > > <https://reviews.apache.org/r/9685/diff/1/?file=263710#file263710line265> > > > > you don't reference this anywhere else, so I'd just inline the check as > > "if (line == null)" and so on. Java is verbose enough as it is :)
I will fix it and other similar kind of things. Actually I prefer a verbose style of coding, but I admit it is a bit excessive :) > On March 1, 2013, 2:54 p.m., Jonathan Coveney wrote: > > contrib/piggybank/java/src/main/java/org/apache/pig/piggybank/storage/LTSVLoader.java, > > line 157 > > <https://reviews.apache.org/r/9685/diff/1/?file=263710#file263710line157> > > > > In the case where they do not give it a schema, I think that they > > shouldn't have to actually specify that it is a map. It's implicit. > > Furthermore, I think that instead of passing the Schema as an argument, if > > they do an "as (schema)" THEN it should attempt to return a Schema in > > accordance with what you provide. > > > > Open to argument, though. > In the case where they do not give it a schema, I think that they shouldn't > have to actually specify that it is a map. Yes ":map[]" is redundant. I will remove it. > if they do an "as (schema)" THEN it should attempt to return a Schema in > accordance with what you provide. Does this code meet your point? <code> access = LOAD 'access.log' USING org.apache.pig.piggybank.storage.LTSVLoader() AS (host:chararray, ua:chararray); ua = FOREACH access GENERATE ua; dump ua; </code> It will be good, but I could not find a way to implement the function in that manner. LoadFunc seems to have no method which can get an input schema specified by users. - Taku ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9685/#review17239 ----------------------------------------------------------- On Feb. 28, 2013, 10:53 p.m., Taku Miyakawa wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9685/ > ----------------------------------------------------------- > > (Updated Feb. 28, 2013, 10:53 p.m.) > > > Review request for pig. > > > Description > ------- > > This is a review board for https://issues.apache.org/jira/browse/PIG-3215 > > The patch adds LTSVLoader function and its test class. > > > This addresses bug PIG-3215. > https://issues.apache.org/jira/browse/PIG-3215 > > > Diffs > ----- > > > contrib/piggybank/java/src/main/java/org/apache/pig/piggybank/storage/LTSVLoader.java > PRE-CREATION > > contrib/piggybank/java/src/test/java/org/apache/pig/piggybank/test/storage/TestLTSVLoader.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/9685/diff/ > > > Testing > ------- > > ant compile-test > cd contrib/piggybank/java/ > ant -Dtestcase=TestLTSVLoader test > > > Thanks, > > Taku Miyakawa > >
