hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ashutosh Chauhan <hashut...@apache.org>
Subject Re: Review Request 54313: HIVE-15251
Date Fri, 02 Dec 2016 20:58:10 GMT

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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message