----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62422/#review186295 -----------------------------------------------------------
src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeJoin.java Lines 153-154 (original), 154-155 (patched) <https://reviews.apache.org/r/62422/#comment262774> Remove src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeJoin.java Line 190 (original), 191 (patched) <https://reviews.apache.org/r/62422/#comment262775> super(copy); src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeJoin.java Lines 194-198 (patched) <https://reviews.apache.org/r/62422/#comment262776> Please mark all of this as transient and remove from here. src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeJoin.java Lines 206-207 (patched) <https://reviews.apache.org/r/62422/#comment262777> Please mark all of this as transient and remove from here. src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeJoin.java Lines 209-211 (patched) <https://reviews.apache.org/r/62422/#comment262778> Please mark all of this as transient and remove from here. src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeJoin.java Lines 215 (patched) <https://reviews.apache.org/r/62422/#comment262779> Please mark as transient and remove from here. src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeJoin.java Line 198 (original), 218 (patched) <https://reviews.apache.org/r/62422/#comment262780> Remove. super(copy) should handle this. src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeJoinTez.java Line 59 (original), 57 (patched) <https://reviews.apache.org/r/62422/#comment262790> transient src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeJoinTez.java Line 80 (original), 78 (patched) <https://reviews.apache.org/r/62422/#comment262791> You will have to implement this as well. Once you construct the index, you will have to cache it. Refer to POBloomFilterRearrangeTez src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeJoinTez.java Line 89 (original), 85 (patched) <https://reviews.apache.org/r/62422/#comment262781> Add this outside of the try catch block similar to POValueInputTez. if (input == null) { throw new ExecException("Input from vertex " + inputKey + " is missing"); } src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeJoinTez.java Line 102 (original), 95 (patched) <https://reviews.apache.org/r/62422/#comment262784> Can you inline this method in the attachInputs() before getRightLoader() ? You will have to cache the index after constructing it. Refer to POBloomFilterRearrangeTez src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeJoinTez.java Line 103 (original), 96 (patched) <https://reviews.apache.org/r/62422/#comment262782> Remove this and name the class variable as reader. src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezCompiler.java Lines 1049 (patched) <https://reviews.apache.org/r/62422/#comment262801> configureIndexerOp src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezCompiler.java Line 1105 (original), 1114 (patched) <https://reviews.apache.org/r/62422/#comment262803> Can you also add indexAggrOper.markIndexer(); src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezCompiler.java Line 1106 (original), 1115 (patched) <https://reviews.apache.org/r/62422/#comment262797> Why is setTaskIndexWithRecordIndexAsKey required? You still need indexAggrOper.setClosed(true); src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezCompiler.java Line 1122 (original), 1133 (patched) <https://reviews.apache.org/r/62422/#comment262798> Remove this line as there is only one DAG now src/org/apache/pig/impl/builtin/TezIndexableLoader.java Line 31 (original), 31 (patched) <https://reviews.apache.org/r/62422/#comment262792> Keep this as setIndex src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeCogroup.java Line 81 (original), 81 (patched) <https://reviews.apache.org/r/62422/#comment262771> Remove this variable. It should work without it as the super class PhysicalOperator has a mTupleFactory. src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeCogroup.java Lines 118 (patched) <https://reviews.apache.org/r/62422/#comment262772> super(copy); src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeCogroup.java Lines 127 (patched) <https://reviews.apache.org/r/62422/#comment262773> Remove. Should be handled by super(copy) src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeCogroupTez.java Lines 39 (patched) <https://reviews.apache.org/r/62422/#comment262796> Implement clone() for POMergeCogroupTez and POMergeJoinTez src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeCogroupTez.java Lines 75 (patched) <https://reviews.apache.org/r/62422/#comment262794> Null check for input src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeCogroupTez.java Lines 104 (patched) <https://reviews.apache.org/r/62422/#comment262795> Cache index and implement addInputsToSkip - Rohini Palaniswamy On Sept. 22, 2017, 5:19 p.m., Satish Saley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62422/ > ----------------------------------------------------------- > > (Updated Sept. 22, 2017, 5:19 p.m.) > > > Review request for pig. > > > Bugs: PIG-4120 > https://issues.apache.org/jira/browse/PIG-4120 > > > Repository: pig-git > > > Description > ------- > > PIG-4120 Broadcast the index file in case of POMergeCoGroup and POMergeJoin > > > Diffs > ----- > > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeCogroup.java > f18d47a34 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeCogroupTez.java > PRE-CREATION > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeJoin.java > 815a32586 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POMergeJoinTez.java > PRE-CREATION > src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezCompiler.java > 79739e98a > src/org/apache/pig/impl/builtin/DefaultIndexableLoader.java a4688e499 > src/org/apache/pig/impl/builtin/TezIndexableLoader.java PRE-CREATION > test/org/apache/pig/test/data/GoldenFiles/tez/TEZC-MergeCogroup-1.gld > PRE-CREATION > test/org/apache/pig/test/data/GoldenFiles/tez/TEZC-MergeJoin-1.gld > PRE-CREATION > test/org/apache/pig/tez/TestTezCompiler.java f99d6f39c > > > Diff: https://reviews.apache.org/r/62422/diff/2/ > > > Testing > ------- > > > Thanks, > > Satish Saley > >
