hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Carl Steinbach" <c...@cloudera.com>
Subject Re: Review Request: HIVE-1790: HAVING clause in Hive
Date Tue, 14 Dec 2010 07:25:46 GMT

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



ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java
<https://reviews.apache.org/r/162/#comment46>

    Please change the type to Map.



ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java
<https://reviews.apache.org/r/162/#comment47>

    Ditto



ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java
<https://reviews.apache.org/r/162/#comment49>

    Please change the return type to Map.
    



ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java
<https://reviews.apache.org/r/162/#comment50>

    Ditto.
    



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/162/#comment51>

    It looks like there is a fair amount of duplicated code between this method and genFilterPlan(String
dest, QB qb, Operator input). If possible can you change the name of genFilterPlan(String,
QB, Operator) to getWherePlan(String, QB, Operator), and have both this method and genHavingPlan()
call a common genFilterPlan(QB, ASTNode, Operator) method?
    



ql/src/test/queries/clientpositive/having.q
<https://reviews.apache.org/r/162/#comment64>

    Please add the following test queries:
    
    * Queries that contain both WHERE and HAVING clauses.
    * A query with an aggregate function in the HAVING clause that also appears in the SELECT
clause without an alias.
    * A query that contains a HAVING clause but no GROUP BY clause.


- Carl


On 2010-12-09 15:59:46, Carl Steinbach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/162/
> -----------------------------------------------------------
> 
> (Updated 2010-12-09 15:59:46)
> 
> 
> Review request for hive.
> 
> 
> Summary
> -------
> 
> Review of HIVE-1790 by CWS.
> 
> 
> This addresses bug HIVE-1790.
>     https://issues.apache.org/jira/browse/HIVE-1790
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 6b47702 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java c36adc7 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java c667a63 
>   ql/src/test/queries/clientpositive/having.q PRE-CREATION 
>   ql/src/test/results/clientpositive/having.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/162/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Carl
> 
>


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