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 51755: Support Intersect Distinct
Date Fri, 07 Oct 2016 02:57:17 GMT

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




ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelFactories.java (line 209)
<https://reviews.apache.org/r/51755/#comment216301>

    Error message: Union or intersect



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveIntersect.java (line
30)
<https://reviews.apache.org/r/51755/#comment220241>

    whitespace



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectMergeRule.java
(line 31)
<https://reviews.apache.org/r/51755/#comment216303>

    It will be good to add a comment describing transformation we are trying to achieve here.
Better will be in terms of sql level transform, if possible.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectMergeRule.java
(line 59)
<https://reviews.apache.org/r/51755/#comment216304>

    Is this check not required on bottomIntersect?



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectMergeRule.java
(line 65)
<https://reviews.apache.org/r/51755/#comment216306>

    This assert should be before if()



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java
(line 64)
<https://reviews.apache.org/r/51755/#comment220242>

    Good to add more details on how this rule is transforming operator tree.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java
(line 102)
<https://reviews.apache.org/r/51755/#comment220243>

    Will be useful to explain why Project is needed here.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java
(line 107)
<https://reviews.apache.org/r/51755/#comment220244>

    Remove TODO



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java
(line 108)
<https://reviews.apache.org/r/51755/#comment220245>

    rethrow after logging.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java
(line 148)
<https://reviews.apache.org/r/51755/#comment220246>

    Add comments on why we need to have project here which is not doing any transformations.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java
(line 150)
<https://reviews.apache.org/r/51755/#comment220247>

    TODO



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java
(line 151)
<https://reviews.apache.org/r/51755/#comment220248>

    log and rethrow.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java
(line 180)
<https://reviews.apache.org/r/51755/#comment220249>

    It is a deterministic function.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java
(line 189)
<https://reviews.apache.org/r/51755/#comment220250>

    TODO



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java
(line 190)
<https://reviews.apache.org/r/51755/#comment220251>

    log and rethrow.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java
(line 212)
<https://reviews.apache.org/r/51755/#comment220252>

    remove TODO



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java
(line 213)
<https://reviews.apache.org/r/51755/#comment220253>

    log and rethrow.



ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java (line 2090)
<https://reviews.apache.org/r/51755/#comment220254>

    ws



ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g (line 93)
<https://reviews.apache.org/r/51755/#comment220255>

    Good to call this TOK_EXCEPTALL since KW is except not minus.



ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g (line 94)
<https://reviews.apache.org/r/51755/#comment220258>

    TOK_EXCEPTDISTINCT



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java (line 516)
<https://reviews.apache.org/r/51755/#comment220256>

    insideView not needed anymore?



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java (line 523)
<https://reviews.apache.org/r/51755/#comment220257>

    insideView not needed anymore?



ql/src/test/queries/clientpositive/intersect.q (line 40)
<https://reviews.apache.org/r/51755/#comment220259>

    Add a test where there is a GBy on different columns with different aggregates on two
branches of intersect.


- Ashutosh Chauhan


On Sept. 9, 2016, 1:41 a.m., pengcheng xiong wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51755/
> -----------------------------------------------------------
> 
> (Updated Sept. 9, 2016, 1:41 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-12765
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelFactories.java cf93ed8

>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveIntersect.java
PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectMergeRule.java
PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java
PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTConverter.java
9f5e733 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java ff94160 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 7ceb005 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g df596ff 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 9ba1865 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/QBExpr.java cccf0f6 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 943d9d7 
>   ql/src/test/queries/clientpositive/intersect.q PRE-CREATION 
>   ql/src/test/results/clientpositive/intersect.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51755/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> pengcheng xiong
> 
>


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