----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29644/#review67958 -----------------------------------------------------------
trunk/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POStore.java <https://reviews.apache.org/r/29644/#comment112033> Why do we need this? trunk/test/org/apache/pig/test/TestMultiQueryLocal.java <https://reviews.apache.org/r/29644/#comment112036> If the location is different why do we have to pass two diffent keys for two different output formats to get the test passing? trunk/test/org/apache/pig/test/TestMultiQueryLocal.java <https://reviews.apache.org/r/29644/#comment112038> Can you add PIG-3993 to the message? Easy to search later trunk/test/org/apache/pig/test/TestMultiQueryLocal.java <https://reviews.apache.org/r/29644/#comment112042> 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); trunk/test/org/apache/pig/test/TestStore.java <https://reviews.apache.org/r/29644/#comment112052> This method should just call setupPigServer. trunk/test/org/apache/pig/test/TestStoreBase.java <https://reviews.apache.org/r/29644/#comment112045> You will have to add @Ignore for it to be skipped in mapred mode when running all tests. I had to add it for TestSecondarySort. I have forgotten the full details, but the comment I had added in that class is //ant sends abstract classes also to junit. skipNonTests attribute does not work with all ant versions. trunk/test/org/apache/pig/test/TestStoreBase.java <https://reviews.apache.org/r/29644/#comment112053> Should remove references to setupPigServer() inside test methods as local mode and cluster mode tests are now separated and PigServer is initialized in setup() method. trunk/test/org/apache/pig/test/TestStoreLocal.java <https://reviews.apache.org/r/29644/#comment112047> The old TestStore code had it, but do we actually need this setting? Not done for other local test classes. trunk/test/org/apache/pig/tez/TestPOPartialAggPlanTez.java <https://reviews.apache.org/r/29644/#comment112049> Can we move this method from the middle to the end after the tests or to the beginning of the tests? - Rohini Palaniswamy On Jan. 6, 2015, 11:47 p.m., Daniel Dai wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29644/ > ----------------------------------------------------------- > > (Updated Jan. 6, 2015, 11:47 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 > 1645046 > trunk/test/excluded-tests-20 1645046 > trunk/test/org/apache/pig/test/TestMultiQueryLocal.java 1645046 > trunk/test/org/apache/pig/test/TestMultiQueryLocalMR.java PRE-CREATION > trunk/test/org/apache/pig/test/TestPOPartialAggPlan.java 1645046 > trunk/test/org/apache/pig/test/TestPOPartialAggPlanMR.java PRE-CREATION > trunk/test/org/apache/pig/test/TestStore.java 1645046 > 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/org/apache/pig/tez/TezUtil.java PRE-CREATION > trunk/test/tez-local-tests 1645046 > > Diff: https://reviews.apache.org/r/29644/diff/ > > > Testing > ------- > > > Thanks, > > Daniel Dai > >
