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/05/02 05:47:32 UTC

Review Request 58914: HIVE-16550

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

Review request for hive, Jason Dere and Sergey Shelukhin.


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


Repository: hive-git


Description
-------

Semijoin Hints should be able to skip the optimization if needed.
In addition to that, the patch fixes several issues with hints in general such as,
- It now works with subqueries and unions.
- Uses a global data structure instead of per QB.


Diffs
-----

  ql/src/java/org/apache/hadoop/hive/ql/optimizer/DynamicPartitionPruningOptimization.java e1a69526bc 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveOpConverter.java d375d1b58d 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 1b054a7e24 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HintParser.g e110fb33df 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 3a1f821bd3 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 54e37f7c80 
  ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java 7bf1c599a5 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 654f3b1772 
  ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 5ea7800528 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDynamicListDesc.java 3143554ec6 
  ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java 032c7bb28d 
  ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java f45daa8828 
  ql/src/test/queries/clientpositive/semijoin_hint.q 5de0c8c8c1 
  ql/src/test/results/clientpositive/llap/semijoin_hint.q.out bc248930ec 


Diff: https://reviews.apache.org/r/58914/diff/1/


Testing
-------


Thanks,

Deepak Jaiswal


Re: Review Request 58914: HIVE-16550

Posted by Deepak Jaiswal <dj...@hortonworks.com>.

> On May 2, 2017, 6:35 p.m., Vineet Garg wrote:
> > ql/src/test/queries/clientpositive/semijoin_hint.q
> > Line 38 (original), 38 (patched)
> > <https://reviews.apache.org/r/58914/diff/1/?file=1705394#file1705394line38>
> >
> >     Why is returnpath on?
> >     This feature is not yet fully developed/supported and is off by default. 
> >     You might want to try the same queries with this feature off.
> 
> Deepak Jaiswal wrote:
>     We have to make sure we dont break it.
>     There are tests below with the feature turned off.
> 
> Vineet Garg wrote:
>     Ok in that case can you add the same tests with returnpath off? I don't see union all query with returnpath off

sure.


- Deepak


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


On May 2, 2017, 5:47 a.m., Deepak Jaiswal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58914/
> -----------------------------------------------------------
> 
> (Updated May 2, 2017, 5:47 a.m.)
> 
> 
> Review request for hive, Jason Dere and Sergey Shelukhin.
> 
> 
> Bugs: HIVE-16550
>     https://issues.apache.org/jira/browse/HIVE-16550
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Semijoin Hints should be able to skip the optimization if needed.
> In addition to that, the patch fixes several issues with hints in general such as,
> - It now works with subqueries and unions.
> - Uses a global data structure instead of per QB.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/DynamicPartitionPruningOptimization.java e1a69526bc 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveOpConverter.java d375d1b58d 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 1b054a7e24 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HintParser.g e110fb33df 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 3a1f821bd3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 54e37f7c80 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java 7bf1c599a5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 654f3b1772 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 5ea7800528 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDynamicListDesc.java 3143554ec6 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java 032c7bb28d 
>   ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java f45daa8828 
>   ql/src/test/queries/clientpositive/semijoin_hint.q 5de0c8c8c1 
>   ql/src/test/results/clientpositive/llap/semijoin_hint.q.out bc248930ec 
> 
> 
> Diff: https://reviews.apache.org/r/58914/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Deepak Jaiswal
> 
>


Re: Review Request 58914: HIVE-16550

Posted by Vineet Garg <vg...@hortonworks.com>.

> On May 2, 2017, 6:35 p.m., Vineet Garg wrote:
> > ql/src/test/queries/clientpositive/semijoin_hint.q
> > Line 38 (original), 38 (patched)
> > <https://reviews.apache.org/r/58914/diff/1/?file=1705394#file1705394line38>
> >
> >     Why is returnpath on?
> >     This feature is not yet fully developed/supported and is off by default. 
> >     You might want to try the same queries with this feature off.
> 
> Deepak Jaiswal wrote:
>     We have to make sure we dont break it.
>     There are tests below with the feature turned off.

Ok in that case can you add the same tests with returnpath off? I don't see union all query with returnpath off


- Vineet


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


On May 2, 2017, 5:47 a.m., Deepak Jaiswal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58914/
> -----------------------------------------------------------
> 
> (Updated May 2, 2017, 5:47 a.m.)
> 
> 
> Review request for hive, Jason Dere and Sergey Shelukhin.
> 
> 
> Bugs: HIVE-16550
>     https://issues.apache.org/jira/browse/HIVE-16550
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Semijoin Hints should be able to skip the optimization if needed.
> In addition to that, the patch fixes several issues with hints in general such as,
> - It now works with subqueries and unions.
> - Uses a global data structure instead of per QB.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/DynamicPartitionPruningOptimization.java e1a69526bc 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveOpConverter.java d375d1b58d 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 1b054a7e24 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HintParser.g e110fb33df 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 3a1f821bd3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 54e37f7c80 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java 7bf1c599a5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 654f3b1772 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 5ea7800528 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDynamicListDesc.java 3143554ec6 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java 032c7bb28d 
>   ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java f45daa8828 
>   ql/src/test/queries/clientpositive/semijoin_hint.q 5de0c8c8c1 
>   ql/src/test/results/clientpositive/llap/semijoin_hint.q.out bc248930ec 
> 
> 
> Diff: https://reviews.apache.org/r/58914/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Deepak Jaiswal
> 
>


Re: Review Request 58914: HIVE-16550

Posted by Deepak Jaiswal <dj...@hortonworks.com>.

> On May 2, 2017, 6:35 p.m., Vineet Garg wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
> > Line 338 (original), 338 (patched)
> > <https://reviews.apache.org/r/58914/diff/1/?file=1705384#file1705384line338>
> >
> >     Can you add comments to explain why are we doing this i.e. why are we propagating hints?

Will do it thanks.


> On May 2, 2017, 6:35 p.m., Vineet Garg wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
> > Lines 368 (patched)
> > <https://reviews.apache.org/r/58914/diff/1/?file=1705384#file1705384line368>
> >
> >     If I understand it correctly before generating calcite plan (i.e. calling getOptimizedAST) QB has hints but after generating optimized AST QB is reset and lose hints? That is why you are propagating hints?

Yes.


> On May 2, 2017, 6:35 p.m., Vineet Garg wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
> > Line 408 (original), 413 (patched)
> > <https://reviews.apache.org/r/58914/diff/1/?file=1705384#file1705384line413>
> >
> >     What happens in this case? Why is it necessary to log warning here?

I think just paranoia. It shouldn't happen, however, if it does, we log it.


> On May 2, 2017, 6:35 p.m., Vineet Garg wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java
> > Line 444 (original), 444 (patched)
> > <https://reviews.apache.org/r/58914/diff/1/?file=1705387#file1705387line444>
> >
> >     Is this because we can have hint now? If so can you rather check for hint and continue instead of continuing for all nodes

Makes sense. Will do.


> On May 2, 2017, 6:35 p.m., Vineet Garg wrote:
> > ql/src/test/queries/clientpositive/semijoin_hint.q
> > Line 38 (original), 38 (patched)
> > <https://reviews.apache.org/r/58914/diff/1/?file=1705394#file1705394line38>
> >
> >     Why is returnpath on?
> >     This feature is not yet fully developed/supported and is off by default. 
> >     You might want to try the same queries with this feature off.

We have to make sure we dont break it.
There are tests below with the feature turned off.


- Deepak


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


On May 2, 2017, 5:47 a.m., Deepak Jaiswal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58914/
> -----------------------------------------------------------
> 
> (Updated May 2, 2017, 5:47 a.m.)
> 
> 
> Review request for hive, Jason Dere and Sergey Shelukhin.
> 
> 
> Bugs: HIVE-16550
>     https://issues.apache.org/jira/browse/HIVE-16550
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Semijoin Hints should be able to skip the optimization if needed.
> In addition to that, the patch fixes several issues with hints in general such as,
> - It now works with subqueries and unions.
> - Uses a global data structure instead of per QB.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/DynamicPartitionPruningOptimization.java e1a69526bc 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveOpConverter.java d375d1b58d 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 1b054a7e24 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HintParser.g e110fb33df 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 3a1f821bd3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 54e37f7c80 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java 7bf1c599a5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 654f3b1772 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 5ea7800528 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDynamicListDesc.java 3143554ec6 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java 032c7bb28d 
>   ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java f45daa8828 
>   ql/src/test/queries/clientpositive/semijoin_hint.q 5de0c8c8c1 
>   ql/src/test/results/clientpositive/llap/semijoin_hint.q.out bc248930ec 
> 
> 
> Diff: https://reviews.apache.org/r/58914/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Deepak Jaiswal
> 
>


Re: Review Request 58914: HIVE-16550

Posted by Vineet Garg <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58914/#review173558
-----------------------------------------------------------




ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
Line 338 (original), 338 (patched)
<https://reviews.apache.org/r/58914/#comment246547>

    Can you add comments to explain why are we doing this i.e. why are we propagating hints?



ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
Lines 368 (patched)
<https://reviews.apache.org/r/58914/#comment246614>

    If I understand it correctly before generating calcite plan (i.e. calling getOptimizedAST) QB has hints but after generating optimized AST QB is reset and lose hints? That is why you are propagating hints?



ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
Line 408 (original), 413 (patched)
<https://reviews.apache.org/r/58914/#comment246551>

    What happens in this case? Why is it necessary to log warning here?



ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java
Line 444 (original), 444 (patched)
<https://reviews.apache.org/r/58914/#comment246553>

    Is this because we can have hint now? If so can you rather check for hint and continue instead of continuing for all nodes



ql/src/test/queries/clientpositive/semijoin_hint.q
Line 38 (original), 38 (patched)
<https://reviews.apache.org/r/58914/#comment246546>

    Why is returnpath on?
    This feature is not yet fully developed/supported and is off by default. 
    You might want to try the same queries with this feature off.


- Vineet Garg


On May 2, 2017, 5:47 a.m., Deepak Jaiswal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58914/
> -----------------------------------------------------------
> 
> (Updated May 2, 2017, 5:47 a.m.)
> 
> 
> Review request for hive, Jason Dere and Sergey Shelukhin.
> 
> 
> Bugs: HIVE-16550
>     https://issues.apache.org/jira/browse/HIVE-16550
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Semijoin Hints should be able to skip the optimization if needed.
> In addition to that, the patch fixes several issues with hints in general such as,
> - It now works with subqueries and unions.
> - Uses a global data structure instead of per QB.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/DynamicPartitionPruningOptimization.java e1a69526bc 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveOpConverter.java d375d1b58d 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 1b054a7e24 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HintParser.g e110fb33df 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 3a1f821bd3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 54e37f7c80 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java 7bf1c599a5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 654f3b1772 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 5ea7800528 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDynamicListDesc.java 3143554ec6 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java 032c7bb28d 
>   ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java f45daa8828 
>   ql/src/test/queries/clientpositive/semijoin_hint.q 5de0c8c8c1 
>   ql/src/test/results/clientpositive/llap/semijoin_hint.q.out bc248930ec 
> 
> 
> Diff: https://reviews.apache.org/r/58914/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Deepak Jaiswal
> 
>


Re: Review Request 58914: HIVE-16550

Posted by Deepak Jaiswal <dj...@hortonworks.com>.

> On May 2, 2017, 8:15 p.m., Jason Dere wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
> > Lines 11192 (patched)
> > <https://reviews.apache.org/r/58914/diff/1/?file=1705389#file1705389line11200>
> >
> >     Can you give the method name something different than OldHints - how about getHintsFromQB?

sure.


> On May 2, 2017, 8:15 p.m., Jason Dere wrote:
> > ql/src/test/queries/clientpositive/semijoin_hint.q
> > Lines 46 (patched)
> > <https://reviews.apache.org/r/58914/diff/1/?file=1705394#file1705394line46>
> >
> >     Does this query, with no hints, have any runtime filtering? Otherwise you wouldn't be able to distinguish whether the None option works or not.

Yes it does.


> On May 2, 2017, 8:15 p.m., Jason Dere wrote:
> > ql/src/test/results/clientpositive/llap/semijoin_hint.q.out
> > Lines 233 (patched)
> > <https://reviews.apache.org/r/58914/diff/1/?file=1705395#file1705395line233>
> >
> >     Should the expectedEntries be 5000 per the hint?

Thanks for brining this up. It is happening incase of return path true. Investigating.


> On May 2, 2017, 8:15 p.m., Jason Dere wrote:
> > ql/src/test/results/clientpositive/llap/semijoin_hint.q.out
> > Lines 266 (patched)
> > <https://reviews.apache.org/r/58914/diff/1/?file=1705395#file1705395line266>
> >
> >     Should the expectedEntries be 5000 per the hint?

Yes, looking into it.


> On May 2, 2017, 8:15 p.m., Jason Dere wrote:
> > ql/src/test/results/clientpositive/llap/semijoin_hint.q.out
> > Lines 300 (patched)
> > <https://reviews.apache.org/r/58914/diff/1/?file=1705395#file1705395line300>
> >
> >     Should the expectedEntries be 5000 per the hint?

ditto


- Deepak


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


On May 2, 2017, 5:47 a.m., Deepak Jaiswal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58914/
> -----------------------------------------------------------
> 
> (Updated May 2, 2017, 5:47 a.m.)
> 
> 
> Review request for hive, Jason Dere and Sergey Shelukhin.
> 
> 
> Bugs: HIVE-16550
>     https://issues.apache.org/jira/browse/HIVE-16550
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Semijoin Hints should be able to skip the optimization if needed.
> In addition to that, the patch fixes several issues with hints in general such as,
> - It now works with subqueries and unions.
> - Uses a global data structure instead of per QB.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/DynamicPartitionPruningOptimization.java e1a69526bc 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveOpConverter.java d375d1b58d 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 1b054a7e24 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HintParser.g e110fb33df 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 3a1f821bd3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 54e37f7c80 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java 7bf1c599a5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 654f3b1772 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 5ea7800528 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDynamicListDesc.java 3143554ec6 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java 032c7bb28d 
>   ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java f45daa8828 
>   ql/src/test/queries/clientpositive/semijoin_hint.q 5de0c8c8c1 
>   ql/src/test/results/clientpositive/llap/semijoin_hint.q.out bc248930ec 
> 
> 
> Diff: https://reviews.apache.org/r/58914/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Deepak Jaiswal
> 
>


Re: Review Request 58914: HIVE-16550

Posted by Jason Dere <jd...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58914/#review173628
-----------------------------------------------------------




ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
Lines 11192 (patched)
<https://reviews.apache.org/r/58914/#comment246628>

    Can you give the method name something different than OldHints - how about getHintsFromQB?



ql/src/test/queries/clientpositive/semijoin_hint.q
Lines 46 (patched)
<https://reviews.apache.org/r/58914/#comment246638>

    Does this query, with no hints, have any runtime filtering? Otherwise you wouldn't be able to distinguish whether the None option works or not.



ql/src/test/results/clientpositive/llap/semijoin_hint.q.out
Lines 233 (patched)
<https://reviews.apache.org/r/58914/#comment246641>

    Should the expectedEntries be 5000 per the hint?



ql/src/test/results/clientpositive/llap/semijoin_hint.q.out
Lines 266 (patched)
<https://reviews.apache.org/r/58914/#comment246642>

    Should the expectedEntries be 5000 per the hint?



ql/src/test/results/clientpositive/llap/semijoin_hint.q.out
Lines 300 (patched)
<https://reviews.apache.org/r/58914/#comment246643>

    Should the expectedEntries be 5000 per the hint?


- Jason Dere


On May 2, 2017, 5:47 a.m., Deepak Jaiswal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58914/
> -----------------------------------------------------------
> 
> (Updated May 2, 2017, 5:47 a.m.)
> 
> 
> Review request for hive, Jason Dere and Sergey Shelukhin.
> 
> 
> Bugs: HIVE-16550
>     https://issues.apache.org/jira/browse/HIVE-16550
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Semijoin Hints should be able to skip the optimization if needed.
> In addition to that, the patch fixes several issues with hints in general such as,
> - It now works with subqueries and unions.
> - Uses a global data structure instead of per QB.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/DynamicPartitionPruningOptimization.java e1a69526bc 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveOpConverter.java d375d1b58d 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 1b054a7e24 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HintParser.g e110fb33df 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 3a1f821bd3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 54e37f7c80 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java 7bf1c599a5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 654f3b1772 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 5ea7800528 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDynamicListDesc.java 3143554ec6 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java 032c7bb28d 
>   ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java f45daa8828 
>   ql/src/test/queries/clientpositive/semijoin_hint.q 5de0c8c8c1 
>   ql/src/test/results/clientpositive/llap/semijoin_hint.q.out bc248930ec 
> 
> 
> Diff: https://reviews.apache.org/r/58914/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Deepak Jaiswal
> 
>


Re: Review Request 58914: HIVE-16550

Posted by Deepak Jaiswal <dj...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58914/
-----------------------------------------------------------

(Updated May 3, 2017, 1:16 a.m.)


Review request for hive, Jason Dere and Sergey Shelukhin.


Changes
-------

Worked on Jason's comments and added some more tests.


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


Repository: hive-git


Description
-------

Semijoin Hints should be able to skip the optimization if needed.
In addition to that, the patch fixes several issues with hints in general such as,
- It now works with subqueries and unions.
- Uses a global data structure instead of per QB.


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/optimizer/DynamicPartitionPruningOptimization.java e1a69526bc 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveOpConverter.java 40c0f3ba2a 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 1b054a7e24 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HintParser.g e110fb33df 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 3a1f821bd3 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 54e37f7c80 
  ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java 7bf1c599a5 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java cbbb7d0c94 
  ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 5ea7800528 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDynamicListDesc.java 3143554ec6 
  ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java 7d4267d6a8 
  ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java f45daa8828 
  ql/src/test/queries/clientpositive/semijoin_hint.q 5de0c8c8c1 
  ql/src/test/results/clientpositive/llap/semijoin_hint.q.out bc248930ec 


Diff: https://reviews.apache.org/r/58914/diff/3/

Changes: https://reviews.apache.org/r/58914/diff/2-3/


Testing
-------


Thanks,

Deepak Jaiswal


Re: Review Request 58914: HIVE-16550

Posted by Deepak Jaiswal <dj...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58914/
-----------------------------------------------------------

(Updated May 2, 2017, 9:01 p.m.)


Review request for hive, Jason Dere and Sergey Shelukhin.


Changes
-------

Implemented review comments.
Added the special case for "None" to skip the runtime filtering.


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


Repository: hive-git


Description
-------

Semijoin Hints should be able to skip the optimization if needed.
In addition to that, the patch fixes several issues with hints in general such as,
- It now works with subqueries and unions.
- Uses a global data structure instead of per QB.


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/optimizer/DynamicPartitionPruningOptimization.java e1a69526bc 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveOpConverter.java 40c0f3ba2a 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 1b054a7e24 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HintParser.g e110fb33df 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 3a1f821bd3 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 54e37f7c80 
  ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java 7bf1c599a5 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java cbbb7d0c94 
  ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 5ea7800528 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDynamicListDesc.java 3143554ec6 
  ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java 7d4267d6a8 
  ql/src/java/org/apache/hadoop/hive/ql/ppd/SyntheticJoinPredicate.java f45daa8828 
  ql/src/test/queries/clientpositive/semijoin_hint.q 5de0c8c8c1 
  ql/src/test/results/clientpositive/llap/semijoin_hint.q.out bc248930ec 


Diff: https://reviews.apache.org/r/58914/diff/2/

Changes: https://reviews.apache.org/r/58914/diff/1-2/


Testing
-------


Thanks,

Deepak Jaiswal