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 2016/12/02 17:24:37 UTC

Review Request 54313: HIVE-15251

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

Review request for hive and Ashutosh Chauhan.


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


Repository: hive-git


Description
-------

HIVE-15251


Diffs
-----

  itests/src/test/resources/testconfiguration.properties e4910e4cda49bc3684e7b9ae5aa0607bd16a0a07 
  ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java b62df35cdddc346d0e5d034b10f7b91f9d17d804 
  ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 5512ee27ce04bb36596288d42281b081ba290d7b 
  ql/src/java/org/apache/hadoop/hive/ql/exec/JoinUtil.java 6cbcab699141ec143053e6ee3e2d60e93eeeb71a 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java 2b93e01f36f185ba755bacda89b081fe6286a9ab 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/MapJoinProcessor.java c6efd5b21dcf2633ec027ced952fe4b04621d9d4 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java 37baaf694c314a1377ded9b337b785c8c5c783e5 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java d55db0accf430724b51a8704838964688a5221ec 
  ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java 2ca6b8f1c5a2ad8c586e26d1cbf70d6f1680ebf0 
  ql/src/test/queries/clientpositive/join46.q PRE-CREATION 
  ql/src/test/queries/clientpositive/mapjoin46.q PRE-CREATION 
  ql/src/test/queries/clientpositive/smb_mapjoin_46.q PRE-CREATION 
  ql/src/test/queries/clientpositive/vectorized_join46.q PRE-CREATION 
  ql/src/test/results/clientpositive/join46.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/join46.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/mapjoin46.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/vectorized_join46.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/mapjoin46.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/smb_mapjoin_46.q.out PRE-CREATION 

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


Testing
-------


Thanks,

Jes�s Camacho Rodr�guez


Re: Review Request 54313: HIVE-15251

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

> On Dec. 2, 2016, 8:58 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java, line 101
> > <https://reviews.apache.org/r/54313/diff/1/?file=1574845#file1574845line101>
> >
> >     Was this just an oversight earlier?

It was exactly that; we might end up calling fallback twice and it was causing an Exception to be thrown.


- Jes�s


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


On Dec. 2, 2016, 5:24 p.m., Jes�s Camacho Rodr�guez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54313/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2016, 5:24 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-15251
>     https://issues.apache.org/jira/browse/HIVE-15251
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-15251
> 
> 
> Diffs
> -----
> 
>   itests/src/test/resources/testconfiguration.properties e4910e4cda49bc3684e7b9ae5aa0607bd16a0a07 
>   ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java b62df35cdddc346d0e5d034b10f7b91f9d17d804 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 5512ee27ce04bb36596288d42281b081ba290d7b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/JoinUtil.java 6cbcab699141ec143053e6ee3e2d60e93eeeb71a 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java 2b93e01f36f185ba755bacda89b081fe6286a9ab 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/MapJoinProcessor.java c6efd5b21dcf2633ec027ced952fe4b04621d9d4 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java 37baaf694c314a1377ded9b337b785c8c5c783e5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java d55db0accf430724b51a8704838964688a5221ec 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java 2ca6b8f1c5a2ad8c586e26d1cbf70d6f1680ebf0 
>   ql/src/test/queries/clientpositive/join46.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/mapjoin46.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/smb_mapjoin_46.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/vectorized_join46.q PRE-CREATION 
>   ql/src/test/results/clientpositive/join46.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/join46.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/mapjoin46.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/vectorized_join46.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/mapjoin46.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/smb_mapjoin_46.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54313/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jes�s Camacho Rodr�guez
> 
>


Re: Review Request 54313: HIVE-15251

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




ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java (line 68)
<https://reviews.apache.org/r/54313/#comment228396>

    Comment: List of evaluators for conditions which appear on on-clause and needs to be evaluated before emitting rows. Currently, relevant only for outer joins. 
    e.g. select * from t1 right outer join t2 on t1.c1 + t2.c2 > t1.c3; 
    Expression evaluator for  t1.c1 + t2.c2 > t1.c3 will be stored in this list.



ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java (line 82)
<https://reviews.apache.org/r/54313/#comment228397>

    Comment: OIs corresponding to residualJoinFilters.



ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java (line 84)
<https://reviews.apache.org/r/54313/#comment228398>

    Will be true depending on state of residualJoinFilters.



ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java (lines 86 - 88)
<https://reviews.apache.org/r/54313/#comment228399>

    This data structure is used to keep track of rows on which residualFilters evaluated to false. We will iterate on this container afterwards
      and emit rows appending NULL values if it was not done.
      Key is relation index.



ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java (line 345)
<https://reviews.apache.org/r/54313/#comment228406>

    We need not limit this mechanism for outer join. We can extend this for inner joins, where its matter of efficiency, since we won't be emitting rows which are thrown away by filter straight away. May want to leave a TODO



ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java (lines 494 - 500)
<https://reviews.apache.org/r/54313/#comment228419>

    This if() seems unnecessary, since genObject() does this initialization also.



ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java (line 543)
<https://reviews.apache.org/r/54313/#comment228417>

    Worth adding a note on why we dont need this for left join.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java (line 101)
<https://reviews.apache.org/r/54313/#comment228420>

    Was this just an oversight earlier?



ql/src/test/queries/clientpositive/join46.q (line 5)
<https://reviews.apache.org/r/54313/#comment228421>

    Few rows with null join columns.



ql/src/test/queries/clientpositive/join46.q (line 221)
<https://reviews.apache.org/r/54313/#comment228423>

    Can you add test like: (ROJ)sq1 full outer join (LOJ)sq2 on (sq1.rightcolumn is null or sq2.leftcolumn is null and sq2.leftcolumn != sq1.sq2.rightcolumn)



ql/src/test/queries/clientpositive/mapjoin46.q (line 222)
<https://reviews.apache.org/r/54313/#comment228425>

    Add a -ve test case for LOJ where left side is broadcasted for map join.


- Ashutosh Chauhan


On Dec. 2, 2016, 5:24 p.m., Jes�s Camacho Rodr�guez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54313/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2016, 5:24 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-15251
>     https://issues.apache.org/jira/browse/HIVE-15251
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-15251
> 
> 
> Diffs
> -----
> 
>   itests/src/test/resources/testconfiguration.properties e4910e4cda49bc3684e7b9ae5aa0607bd16a0a07 
>   ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java b62df35cdddc346d0e5d034b10f7b91f9d17d804 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 5512ee27ce04bb36596288d42281b081ba290d7b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/JoinUtil.java 6cbcab699141ec143053e6ee3e2d60e93eeeb71a 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java 2b93e01f36f185ba755bacda89b081fe6286a9ab 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/MapJoinProcessor.java c6efd5b21dcf2633ec027ced952fe4b04621d9d4 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java 37baaf694c314a1377ded9b337b785c8c5c783e5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java d55db0accf430724b51a8704838964688a5221ec 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java 2ca6b8f1c5a2ad8c586e26d1cbf70d6f1680ebf0 
>   ql/src/test/queries/clientpositive/join46.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/mapjoin46.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/smb_mapjoin_46.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/vectorized_join46.q PRE-CREATION 
>   ql/src/test/results/clientpositive/join46.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/join46.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/mapjoin46.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/vectorized_join46.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/mapjoin46.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/smb_mapjoin_46.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54313/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jes�s Camacho Rodr�guez
> 
>


Re: Review Request 54313: HIVE-15251

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

(Updated Dec. 5, 2016, 7:37 p.m.)


Review request for hive and Ashutosh Chauhan.


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


Repository: hive-git


Description
-------

HIVE-15251


Diffs (updated)
-----

  itests/src/test/resources/testconfiguration.properties 772e1233f5662f243ada0a55ba64cd5de7f5d980 
  ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java b62df35cdddc346d0e5d034b10f7b91f9d17d804 
  ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 5512ee27ce04bb36596288d42281b081ba290d7b 
  ql/src/java/org/apache/hadoop/hive/ql/exec/JoinUtil.java 6cbcab699141ec143053e6ee3e2d60e93eeeb71a 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java 7441f1e727a0e218ecaa6a9c497ae37ad9a990f2 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/MapJoinProcessor.java c6efd5b21dcf2633ec027ced952fe4b04621d9d4 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java 37baaf694c314a1377ded9b337b785c8c5c783e5 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 42a7ab9ef7f7295ed405ae21b9750e0b48c730b2 
  ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java 2ca6b8f1c5a2ad8c586e26d1cbf70d6f1680ebf0 
  ql/src/test/queries/clientpositive/join46.q PRE-CREATION 
  ql/src/test/queries/clientpositive/mapjoin46.q PRE-CREATION 
  ql/src/test/queries/clientpositive/smb_mapjoin_46.q PRE-CREATION 
  ql/src/test/queries/clientpositive/vectorized_join46.q PRE-CREATION 
  ql/src/test/results/clientpositive/join46.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/join46.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/mapjoin46.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/vectorized_join46.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/mapjoin46.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/smb_mapjoin_46.q.out PRE-CREATION 

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


Testing
-------


Thanks,

Jes�s Camacho Rodr�guez


Re: Review Request 54313: HIVE-15251

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/54313/#review157998
-----------------------------------------------------------




ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java (lines 494 - 500)
<https://reviews.apache.org/r/54313/#comment228654>

    We need this block for the case when we have only one Join (and it is is right outer or full outer); observe that in that case, when we call genObject, we do it with alias = 1, thus we will not initialize the row container at that point.


- Jes�s Camacho Rodr�guez


On Dec. 2, 2016, 5:24 p.m., Jes�s Camacho Rodr�guez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54313/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2016, 5:24 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-15251
>     https://issues.apache.org/jira/browse/HIVE-15251
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-15251
> 
> 
> Diffs
> -----
> 
>   itests/src/test/resources/testconfiguration.properties e4910e4cda49bc3684e7b9ae5aa0607bd16a0a07 
>   ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java b62df35cdddc346d0e5d034b10f7b91f9d17d804 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java 5512ee27ce04bb36596288d42281b081ba290d7b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/JoinUtil.java 6cbcab699141ec143053e6ee3e2d60e93eeeb71a 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java 2b93e01f36f185ba755bacda89b081fe6286a9ab 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/MapJoinProcessor.java c6efd5b21dcf2633ec027ced952fe4b04621d9d4 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/Vectorizer.java 37baaf694c314a1377ded9b337b785c8c5c783e5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java d55db0accf430724b51a8704838964688a5221ec 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/JoinDesc.java 2ca6b8f1c5a2ad8c586e26d1cbf70d6f1680ebf0 
>   ql/src/test/queries/clientpositive/join46.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/mapjoin46.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/smb_mapjoin_46.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/vectorized_join46.q PRE-CREATION 
>   ql/src/test/results/clientpositive/join46.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/join46.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/mapjoin46.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/vectorized_join46.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/mapjoin46.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/smb_mapjoin_46.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/54313/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jes�s Camacho Rodr�guez
> 
>