You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@hive.apache.org by Harish Butani <rh...@gmail.com> on 2014/06/18 22:40:42 UTC

Review Request 22752: HIVE-7063: Optimize for the Top N within a Group use case

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

Review request for hive and Ashutosh Chauhan.


Bugs: HIVE-7063
    https://issues.apache.org/jira/browse/HIVE-7063


Repository: hive-git


Description
-------

It is common to rank within a Group/Partition and then only return the Top N entries within each Group.
With Streaming mode for Windowing, we should push the post filter on the rank into the Windowing processing as a Limit expression.


Diffs
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/PTFTopNHash.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java 03a64e8 
  ql/src/java/org/apache/hadoop/hive/ql/exec/TopNHash.java bc81467 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorReduceSinkOperator.java 11024da 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ReduceSinkDesc.java 8c1d336 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ptf/WindowTableFunctionDef.java c547e62 
  ql/src/java/org/apache/hadoop/hive/ql/ppd/OpProcFactory.java 7aaf455 
  ql/src/java/org/apache/hadoop/hive/ql/udf/ptf/WindowingTableFunction.java 2290766 
  ql/src/test/queries/clientpositive/windowing_streaming.q PRE-CREATION 
  ql/src/test/results/clientpositive/windowing_streaming.q.out PRE-CREATION 

Diff: https://reviews.apache.org/r/22752/diff/


Testing
-------

added new .q tests


Thanks,

Harish Butani

Re: Review Request 22752: HIVE-7063: Optimize for the Top N within a Group use case

Posted by Harish Butani <rh...@gmail.com>.


> On June 28, 2014, 3:34 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/ppd/OpProcFactory.java, line 250
> > <https://reviews.apache.org/r/22752/diff/2/?file=612613#file612613line250>
> >
> >     will be good to document that we currently optimize only for expressions like rank() < integer-constant.

done


> On June 28, 2014, 3:34 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/ppd/OpProcFactory.java, line 299
> > <https://reviews.apache.org/r/22752/diff/2/?file=612613#file612613line299>
> >
> >     Is this correct? Can we push for lead as well ?

yes we can push lead. have added a comment explaining this.


> On June 28, 2014, 3:34 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/ppd/OpProcFactory.java, line 304
> > <https://reviews.apache.org/r/22752/diff/2/?file=612613#file612613line304>
> >
> >     Don't we need to do this and following check even for rank functions? 
> >     Can you add some comments for this function, I didnt understand it fully.

for rank, drank, lead we only need to consider rows before current row, so window doesn't matter. Also for rank/drank window is not allowed.


> On June 28, 2014, 3:34 a.m., Ashutosh Chauhan wrote:
> > ql/src/test/queries/clientpositive/windowing_streaming.q, line 38
> > <https://reviews.apache.org/r/22752/diff/2/?file=612615#file612615line38>
> >
> >     Can you also add explains for these plans, which hopefully also indicates that rank got pushed in map. Figuring whether rank got pushed or not will be useful in future for perf debugging.

done


> On June 28, 2014, 3:34 a.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java, line 179
> > <https://reviews.apache.org/r/22752/diff/2/?file=612608#file612608line179>
> >
> >     Wondering if their is a better way to do this conditional initialization, especially given Operator::acceptLimitPushdown() which will come in play when we optimize for limit clause. Though, current way work for what we are trying to optimize at the moment.

Yes this is not an issue because today limit doesn't get pushed past PTF. If we do push past limit then as you pointed out it would still get applied in a manner similar to a rank predicate.


- Harish


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


On June 18, 2014, 8:40 p.m., Harish Butani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22752/
> -----------------------------------------------------------
> 
> (Updated June 18, 2014, 8:40 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-7063
>     https://issues.apache.org/jira/browse/HIVE-7063
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> It is common to rank within a Group/Partition and then only return the Top N entries within each Group.
> With Streaming mode for Windowing, we should push the post filter on the rank into the Windowing processing as a Limit expression.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/PTFTopNHash.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java 03a64e8 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TopNHash.java bc81467 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorReduceSinkOperator.java 11024da 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ReduceSinkDesc.java 8c1d336 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ptf/WindowTableFunctionDef.java c547e62 
>   ql/src/java/org/apache/hadoop/hive/ql/ppd/OpProcFactory.java 7aaf455 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/ptf/WindowingTableFunction.java 2290766 
>   ql/src/test/queries/clientpositive/windowing_streaming.q PRE-CREATION 
>   ql/src/test/results/clientpositive/windowing_streaming.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22752/diff/
> 
> 
> Testing
> -------
> 
> added new .q tests
> 
> 
> Thanks,
> 
> Harish Butani
> 
>

Re: Review Request 22752: HIVE-7063: Optimize for the Top N within a Group use case

Posted by Ashutosh Chauhan <ha...@apache.org>.

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



ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java
<https://reviews.apache.org/r/22752/#comment82505>

    Wondering if their is a better way to do this conditional initialization, especially given Operator::acceptLimitPushdown() which will come in play when we optimize for limit clause. Though, current way work for what we are trying to optimize at the moment.



ql/src/java/org/apache/hadoop/hive/ql/ppd/OpProcFactory.java
<https://reviews.apache.org/r/22752/#comment82507>

    will be good to document that we currently optimize only for expressions like rank() < integer-constant.



ql/src/java/org/apache/hadoop/hive/ql/ppd/OpProcFactory.java
<https://reviews.apache.org/r/22752/#comment82506>

    Is this correct? Can we push for lead as well ?



ql/src/java/org/apache/hadoop/hive/ql/ppd/OpProcFactory.java
<https://reviews.apache.org/r/22752/#comment82508>

    Don't we need to do this and following check even for rank functions? 
    Can you add some comments for this function, I didnt understand it fully.



ql/src/test/queries/clientpositive/windowing_streaming.q
<https://reviews.apache.org/r/22752/#comment82509>

    Can you also add explains for these plans, which hopefully also indicates that rank got pushed in map. Figuring whether rank got pushed or not will be useful in future for perf debugging.


- Ashutosh Chauhan


On June 18, 2014, 8:40 p.m., Harish Butani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22752/
> -----------------------------------------------------------
> 
> (Updated June 18, 2014, 8:40 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-7063
>     https://issues.apache.org/jira/browse/HIVE-7063
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> It is common to rank within a Group/Partition and then only return the Top N entries within each Group.
> With Streaming mode for Windowing, we should push the post filter on the rank into the Windowing processing as a Limit expression.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/PTFTopNHash.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ReduceSinkOperator.java 03a64e8 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TopNHash.java bc81467 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorReduceSinkOperator.java 11024da 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ReduceSinkDesc.java 8c1d336 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ptf/WindowTableFunctionDef.java c547e62 
>   ql/src/java/org/apache/hadoop/hive/ql/ppd/OpProcFactory.java 7aaf455 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/ptf/WindowingTableFunction.java 2290766 
>   ql/src/test/queries/clientpositive/windowing_streaming.q PRE-CREATION 
>   ql/src/test/results/clientpositive/windowing_streaming.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22752/diff/
> 
> 
> Testing
> -------
> 
> added new .q tests
> 
> 
> Thanks,
> 
> Harish Butani
> 
>