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 2017/10/26 21:12:49 UTC

Review Request 63343: HIVE-17766

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

Review request for hive, Ashutosh Chauhan and Vineet Garg.


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


Repository: hive-git


Description
-------

HIVE-17766


Diffs
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 3573d07e9a3bab4dabdc0b73292bd02ad05cbc89 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 46493ac006d00f2e6c0054da2438c0b01c1e3fd3 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5ccb69ac4824a2002b2bddb1ad206cb7acd3ace3 
  ql/src/test/queries/clientpositive/semijoin6.q PRE-CREATION 
  ql/src/test/results/clientpositive/semijoin2.q.out 2c3c5d8db3d5422f21fde020199e878b87b0507f 
  ql/src/test/results/clientpositive/semijoin6.q.out PRE-CREATION 


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


Testing
-------


Thanks,

Jesús Camacho Rodríguez


Re: Review Request 63343: HIVE-17766

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

> On Oct. 30, 2017, 3:19 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
> > Lines 2829-2837 (original), 2829-2837 (patched)
> > <https://reviews.apache.org/r/63343/diff/5/?file=1872020#file1872020line2829>
> >
> >     This comments need to be updated. Way outdated.

I have updated this comment.


> On Oct. 30, 2017, 3:19 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
> > Line 2845 (original), 2845 (patched)
> > <https://reviews.apache.org/r/63343/diff/5/?file=1872020#file1872020line2845>
> >
> >     Seems like this doesn't throw anymore. Can remove SemanticException.

It does, actually _parseJoinCondPopulateAlias_ throws it.


> On Oct. 30, 2017, 3:19 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
> > Lines 8278 (patched)
> > <https://reviews.apache.org/r/63343/diff/5/?file=1872020#file1872020line8278>
> >
> >     Whats the relation b/w this and insertSelectForSemijoin() ? Do we need both

Yes, this is independent. The _insertSelectForSemijoin_ method will introduce a Project *below* the right input of the join to select only the columns from the right input that are needed for join processing, i.e., that are needed by the semijoin condition.
In turn, this block introduces a Project *above* the semijoin operator to remove all the columns from the right hand side. This is only needed when there are residual conditions to evaluate by the semijoin, since it needs to see all the input columns to evaluate the condition (otherwise, the semijoin itself prunes the columns coming from that input).


> On Oct. 30, 2017, 3:19 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
> > Lines 8545 (patched)
> > <https://reviews.apache.org/r/63343/diff/5/?file=1872020#file1872020line8547>
> >
> >     Can't expr be of type different than ExprNodeColumnDesc or ExprNodeConstantDesc? If not, need to throw in those cases.

Thanks, that was a good catch! As you mentioned, although we can only generate from column references, we might generate another type of expressions, e.g., field expressions for nested fields. Since this is an additional optimization, I have changed the code so we just return the input operator in case we find an expression that we cannot recognize, without introducing the Select+GroupBy chain. I added an additional test for nested fields.


> On Oct. 30, 2017, 3:19 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
> > Lines 8594 (patched)
> > <https://reviews.apache.org/r/63343/diff/5/?file=1872020#file1872020line8607>
> >
> >     other exprNode types?

Same as above.


> On Oct. 30, 2017, 3:19 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/queries/clientpositive/semijoin6.q
> > Lines 18 (patched)
> > <https://reviews.apache.org/r/63343/diff/5/?file=1872022#file1872022line18>
> >
> >     If left semi join supports null-safe join, can you also add test for: 
> >     select * from tx1 u left semi join tx2 v on u.b <=> v.b;

I just added it.


- Jesús


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


On Oct. 28, 2017, 8:06 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63343/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2017, 8:06 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Vineet Garg.
> 
> 
> Bugs: HIVE-17766
>     https://issues.apache.org/jira/browse/HIVE-17766
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-17766
> 
> 
> Diffs
> -----
> 
>   itests/src/test/resources/testconfiguration.properties a4648beae1ec18c88bab35ae3e7f9d9f202e40e5 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 3573d07e9a3bab4dabdc0b73292bd02ad05cbc89 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 46493ac006d00f2e6c0054da2438c0b01c1e3fd3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5ccb69ac4824a2002b2bddb1ad206cb7acd3ace3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 4ddf1d5b5e87a43f7f606ce3c09ac295f5319333 
>   ql/src/test/queries/clientpositive/semijoin6.q PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/semijoin6.q.out PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63343/diff/5/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 63343: HIVE-17766

Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63343/#review189588
-----------------------------------------------------------




ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
Lines 2829-2837 (original), 2829-2837 (patched)
<https://reviews.apache.org/r/63343/#comment266734>

    This comments need to be updated. Way outdated.



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
Line 2845 (original), 2845 (patched)
<https://reviews.apache.org/r/63343/#comment266735>

    Seems like this doesn't throw anymore. Can remove SemanticException.



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
Lines 2857-2862 (patched)
<https://reviews.apache.org/r/63343/#comment266736>

    How come supporting left-semi join resulted in this needing change? How does this impact other join types.



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
Lines 8278 (patched)
<https://reviews.apache.org/r/63343/#comment266739>

    Whats the relation b/w this and insertSelectForSemijoin() ? Do we need both



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
Lines 8545 (patched)
<https://reviews.apache.org/r/63343/#comment266737>

    Can't expr be of type different than ExprNodeColumnDesc or ExprNodeConstantDesc? If not, need to throw in those cases.



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
Lines 8594 (patched)
<https://reviews.apache.org/r/63343/#comment266738>

    other exprNode types?



ql/src/test/queries/clientpositive/semijoin6.q
Lines 18 (patched)
<https://reviews.apache.org/r/63343/#comment266740>

    If left semi join supports null-safe join, can you also add test for: 
    select * from tx1 u left semi join tx2 v on u.b <=> v.b;


- Ashutosh Chauhan


On Oct. 28, 2017, 8:06 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63343/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2017, 8:06 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Vineet Garg.
> 
> 
> Bugs: HIVE-17766
>     https://issues.apache.org/jira/browse/HIVE-17766
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-17766
> 
> 
> Diffs
> -----
> 
>   itests/src/test/resources/testconfiguration.properties a4648beae1ec18c88bab35ae3e7f9d9f202e40e5 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 3573d07e9a3bab4dabdc0b73292bd02ad05cbc89 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 46493ac006d00f2e6c0054da2438c0b01c1e3fd3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5ccb69ac4824a2002b2bddb1ad206cb7acd3ace3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 4ddf1d5b5e87a43f7f606ce3c09ac295f5319333 
>   ql/src/test/queries/clientpositive/semijoin6.q PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/semijoin6.q.out PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63343/diff/5/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 63343: HIVE-17766

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/63343/
-----------------------------------------------------------

(Updated Oct. 30, 2017, 6:50 p.m.)


Review request for hive, Ashutosh Chauhan and Vineet Garg.


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


Repository: hive-git


Description
-------

HIVE-17766


Diffs (updated)
-----

  itests/src/test/resources/testconfiguration.properties 5ff2de35b4784c8ce04191097ab1f8847218edab 
  ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 3573d07e9a3bab4dabdc0b73292bd02ad05cbc89 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 46493ac006d00f2e6c0054da2438c0b01c1e3fd3 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 0a9e6fe21d8ca6bb8fd9650add1c1aace892af5e 
  ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 4ddf1d5b5e87a43f7f606ce3c09ac295f5319333 
  ql/src/test/queries/clientpositive/nested_column_pruning.q 80a791ac1b6ea3652bef0d5d8af619869a0c30cb 
  ql/src/test/queries/clientpositive/semijoin6.q PRE-CREATION 
  ql/src/test/queries/clientpositive/semijoin7.q PRE-CREATION 
  ql/src/test/results/clientpositive/llap/semijoin6.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/semijoin7.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/nested_column_pruning.q.out 5fe83ba390b8f72ae35e324c51250bbef288403c 


Diff: https://reviews.apache.org/r/63343/diff/6/

Changes: https://reviews.apache.org/r/63343/diff/5-6/


Testing
-------


Thanks,

Jesús Camacho Rodríguez


Re: Review Request 63343: HIVE-17766

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/63343/
-----------------------------------------------------------

(Updated Oct. 28, 2017, 8:06 p.m.)


Review request for hive, Ashutosh Chauhan and Vineet Garg.


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


Repository: hive-git


Description
-------

HIVE-17766


Diffs (updated)
-----

  itests/src/test/resources/testconfiguration.properties a4648beae1ec18c88bab35ae3e7f9d9f202e40e5 
  ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 3573d07e9a3bab4dabdc0b73292bd02ad05cbc89 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 46493ac006d00f2e6c0054da2438c0b01c1e3fd3 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5ccb69ac4824a2002b2bddb1ad206cb7acd3ace3 
  ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 4ddf1d5b5e87a43f7f606ce3c09ac295f5319333 
  ql/src/test/queries/clientpositive/semijoin6.q PRE-CREATION 
  ql/src/test/results/clientpositive/llap/semijoin6.q.out PRE-CREATION 


Diff: https://reviews.apache.org/r/63343/diff/5/

Changes: https://reviews.apache.org/r/63343/diff/4-5/


Testing
-------


Thanks,

Jesús Camacho Rodríguez


Re: Review Request 63343: HIVE-17766

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/63343/
-----------------------------------------------------------

(Updated Oct. 28, 2017, 12:16 a.m.)


Review request for hive, Ashutosh Chauhan and Vineet Garg.


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


Repository: hive-git


Description
-------

HIVE-17766


Diffs (updated)
-----

  itests/src/test/resources/testconfiguration.properties a4648beae1ec18c88bab35ae3e7f9d9f202e40e5 
  ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 3573d07e9a3bab4dabdc0b73292bd02ad05cbc89 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 46493ac006d00f2e6c0054da2438c0b01c1e3fd3 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5ccb69ac4824a2002b2bddb1ad206cb7acd3ace3 
  ql/src/test/queries/clientpositive/semijoin6.q PRE-CREATION 
  ql/src/test/results/clientpositive/llap/semijoin6.q.out PRE-CREATION 


Diff: https://reviews.apache.org/r/63343/diff/4/

Changes: https://reviews.apache.org/r/63343/diff/3-4/


Testing
-------


Thanks,

Jesús Camacho Rodríguez


Re: Review Request 63343: HIVE-17766

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/63343/
-----------------------------------------------------------

(Updated Oct. 27, 2017, 1:01 a.m.)


Review request for hive, Ashutosh Chauhan and Vineet Garg.


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


Repository: hive-git


Description
-------

HIVE-17766


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 3573d07e9a3bab4dabdc0b73292bd02ad05cbc89 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 46493ac006d00f2e6c0054da2438c0b01c1e3fd3 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5ccb69ac4824a2002b2bddb1ad206cb7acd3ace3 
  ql/src/test/queries/clientpositive/semijoin6.q PRE-CREATION 
  ql/src/test/results/clientpositive/semijoin2.q.out 2c3c5d8db3d5422f21fde020199e878b87b0507f 
  ql/src/test/results/clientpositive/semijoin6.q.out PRE-CREATION 


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

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


Testing
-------


Thanks,

Jesús Camacho Rodríguez


Re: Review Request 63343: HIVE-17766

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

> On Oct. 26, 2017, 11:08 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
> > Lines 2136-2137 (original), 2119-2120 (patched)
> > <https://reviews.apache.org/r/63343/diff/2/?file=1869813#file1869813line2136>
> >
> >     Shall we try to get equi joins keys and populate them in operator here? Will it happen later in planning.
> >     Either way good to add a comment about this.

This is in Calcite operators, not in Hive. The reason keys are there is because semijoin extends equijoin. I do not know if they are effectively used, the logic here was pushing the computation of expressions to children project, but that seems to be happening when we trigger rewriting rules in any case, thus I do not think we need to do it here (maybe this code was written before some of the rules that take care of this were enabled in Hive?).


- Jesús


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


On Oct. 26, 2017, 9:26 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63343/
> -----------------------------------------------------------
> 
> (Updated Oct. 26, 2017, 9:26 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Vineet Garg.
> 
> 
> Bugs: HIVE-17766
>     https://issues.apache.org/jira/browse/HIVE-17766
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-17766
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 3573d07e9a3bab4dabdc0b73292bd02ad05cbc89 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 46493ac006d00f2e6c0054da2438c0b01c1e3fd3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5ccb69ac4824a2002b2bddb1ad206cb7acd3ace3 
>   ql/src/test/queries/clientpositive/semijoin6.q PRE-CREATION 
>   ql/src/test/results/clientpositive/semijoin2.q.out 2c3c5d8db3d5422f21fde020199e878b87b0507f 
>   ql/src/test/results/clientpositive/semijoin6.q.out PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63343/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 63343: HIVE-17766

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

> On Oct. 26, 2017, 11:08 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
> > Lines 2136-2137 (original), 2119-2120 (patched)
> > <https://reviews.apache.org/r/63343/diff/2/?file=1869813#file1869813line2136>
> >
> >     Shall we try to get equi joins keys and populate them in operator here? Will it happen later in planning.
> >     Either way good to add a comment about this.
> 
> Jesús Camacho Rodríguez wrote:
>     This is in Calcite operators, not in Hive. The reason keys are there is because semijoin extends equijoin. I do not know if they are effectively used, the logic here was pushing the computation of expressions to children project, but that seems to be happening when we trigger rewriting rules in any case, thus I do not think we need to do it here (maybe this code was written before some of the rules that take care of this were enabled in Hive?).

I discovered some tests in previous iteration of the patch where filters that were applying to only one input where not pushed below the semijoin.
I have fixed this logic to push whatever we can push below the semijoin and create the semijoin condition with the rest.


- Jesús


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


On Oct. 28, 2017, 8:06 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63343/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2017, 8:06 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Vineet Garg.
> 
> 
> Bugs: HIVE-17766
>     https://issues.apache.org/jira/browse/HIVE-17766
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-17766
> 
> 
> Diffs
> -----
> 
>   itests/src/test/resources/testconfiguration.properties a4648beae1ec18c88bab35ae3e7f9d9f202e40e5 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 3573d07e9a3bab4dabdc0b73292bd02ad05cbc89 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 46493ac006d00f2e6c0054da2438c0b01c1e3fd3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5ccb69ac4824a2002b2bddb1ad206cb7acd3ace3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 4ddf1d5b5e87a43f7f606ce3c09ac295f5319333 
>   ql/src/test/queries/clientpositive/semijoin6.q PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/semijoin6.q.out PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63343/diff/5/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 63343: HIVE-17766

Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63343/#review189370
-----------------------------------------------------------




ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
Lines 2136-2137 (original), 2119-2120 (patched)
<https://reviews.apache.org/r/63343/#comment266407>

    Shall we try to get equi joins keys and populate them in operator here? Will it happen later in planning.
    Either way good to add a comment about this.



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
Line 8257 (original), 8268 (patched)
<https://reviews.apache.org/r/63343/#comment266408>

    comment: Adding a select operator to top of join to ensure projection of only left side columns.



ql/src/test/queries/clientpositive/semijoin6.q
Lines 10 (patched)
<https://reviews.apache.org/r/63343/#comment266421>

    No need for this test.



ql/src/test/queries/clientpositive/semijoin6.q
Lines 14 (patched)
<https://reviews.apache.org/r/63343/#comment266417>

    Please add explain for this query.



ql/src/test/queries/clientpositive/semijoin6.q
Lines 16 (patched)
<https://reviews.apache.org/r/63343/#comment266415>

    Please add explain for this query.



ql/src/test/queries/clientpositive/semijoin6.q
Lines 18 (patched)
<https://reviews.apache.org/r/63343/#comment266412>

    Please add explain for this query.



ql/src/test/queries/clientpositive/semijoin6.q
Lines 20 (patched)
<https://reviews.apache.org/r/63343/#comment266413>

    Please add explain for this query.



ql/src/test/queries/clientpositive/semijoin6.q
Lines 22 (patched)
<https://reviews.apache.org/r/63343/#comment266420>

    No need for this test.



ql/src/test/queries/clientpositive/semijoin6.q
Lines 25 (patched)
<https://reviews.apache.org/r/63343/#comment266422>

    One more test (with explain):
    select * from tx1 u join tx2 v on (u.a + v.b > 400) and ((case when u.a > 3 then true when v.b > 1900 then true else false) or (coalesce(u.a) + coalesce(v.b) > 1900) and u.a = v.a



ql/src/test/queries/clientpositive/semijoin6.q
Lines 26 (patched)
<https://reviews.apache.org/r/63343/#comment266419>

    No need for this test.



ql/src/test/queries/clientpositive/semijoin6.q
Lines 28 (patched)
<https://reviews.apache.org/r/63343/#comment266418>

    No need for this test.


- Ashutosh Chauhan


On Oct. 26, 2017, 9:26 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63343/
> -----------------------------------------------------------
> 
> (Updated Oct. 26, 2017, 9:26 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Vineet Garg.
> 
> 
> Bugs: HIVE-17766
>     https://issues.apache.org/jira/browse/HIVE-17766
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-17766
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 3573d07e9a3bab4dabdc0b73292bd02ad05cbc89 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 46493ac006d00f2e6c0054da2438c0b01c1e3fd3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5ccb69ac4824a2002b2bddb1ad206cb7acd3ace3 
>   ql/src/test/queries/clientpositive/semijoin6.q PRE-CREATION 
>   ql/src/test/results/clientpositive/semijoin2.q.out 2c3c5d8db3d5422f21fde020199e878b87b0507f 
>   ql/src/test/results/clientpositive/semijoin6.q.out PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63343/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 63343: HIVE-17766

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



Looks good to me

- Vineet Garg


On Oct. 26, 2017, 9:26 p.m., Jesús Camacho Rodríguez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63343/
> -----------------------------------------------------------
> 
> (Updated Oct. 26, 2017, 9:26 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Vineet Garg.
> 
> 
> Bugs: HIVE-17766
>     https://issues.apache.org/jira/browse/HIVE-17766
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-17766
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 3573d07e9a3bab4dabdc0b73292bd02ad05cbc89 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 46493ac006d00f2e6c0054da2438c0b01c1e3fd3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5ccb69ac4824a2002b2bddb1ad206cb7acd3ace3 
>   ql/src/test/queries/clientpositive/semijoin6.q PRE-CREATION 
>   ql/src/test/results/clientpositive/semijoin2.q.out 2c3c5d8db3d5422f21fde020199e878b87b0507f 
>   ql/src/test/results/clientpositive/semijoin6.q.out PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63343/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jesús Camacho Rodríguez
> 
>


Re: Review Request 63343: HIVE-17766

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/63343/
-----------------------------------------------------------

(Updated Oct. 26, 2017, 9:26 p.m.)


Review request for hive, Ashutosh Chauhan and Vineet Garg.


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


Repository: hive-git


Description
-------

HIVE-17766


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 3573d07e9a3bab4dabdc0b73292bd02ad05cbc89 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 46493ac006d00f2e6c0054da2438c0b01c1e3fd3 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5ccb69ac4824a2002b2bddb1ad206cb7acd3ace3 
  ql/src/test/queries/clientpositive/semijoin6.q PRE-CREATION 
  ql/src/test/results/clientpositive/semijoin2.q.out 2c3c5d8db3d5422f21fde020199e878b87b0507f 
  ql/src/test/results/clientpositive/semijoin6.q.out PRE-CREATION 


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

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


Testing
-------


Thanks,

Jesús Camacho Rodríguez