> On June 15, 2014, 3:57 p.m., Rohini Palaniswamy wrote: > > test/org/apache/pig/test/Util.java, line 849 > > <https://reviews.apache.org/r/22535/diff/1/?file=608350#file608350line849> > > > > comp.aggregateScalarsFiles(); here? > > Koji Noguchi wrote: > I'm not familiar with the code, but it seems like test has two version of > MRPlan creation. > buildMRPlan() and buildMRPlanWithOptimizer(). Only the latter contains > comp.aggregateScalarsFiles. > > Since it was like this BEFORE my patch, I wanted to keep it that way. > I needed to add comp.connectSoftLink() since this was taken out from > comp.compile() by my patch. > > Having said that, I still need to run a full unit/e2e test. > > > > > > 853 public static MROperPlan buildMRPlan(PhysicalPlan pp, > PigContext pc) throws Exception{ > > 854 MRCompiler comp = new MRCompiler(pp, pc); > > 855 comp.compile(); > > + 856 comp.connectSoftLink(); > > 857 return comp.getMRPlan(); > > 858 } > > 859 > > 860 public static MROperPlan > buildMRPlanWithOptimizer(PhysicalPlan pp, PigContext pc) throws Exception { > > 861 MapRedUtil.checkLeafIsStore(pp, pc); > > 862 > > 863 MapReduceLauncher launcher = new MapReduceLauncher(); > > 864 > > 865 java.lang.reflect.Method compile = launcher.getClass() > > 866 .getDeclaredMethod("compile", > > 867 new Class[] { PhysicalPlan.class, > PigContext.class }); > > 868 > > 869 compile.setAccessible(true); > > 870 > > 871 return (MROperPlan) compile.invoke(launcher, new > Object[] { pp, pc }); > > 872 }
I ran the full unit test suite applying the patch and also adding comp.aggregateScalarsFiles(); to Util.buildMRPlan. It passed. So we should be good to add that and check in. Having comp.aggregateScalarsFiles(); added will make it more closer to the actual execution patch during tests as well. - Rohini ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22535/#review45552 ----------------------------------------------------------- On June 12, 2014, 9:51 p.m., Koji Noguchi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22535/ > ----------------------------------------------------------- > > (Updated June 12, 2014, 9:51 p.m.) > > > Review request for pig, Daniel Dai and Rohini Palaniswamy. > > > Bugs: PIG-3975 > https://issues.apache.org/jira/browse/PIG-3975 > > > Repository: pig-git > > > Description > ------- > > PIG-3975: Multiple Scalar reference calls leading to missing records > > > Diffs > ----- > > > src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/MRCompiler.java > 51014eb > > src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/MapReduceLauncher.java > e6a4261 > test/org/apache/pig/test/TestFRJoin2.java c32a2c5 > test/org/apache/pig/test/Util.java 97c45c7 > > Diff: https://reviews.apache.org/r/22535/diff/ > > > Testing > ------- > > > Thanks, > > Koji Noguchi > >
