You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Jesús Camacho Rodríguez <jc...@hortonworks.com> on 2015/02/02 20:03:40 UTC

Re: Review Request 30254: HIVE-9444

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

(Updated Feb. 2, 2015, 7:03 p.m.)


Review request for hive and John Pullokkaran.


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


Repository: hive-git


Description
-------

HIVE-9444


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java 4d9d1da77394125cfcb9ac9ccf1c00528664b981 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/GlobalLimitOptimizer.java c9848dacd1a02db321583c2b91eb6d7317c295ff 

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


Testing
-------

Existing tests.


Thanks,

Jesús Camacho Rodríguez


Re: Review Request 30254: HIVE-9444

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30254/#review70991
-----------------------------------------------------------

Ship it!


Ship It!

- John Pullokkaran


On Feb. 4, 2015, 9:36 a.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30254/
> -----------------------------------------------------------
> 
> (Updated Feb. 4, 2015, 9:36 a.m.)
> 
> 
> Review request for hive and John Pullokkaran.
> 
> 
> Bugs: HIVE-9444
>     https://issues.apache.org/jira/browse/HIVE-9444
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-9444
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java 4d9d1da77394125cfcb9ac9ccf1c00528664b981 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/GlobalLimitOptimizer.java c9848dacd1a02db321583c2b91eb6d7317c295ff 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/GroupByDesc.java f031b281ff64706cdbbc1695a6f51282127487fa 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ReduceSinkDesc.java 43f8321107304c38ecd9f603cd3f8ce43e8496c8 
> 
> Diff: https://reviews.apache.org/r/30254/diff/
> 
> 
> Testing
> -------
> 
> Existing tests (global_limit.q).
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 30254: HIVE-9444

Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30254/
-----------------------------------------------------------

(Updated Feb. 4, 2015, 9:36 a.m.)


Review request for hive and John Pullokkaran.


Changes
-------

Adding additional information about classes/subclasses to API.


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


Repository: hive-git


Description
-------

HIVE-9444


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java 4d9d1da77394125cfcb9ac9ccf1c00528664b981 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/GlobalLimitOptimizer.java c9848dacd1a02db321583c2b91eb6d7317c295ff 
  ql/src/java/org/apache/hadoop/hive/ql/plan/GroupByDesc.java f031b281ff64706cdbbc1695a6f51282127487fa 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ReduceSinkDesc.java 43f8321107304c38ecd9f603cd3f8ce43e8496c8 

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


Testing
-------

Existing tests (global_limit.q).


Thanks,

Jesús Camacho Rodríguez


Re: Review Request 30254: HIVE-9444

Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.

> On Feb. 3, 2015, 8:25 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java, line 175
> > <https://reviews.apache.org/r/30254/diff/4/?file=846475#file846475line175>
> >
> >     What happens when you have more than one operator for the given class? wouldn't it overwrite the old value?

It is a multimap, so it will have a list of values (in this case operators) for every key (in this case, classes).


- Jesús


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


On Feb. 3, 2015, 7:51 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30254/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2015, 7:51 p.m.)
> 
> 
> Review request for hive and John Pullokkaran.
> 
> 
> Bugs: HIVE-9444
>     https://issues.apache.org/jira/browse/HIVE-9444
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-9444
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java 4d9d1da77394125cfcb9ac9ccf1c00528664b981 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/GlobalLimitOptimizer.java c9848dacd1a02db321583c2b91eb6d7317c295ff 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/GroupByDesc.java f031b281ff64706cdbbc1695a6f51282127487fa 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ReduceSinkDesc.java 43f8321107304c38ecd9f603cd3f8ce43e8496c8 
> 
> Diff: https://reviews.apache.org/r/30254/diff/
> 
> 
> Testing
> -------
> 
> Existing tests (global_limit.q).
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 30254: HIVE-9444

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30254/#review70820
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java
<https://reviews.apache.org/r/30254/#comment116259>

    What happens when you have more than one operator for the given class? wouldn't it overwrite the old value?


- John Pullokkaran


On Feb. 3, 2015, 7:51 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30254/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2015, 7:51 p.m.)
> 
> 
> Review request for hive and John Pullokkaran.
> 
> 
> Bugs: HIVE-9444
>     https://issues.apache.org/jira/browse/HIVE-9444
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-9444
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java 4d9d1da77394125cfcb9ac9ccf1c00528664b981 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/GlobalLimitOptimizer.java c9848dacd1a02db321583c2b91eb6d7317c295ff 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/GroupByDesc.java f031b281ff64706cdbbc1695a6f51282127487fa 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ReduceSinkDesc.java 43f8321107304c38ecd9f603cd3f8ce43e8496c8 
> 
> Diff: https://reviews.apache.org/r/30254/diff/
> 
> 
> Testing
> -------
> 
> Existing tests (global_limit.q).
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 30254: HIVE-9444

Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.

> On Feb. 3, 2015, 8:20 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java, line 155
> > <https://reviews.apache.org/r/30254/diff/4/?file=846475#file846475line155>
> >
> >     How are we handling subclassess. i.e caller specified both parent class and child class ?
> >     
> >     Will it belong to two buckets or the first one?
> >     We may want to call out the behavior. May we just document that the API won't handle Parent, child classes both being specified. An assertion/exception would be nice.

In this case, it will belong to both.

I think it makes sense that the method handles Parent classes, e.g. assume we want to retrieve all the Join operators in the tree, independently of them being JoinOperator, MapJoinOperator, SMBMapJoinOperator, etc.; or all the join operators, and at the same time separate them following some hierarchy. 

But I agree a comment should be added about the behaviour when an operator implements more than one class; I'll do it right now.


- Jesús


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


On Feb. 3, 2015, 7:51 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30254/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2015, 7:51 p.m.)
> 
> 
> Review request for hive and John Pullokkaran.
> 
> 
> Bugs: HIVE-9444
>     https://issues.apache.org/jira/browse/HIVE-9444
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-9444
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java 4d9d1da77394125cfcb9ac9ccf1c00528664b981 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/GlobalLimitOptimizer.java c9848dacd1a02db321583c2b91eb6d7317c295ff 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/GroupByDesc.java f031b281ff64706cdbbc1695a6f51282127487fa 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ReduceSinkDesc.java 43f8321107304c38ecd9f603cd3f8ce43e8496c8 
> 
> Diff: https://reviews.apache.org/r/30254/diff/
> 
> 
> Testing
> -------
> 
> Existing tests (global_limit.q).
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 30254: HIVE-9444

Posted by John Pullokkaran <jp...@hortonworks.com>.

> On Feb. 3, 2015, 8:20 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java, line 155
> > <https://reviews.apache.org/r/30254/diff/4/?file=846475#file846475line155>
> >
> >     How are we handling subclassess. i.e caller specified both parent class and child class ?
> >     
> >     Will it belong to two buckets or the first one?
> >     We may want to call out the behavior. May we just document that the API won't handle Parent, child classes both being specified. An assertion/exception would be nice.
> 
> Jesús Camacho Rodríguez wrote:
>     In this case, it will belong to both.
>     
>     I think it makes sense that the method handles Parent classes, e.g. assume we want to retrieve all the Join operators in the tree, independently of them being JoinOperator, MapJoinOperator, SMBMapJoinOperator, etc.; or all the join operators, and at the same time separate them following some hierarchy. 
>     
>     But I agree a comment should be added about the behaviour when an operator implements more than one class; I'll do it right now.

yes, thats a valid point.


- John


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


On Feb. 3, 2015, 7:51 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30254/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2015, 7:51 p.m.)
> 
> 
> Review request for hive and John Pullokkaran.
> 
> 
> Bugs: HIVE-9444
>     https://issues.apache.org/jira/browse/HIVE-9444
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-9444
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java 4d9d1da77394125cfcb9ac9ccf1c00528664b981 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/GlobalLimitOptimizer.java c9848dacd1a02db321583c2b91eb6d7317c295ff 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/GroupByDesc.java f031b281ff64706cdbbc1695a6f51282127487fa 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ReduceSinkDesc.java 43f8321107304c38ecd9f603cd3f8ce43e8496c8 
> 
> Diff: https://reviews.apache.org/r/30254/diff/
> 
> 
> Testing
> -------
> 
> Existing tests (global_limit.q).
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 30254: HIVE-9444

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30254/#review70819
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java
<https://reviews.apache.org/r/30254/#comment116258>

    How are we handling subclassess. i.e caller specified both parent class and child class ?
    
    Will it belong to two buckets or the first one?
    We may want to call out the behavior. May we just document that the API won't handle Parent, child classes both being specified. An assertion/exception would be nice.


- John Pullokkaran


On Feb. 3, 2015, 7:51 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30254/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2015, 7:51 p.m.)
> 
> 
> Review request for hive and John Pullokkaran.
> 
> 
> Bugs: HIVE-9444
>     https://issues.apache.org/jira/browse/HIVE-9444
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-9444
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java 4d9d1da77394125cfcb9ac9ccf1c00528664b981 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/GlobalLimitOptimizer.java c9848dacd1a02db321583c2b91eb6d7317c295ff 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/GroupByDesc.java f031b281ff64706cdbbc1695a6f51282127487fa 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ReduceSinkDesc.java 43f8321107304c38ecd9f603cd3f8ce43e8496c8 
> 
> Diff: https://reviews.apache.org/r/30254/diff/
> 
> 
> Testing
> -------
> 
> Existing tests (global_limit.q).
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 30254: HIVE-9444

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30254/#review70854
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java
<https://reviews.apache.org/r/30254/#comment116300>

    Please add documentation to both api about parent-child relationships


- John Pullokkaran


On Feb. 3, 2015, 7:51 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30254/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2015, 7:51 p.m.)
> 
> 
> Review request for hive and John Pullokkaran.
> 
> 
> Bugs: HIVE-9444
>     https://issues.apache.org/jira/browse/HIVE-9444
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-9444
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java 4d9d1da77394125cfcb9ac9ccf1c00528664b981 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/GlobalLimitOptimizer.java c9848dacd1a02db321583c2b91eb6d7317c295ff 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/GroupByDesc.java f031b281ff64706cdbbc1695a6f51282127487fa 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ReduceSinkDesc.java 43f8321107304c38ecd9f603cd3f8ce43e8496c8 
> 
> Diff: https://reviews.apache.org/r/30254/diff/
> 
> 
> Testing
> -------
> 
> Existing tests (global_limit.q).
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 30254: HIVE-9444

Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30254/
-----------------------------------------------------------

(Updated Feb. 3, 2015, 7:51 p.m.)


Review request for hive and John Pullokkaran.


Changes
-------

Addressing John's comments.


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


Repository: hive-git


Description
-------

HIVE-9444


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java 4d9d1da77394125cfcb9ac9ccf1c00528664b981 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/GlobalLimitOptimizer.java c9848dacd1a02db321583c2b91eb6d7317c295ff 
  ql/src/java/org/apache/hadoop/hive/ql/plan/GroupByDesc.java f031b281ff64706cdbbc1695a6f51282127487fa 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ReduceSinkDesc.java 43f8321107304c38ecd9f603cd3f8ce43e8496c8 

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


Testing
-------

Existing tests (global_limit.q).


Thanks,

Jesús Camacho Rodríguez


Re: Review Request 30254: HIVE-9444

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30254/#review70773
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java
<https://reviews.apache.org/r/30254/#comment116181>

    Jesus,
    
      Instead of traversing the operator tree seperately looking for RS,GB,Filter,Limit it seems like if we have a method that can traverse the op tree and group ops to different buckets (like RS, Fil, GB,Sel, join, union...) that would be better. That way we traverse the tree once. 
      
     The other API (like existsOrdering...) could then take a collection of operators (RS, FIL ...)and asnwer the question.


- John Pullokkaran


On Feb. 3, 2015, 10:52 a.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30254/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2015, 10:52 a.m.)
> 
> 
> Review request for hive and John Pullokkaran.
> 
> 
> Bugs: HIVE-9444
>     https://issues.apache.org/jira/browse/HIVE-9444
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-9444
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java 4d9d1da77394125cfcb9ac9ccf1c00528664b981 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/GlobalLimitOptimizer.java c9848dacd1a02db321583c2b91eb6d7317c295ff 
> 
> Diff: https://reviews.apache.org/r/30254/diff/
> 
> 
> Testing
> -------
> 
> Existing tests (global_limit.q).
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 30254: HIVE-9444

Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30254/
-----------------------------------------------------------

(Updated Feb. 3, 2015, 10:52 a.m.)


Review request for hive and John Pullokkaran.


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


Repository: hive-git


Description
-------

HIVE-9444


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java 4d9d1da77394125cfcb9ac9ccf1c00528664b981 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/GlobalLimitOptimizer.java c9848dacd1a02db321583c2b91eb6d7317c295ff 

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


Testing
-------

Existing tests (global_limit.q).


Thanks,

Jesús Camacho Rodríguez


Re: Review Request 30254: HIVE-9444

Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30254/
-----------------------------------------------------------

(Updated Feb. 3, 2015, 10:46 a.m.)


Review request for hive and John Pullokkaran.


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


Repository: hive-git


Description
-------

HIVE-9444


Diffs
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/OperatorUtils.java 4d9d1da77394125cfcb9ac9ccf1c00528664b981 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/GlobalLimitOptimizer.java c9848dacd1a02db321583c2b91eb6d7317c295ff 

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


Testing (updated)
-------

Existing tests (global_limit.q).


Thanks,

Jesús Camacho Rodríguez