You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Deepak Jaiswal <dj...@hortonworks.com> on 2017/01/30 08:30:42 UTC
Review Request 56070: HIVE-15748
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56070/
-----------------------------------------------------------
Review request for hive and Jason Dere.
Bugs: HIVE-15748
https://issues.apache.org/jira/browse/HIVE-15748
Repository: hive-git
Description
-------
Remove cycles due to Mapjoin and semijoin combo
Diffs
-----
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java beed6b8
ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezUtils.java 6141391
ql/src/test/queries/clientpositive/dynamic_semijoin_reduction.q 13797c0
ql/src/test/results/clientpositive/llap/dynamic_semijoin_reduction.q.out e89526e
Diff: https://reviews.apache.org/r/56070/diff/
Testing
-------
Thanks,
Deepak Jaiswal
Re: Review Request 56070: HIVE-15748
Posted by Deepak Jaiswal <dj...@hortonworks.com>.
> On Jan. 31, 2017, 4:46 a.m., Jason Dere wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java, line 808
> > <https://reviews.apache.org/r/56070/diff/2/?file=1619635#file1619635line808>
> >
> > Can the parent be the result of a join in this case? If so there may be more than 1 TableScan Operator coming back from this.
Its a valid point. Will fix this in next patch.
- Deepak
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56070/#review163620
-----------------------------------------------------------
On Jan. 31, 2017, 12:44 a.m., Deepak Jaiswal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56070/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 12:44 a.m.)
>
>
> Review request for hive and Jason Dere.
>
>
> Bugs: HIVE-15748
> https://issues.apache.org/jira/browse/HIVE-15748
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Remove cycles due to Mapjoin and semijoin combo
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java beed6b8
> ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezUtils.java 6141391
> ql/src/test/queries/clientpositive/dynamic_semijoin_reduction.q 13797c0
> ql/src/test/results/clientpositive/llap/dynamic_semijoin_reduction.q.out e89526e
>
> Diff: https://reviews.apache.org/r/56070/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Deepak Jaiswal
>
>
Re: Review Request 56070: HIVE-15748
Posted by Jason Dere <jd...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56070/#review163620
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java (line 808)
<https://reviews.apache.org/r/56070/#comment235091>
Can the parent be the result of a join in this case? If so there may be more than 1 TableScan Operator coming back from this.
- Jason Dere
On Jan. 31, 2017, 12:44 a.m., Deepak Jaiswal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56070/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 12:44 a.m.)
>
>
> Review request for hive and Jason Dere.
>
>
> Bugs: HIVE-15748
> https://issues.apache.org/jira/browse/HIVE-15748
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Remove cycles due to Mapjoin and semijoin combo
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java beed6b8
> ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezUtils.java 6141391
> ql/src/test/queries/clientpositive/dynamic_semijoin_reduction.q 13797c0
> ql/src/test/results/clientpositive/llap/dynamic_semijoin_reduction.q.out e89526e
>
> Diff: https://reviews.apache.org/r/56070/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Deepak Jaiswal
>
>
Re: Review Request 56070: HIVE-15748
Posted by Deepak Jaiswal <dj...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56070/
-----------------------------------------------------------
(Updated Jan. 31, 2017, 11:30 p.m.)
Review request for hive and Jason Dere.
Changes
-------
Implemented review comments
Bugs: HIVE-15748
https://issues.apache.org/jira/browse/HIVE-15748
Repository: hive-git
Description
-------
Remove cycles due to Mapjoin and semijoin combo
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java beed6b8
ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezUtils.java 6141391
ql/src/test/queries/clientpositive/dynamic_semijoin_reduction.q 13797c0
ql/src/test/results/clientpositive/llap/dynamic_semijoin_reduction.q.out e89526e
Diff: https://reviews.apache.org/r/56070/diff/
Testing
-------
Thanks,
Deepak Jaiswal
Re: Review Request 56070: HIVE-15748
Posted by Deepak Jaiswal <dj...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56070/
-----------------------------------------------------------
(Updated Jan. 31, 2017, 12:44 a.m.)
Review request for hive and Jason Dere.
Changes
-------
Implemented the review comments.
Bugs: HIVE-15748
https://issues.apache.org/jira/browse/HIVE-15748
Repository: hive-git
Description
-------
Remove cycles due to Mapjoin and semijoin combo
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java beed6b8
ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezUtils.java 6141391
ql/src/test/queries/clientpositive/dynamic_semijoin_reduction.q 13797c0
ql/src/test/results/clientpositive/llap/dynamic_semijoin_reduction.q.out e89526e
Diff: https://reviews.apache.org/r/56070/diff/
Testing
-------
Thanks,
Deepak Jaiswal
Re: Review Request 56070: HIVE-15748
Posted by Deepak Jaiswal <dj...@hortonworks.com>.
> On Jan. 30, 2017, 10:27 p.m., Jason Dere wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java, line 33
> > <https://reviews.apache.org/r/56070/diff/1/?file=1618911#file1618911line33>
> >
> > importing * is not normally considered to be good form, try to avoid doing that if possible.
This was unintentional and done by the IDE, will fix this in both the files in next patch.
> On Jan. 30, 2017, 10:27 p.m., Jason Dere wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java, line 785
> > <https://reviews.apache.org/r/56070/diff/1/?file=1618911#file1618911line785>
> >
> > Not sure if the parent of the ReduceSink parent is necesaarily always a SELECT operator. Might want to try some tests with select * or selecting all columns to verify, since the SelectOperator might be omitted if there is no need for projection.
In such case, there wont be any semijoin branch except when a mapjoin is followed by mapjoin which is handled separately.
> On Jan. 30, 2017, 10:27 p.m., Jason Dere wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java, line 804
> > <https://reviews.apache.org/r/56070/diff/1/?file=1618911#file1618911line804>
> >
> > Maybe use OperatorUtils.findOperatorsUpstream()
Thanks, will do this in next patch.
> On Jan. 30, 2017, 10:27 p.m., Jason Dere wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezUtils.java, line 571
> > <https://reviews.apache.org/r/56070/diff/1/?file=1618912#file1618912line571>
> >
> > Clean this up
Thanks for pointing this out.
- Deepak
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56070/#review163574
-----------------------------------------------------------
On Jan. 30, 2017, 8:30 a.m., Deepak Jaiswal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56070/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2017, 8:30 a.m.)
>
>
> Review request for hive and Jason Dere.
>
>
> Bugs: HIVE-15748
> https://issues.apache.org/jira/browse/HIVE-15748
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Remove cycles due to Mapjoin and semijoin combo
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java beed6b8
> ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezUtils.java 6141391
> ql/src/test/queries/clientpositive/dynamic_semijoin_reduction.q 13797c0
> ql/src/test/results/clientpositive/llap/dynamic_semijoin_reduction.q.out e89526e
>
> Diff: https://reviews.apache.org/r/56070/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Deepak Jaiswal
>
>
Re: Review Request 56070: HIVE-15748
Posted by Jason Dere <jd...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56070/#review163574
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java (line 33)
<https://reviews.apache.org/r/56070/#comment235027>
importing * is not normally considered to be good form, try to avoid doing that if possible.
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java (line 772)
<https://reviews.apache.org/r/56070/#comment235028>
Not sure if the parent of the ReduceSink parent is necesaarily always a SELECT operator. Might want to try some tests with select * or selecting all columns to verify, since the SelectOperator might be omitted if there is no need for projection.
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java (line 791)
<https://reviews.apache.org/r/56070/#comment235033>
Maybe use OperatorUtils.findOperatorsUpstream()
ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezUtils.java (line 560)
<https://reviews.apache.org/r/56070/#comment235035>
Clean this up
- Jason Dere
On Jan. 30, 2017, 8:30 a.m., Deepak Jaiswal wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56070/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2017, 8:30 a.m.)
>
>
> Review request for hive and Jason Dere.
>
>
> Bugs: HIVE-15748
> https://issues.apache.org/jira/browse/HIVE-15748
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Remove cycles due to Mapjoin and semijoin combo
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java beed6b8
> ql/src/java/org/apache/hadoop/hive/ql/parse/GenTezUtils.java 6141391
> ql/src/test/queries/clientpositive/dynamic_semijoin_reduction.q 13797c0
> ql/src/test/results/clientpositive/llap/dynamic_semijoin_reduction.q.out e89526e
>
> Diff: https://reviews.apache.org/r/56070/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Deepak Jaiswal
>
>