> On Oct. 3, 2012, 6:29 p.m., Julien Le Dem wrote: > > src/org/apache/pig/data/SchemaTupleBackend.java, line 311 > > <https://reviews.apache.org/r/7336/diff/1/?file=160420#file160420line311> > > > > why would you not throw an exception here?
Good call. > On Oct. 3, 2012, 6:29 p.m., Julien Le Dem wrote: > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POForEach.java, > > line 126 > > <https://reviews.apache.org/r/7336/diff/1/?file=160416#file160416line126> > > > > feel free to remove constructors that are not used anymore There's only one that isn't used anywhere (which I have now removed). The rest are used in niche places. > On Oct. 3, 2012, 6:29 p.m., Julien Le Dem wrote: > > src/org/apache/pig/data/SchemaTupleFactory.java, line 55 > > <https://reviews.apache.org/r/7336/diff/1/?file=160422#file160422line55> > > > > is empty schema the same as no schema? I guess my thought was that there isn't much value in generating empty Tuples. But I guess there could be if there are a lot of them? > On Oct. 3, 2012, 6:29 p.m., Julien Le Dem wrote: > > src/org/apache/pig/data/SchemaTupleFrontend.java, lines 124-125 > > <https://reviews.apache.org/r/7336/diff/1/?file=160423#file160423line124> > > > > why did you add this? Whoops. This is an artifact of some bugtesting. > On Oct. 3, 2012, 6:29 p.m., Julien Le Dem wrote: > > src/org/apache/pig/newplan/logical/relational/LogToPhyTranslationVisitor.java, > > line 864 > > <https://reviews.apache.org/r/7336/diff/1/?file=160426#file160426line864> > > > > when would that happen? If we believe it can't happen let's throw an > > exception here. > > That's the best way to get it fixed. amen > On Oct. 3, 2012, 6:29 p.m., Julien Le Dem wrote: > > test/org/apache/pig/test/TestLoad.java, line 333 > > <https://reviews.apache.org/r/7336/diff/1/?file=160432#file160432line333> > > > > when using assertTrue you can pass a message to help debugging. Like > > op.getClass().getName() I just changed the Assert. => static import (for a consistent style), but you're right in general. > On Oct. 3, 2012, 6:29 p.m., Julien Le Dem wrote: > > test/org/apache/pig/test/TestLoad.java, lines 354-357 > > <https://reviews.apache.org/r/7336/diff/1/?file=160432#file160432line354> > > > > probably not what you want lol. death to IDEs - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7336/#review12139 ----------------------------------------------------------- On Sept. 28, 2012, 12:21 a.m., Jonathan Coveney wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7336/ > ----------------------------------------------------------- > > (Updated Sept. 28, 2012, 12:21 a.m.) > > > Review request for pig. > > > Description > ------- > > This uses the SchemaTuple in Foreach, which means it will work after Loads > > > This addresses bug PIG-2877. > https://issues.apache.org/jira/browse/PIG-2877 > > > Diffs > ----- > > src/org/apache/pig/PigConfiguration.java bf13d9d > src/org/apache/pig/PigConstants.java PRE-CREATION > src/org/apache/pig/backend/hadoop/executionengine/HExecutionEngine.java > d5c9315 > src/org/apache/pig/backend/hadoop/executionengine/HJob.java 2ff294a > > src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java > 86d04f8 > > src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigGenericMapBase.java > 6ab33a4 > > src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigGenericMapReduce.java > 2e70462 > > src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/partitioners/WeightedRangePartitioner.java > d7504ae > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/PhysicalOperator.java > 26d5389 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POForEach.java > 4e6876a > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POSort.java > 702a01b > src/org/apache/pig/data/BinInterSedes.java a7ba39b > src/org/apache/pig/data/SchemaTuple.java 4f04834 > src/org/apache/pig/data/SchemaTupleBackend.java 2746316 > src/org/apache/pig/data/SchemaTupleClassGenerator.java ef15318 > src/org/apache/pig/data/SchemaTupleFactory.java b467d55 > src/org/apache/pig/data/SchemaTupleFrontend.java a8bfbab > src/org/apache/pig/impl/builtin/FindQuantiles.java 1fcbdb1 > src/org/apache/pig/impl/io/ReadToEndLoader.java afb6ebd > > src/org/apache/pig/newplan/logical/relational/LogToPhyTranslationVisitor.java > 5c3dc43 > src/org/apache/pig/parser/LogicalPlanBuilder.java b0a8aa0 > test/org/apache/pig/data/TestSchemaTuple.java 24142c2 > test/org/apache/pig/test/TestBuiltin.java a835993 > test/org/apache/pig/test/TestCommit.java 5baaedc > test/org/apache/pig/test/TestExampleGenerator.java 4babccb > test/org/apache/pig/test/TestLoad.java a34e2bb > test/org/apache/pig/test/TestPigServer.java 18a3dff > test/org/apache/pig/test/TestPigServerWithMacros.java c6d8c4e > > Diff: https://reviews.apache.org/r/7336/diff/ > > > Testing > ------- > > > Thanks, > > Jonathan Coveney > >
