> On Jan. 13, 2015, 10:52 p.m., Rohini Palaniswamy wrote: > > trunk/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POStore.java, > > line 338 > > <https://reviews.apache.org/r/29644/diff/1/?file=808161#file808161line338> > > > > Why do we need this? > > Daniel Dai wrote: > I need PigStoreTez comparable with PigStore. In TezCompiler, we add > PigStore into phyToTezOpMap, and later we change PigStore to PigStoreTez, > then we were not able to retrieve the Tez plan back.
Somehow does not feel clean. Can you clarify on where in TezCompiler this causes problem. We do this changing of POxxx to POxxxTez for other operators too. Might have to be handled for them as well. > On Jan. 13, 2015, 10:52 p.m., Rohini Palaniswamy wrote: > > trunk/test/org/apache/pig/test/TestMultiQueryLocal.java, line 447 > > <https://reviews.apache.org/r/29644/diff/1/?file=808163#file808163line447> > > > > If the location is different why do we have to pass two diffent keys > > for two different output formats to get the test passing? > > Daniel Dai wrote: > This test is to make sure different storer is using different context. So > I use different key for different storer for the test. There is a difference > which makes the test pass in MR. In tez, OutputFormat and Storer is using the > same context, while MR is not. I don't think this is a big issue. Don't we clone the context and use it for different Storers/OutputFormats? May be that is missing in Tez. We should fix it as using of same context by different Storers will cause settings to be overwritten. > On Jan. 13, 2015, 10:52 p.m., Rohini Palaniswamy wrote: > > trunk/test/org/apache/pig/test/TestMultiQueryLocal.java, line 680 > > <https://reviews.apache.org/r/29644/diff/1/?file=808163#file808163line680> > > > > Instead of splitting this test class into MR and Tez classes can we > > just add a method to Util class for the below line? > > > > Launcher launcher = Util.getLauncher(execType); > > Daniel Dai wrote: > I try to, but how to do that? Use reflection? Realized we don't have Tez classes in Util. We can add the static function to MiniCluster or create another shims TestUtil class in shims/test/hadoop23/org/apache/pig/test/ - Rohini ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29644/#review67958 ----------------------------------------------------------- On Jan. 16, 2015, 11:58 p.m., Daniel Dai wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29644/ > ----------------------------------------------------------- > > (Updated Jan. 16, 2015, 11:58 p.m.) > > > Review request for pig and Rohini Palaniswamy. > > > Repository: pig > > > Description > ------- > > See PIG-4359 > > > Diffs > ----- > > > trunk/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POStore.java > 1652499 > trunk/test/excluded-tests-20 1652499 > trunk/test/org/apache/pig/test/TestMultiQueryLocal.java 1652499 > trunk/test/org/apache/pig/test/TestMultiQueryLocalMR.java PRE-CREATION > trunk/test/org/apache/pig/test/TestPOPartialAggPlan.java 1652499 > trunk/test/org/apache/pig/test/TestPOPartialAggPlanMR.java PRE-CREATION > trunk/test/org/apache/pig/test/TestStore.java 1652499 > trunk/test/org/apache/pig/test/TestStoreBase.java PRE-CREATION > trunk/test/org/apache/pig/test/TestStoreLocal.java PRE-CREATION > trunk/test/org/apache/pig/tez/TestMultiQueryLocalTez.java PRE-CREATION > trunk/test/org/apache/pig/tez/TestPOPartialAggPlanTez.java PRE-CREATION > trunk/test/tez-local-tests 1652499 > > Diff: https://reviews.apache.org/r/29644/diff/ > > > Testing > ------- > > > Thanks, > > Daniel Dai > >
