hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ashutosh Chauhan" <hashut...@apache.org>
Subject Re: Review Request 12767: [HIVE-4877] In ExecReducer, remove tag from the row which will be passed to the first Operator at the Reduce-side
Date Fri, 19 Jul 2013 18:02:15 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12767/#review23527
-----------------------------------------------------------


Good work, Yin! some minor comments.


ql/src/java/org/apache/hadoop/hive/ql/exec/DemuxOperator.java
<https://reviews.apache.org/r/12767/#comment47408>

    I didn't get why there is an if check here? Can you add a comment explaining in which
case we need not to update this childOIs map?



ql/src/java/org/apache/hadoop/hive/ql/exec/DemuxOperator.java
<https://reviews.apache.org/r/12767/#comment47409>

    I think we should remove this if branch since its in inner loop of processing. We should
put this check in initialization time of the Demux operator. Even if we cannot put it there,
this will result in runtime exception which I think is fine.



ql/src/java/org/apache/hadoop/hive/ql/exec/DemuxOperator.java
<https://reviews.apache.org/r/12767/#comment47410>

    Is this better wording for this comment:
    // Demux operator forwards a row to exactly one child in its children list based on the
tag and newTagToChildIndex in process() method, so we need not to do anything in here.



ql/src/java/org/apache/hadoop/hive/ql/exec/MuxOperator.java
<https://reviews.apache.org/r/12767/#comment47414>

    Can you also add a line in comment saying, this key-val-tag structure is used by JoinOperator
and Groupby operators to function correctly.
    



ql/src/java/org/apache/hadoop/hive/ql/exec/MuxOperator.java
<https://reviews.apache.org/r/12767/#comment47411>

    Same comment as in Demux operator.



ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecReducer.java
<https://reviews.apache.org/r/12767/#comment47417>

    I think this should read as:
    // remove the tag from key coming out of reducer and store it in separate variable.


- Ashutosh Chauhan


On July 19, 2013, 5 p.m., Yin Huai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12767/
> -----------------------------------------------------------
> 
> (Updated July 19, 2013, 5 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-4877
>     https://issues.apache.org/jira/browse/HIVE-4877
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/HIVE-4877
> 
> 
> Diffs
> -----
> 
>   data/files/kv1kv2.cogroup.txt 6d36e22 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DemuxOperator.java 9898495 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/MuxOperator.java d4be3d9 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Operator.java ee76917 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java cbda70b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecReducer.java d12a53c 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/TestExecDriver.java 6a74ae4 
> 
> Diff: https://reviews.apache.org/r/12767/diff/
> 
> 
> Testing
> -------
> 
> Tests	Failures	Errors	Success rate	Time
> 2688	2	        0	99.93%	        43249.945
> 
> Two failures are hbase_stats_empty_partition.q and ppd_key_ranges.q in TestHBaseCliDriver.
> 
> I manually tested these two in my mac and tests passed. 
> 
> 
> Thanks,
> 
> Yin Huai
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message