----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62600/#review186428 -----------------------------------------------------------
Skipping null keys needs to be also implemented for skewed join and frjoin. Also needs to be done in MR mode. src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POLocalRearrange.java Line 379 (original), 394 (patched) <https://reviews.apache.org/r/62600/#comment262989> Remove this line src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POLocalRearrange.java Lines 413-418 (patched) <https://reviews.apache.org/r/62600/#comment262994> This should be in POJoinLocalRearrange src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POLocalRearrange.java Lines 414 (patched) <https://reviews.apache.org/r/62600/#comment262993> This should be part of PigCounters group incrCounter(PigCounters.SKIPPED_NULL_KEYS, 1) src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezCompiler.java Line 893 (original), 894-895 (patched) <https://reviews.apache.org/r/62600/#comment262999> Looks like dev changes. Revert src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezCompiler.java Lines 1389 (patched) <https://reviews.apache.org/r/62600/#comment263001> Instead of POLocalRearrangeTez extending POJoinLocalRearrange, can we have a POJoinLocalRearrangeTez that extends POLocalRearrangeTez and overrides just constructLROutput method. We can replace the POLocalRearrangeTez with POJoinLocalRearrangeTez here if we are setting skipNullKeys to true. This will avoid the skipNullKeys check for other rearranges (group by, order by, etc) The POJoinLocalRearrange operator can be used with mapreduce. src/org/apache/pig/backend/hadoop/executionengine/tez/plan/operator/POJoinLocalRearrange.java Lines 36 (patched) <https://reviews.apache.org/r/62600/#comment263000> Implement clone() method src/org/apache/pig/backend/hadoop/executionengine/tez/plan/operator/POJoinLocalRearrange.java Lines 58 (patched) <https://reviews.apache.org/r/62600/#comment262995> Should be overriding constructLROutput and not constructKey src/org/apache/pig/backend/hadoop/executionengine/tez/plan/operator/POJoinLocalRearrange.java Lines 60 (patched) <https://reviews.apache.org/r/62600/#comment262988> if (this.skipNullKeys && !useSecondaryKey && isNullKey(key)) { PigStatusReporter.getInstance().incrCounter(PigCounters.SKIPPED_NULL_KEYS, 1); ret.returnStatus = POStatus.STATUS_NULL; return; } ret.result = constructLROutput(key, value); ret.returnStatus = POStatus.STATUS_OK; src/org/apache/pig/backend/hadoop/executionengine/tez/plan/operator/POJoinLocalRearrange.java Lines 72-73 (patched) <https://reviews.apache.org/r/62600/#comment262996> } else if src/org/apache/pig/backend/hadoop/executionengine/tez/plan/operator/POJoinLocalRearrange.java Lines 73 (patched) <https://reviews.apache.org/r/62600/#comment262997> getKeyType() -> keyType - Rohini Palaniswamy On Sept. 26, 2017, 11:27 p.m., Satish Saley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62600/ > ----------------------------------------------------------- > > (Updated Sept. 26, 2017, 11:27 p.m.) > > > Review request for pig. > > > Bugs: PIG-4662 > https://issues.apache.org/jira/browse/PIG-4662 > > > Repository: pig-git > > > Description > ------- > > PIG-4662 New optimizer rule: filter nulls before inner joins > > > Diffs > ----- > > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POLocalRearrange.java > 4e39beb8b > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POPartitionRearrange.java > 9181a7b78 > src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezCompiler.java > 79739e98a > > src/org/apache/pig/backend/hadoop/executionengine/tez/plan/operator/POJoinLocalRearrange.java > PRE-CREATION > > src/org/apache/pig/backend/hadoop/executionengine/tez/plan/operator/POLocalRearrangeTez.java > 1552103cd > > src/org/apache/pig/backend/hadoop/executionengine/tez/plan/operator/POPartitionRearrangeTez.java > 95034924d > > > Diff: https://reviews.apache.org/r/62600/diff/1/ > > > Testing > ------- > > > Thanks, > > Satish Saley > >
