hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "John Sichi" <jsi...@fb.com>
Subject Re: Review Request: HIVE-1694: Accelerate GROUP BY execution using indexes
Date Fri, 18 Mar 2011 00:04:02 GMT

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



ql/src/java/org/apache/hadoop/hive/ql/index/AggregateIndexHandler.java
<https://reviews.apache.org/r/505/#comment684>

    Suggestion is to make this configurable (via IDXPROPERTIES) to save space when column
is known NOT NULL.  (Also later to allow for specification of other aggregates.)



ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteParseContextGenerator.java
<https://reviews.apache.org/r/505/#comment686>

    Indentation is messed up here.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteParseContextGenerator.java
<https://reviews.apache.org/r/505/#comment685>

    Please eliminate all TODO's, and don't use printStackTrace.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteParseContextGenerator.java
<https://reviews.apache.org/r/505/#comment687>

    Instead of downcasting over and over, you should probably be doing it just once in the
calling method (and asserting that you got the right type since otherwise generateOperatorTree
is not going to have the desired effect.
    



ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteCanApplyCtx.java
<https://reviews.apache.org/r/505/#comment688>

    Hive naming convention for variables is camelCase, not under_score.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteCanApplyCtx.java
<https://reviews.apache.org/r/505/#comment690>

    I see query_has_distinct being written but never read.  Why do we need it?  I don't think
we want to be relying on the parse tree at all.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteCanApplyCtx.java
<https://reviews.apache.org/r/505/#comment689>

    Don't swallow exceptions like this.


- John


On 2011-03-13 20:00:28, Prajakta Kalmegh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/505/
> -----------------------------------------------------------
> 
> (Updated 2011-03-13 20:00:28)
> 
> 
> Review request for hive.
> 
> 
> Summary
> -------
> 
> New Review starting from patch 3.
> 
> 
> This addresses bug HIVE-1694.
>     https://issues.apache.org/jira/browse/HIVE-1694
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 46739b7 
>   ql/src/java/org/apache/hadoop/hive/ql/index/AggregateIndexHandler.java PRE-CREATION

>   ql/src/java/org/apache/hadoop/hive/ql/index/HiveIndex.java 308d985 
>   ql/src/java/org/apache/hadoop/hive/ql/index/TableBasedIndexHandler.java PRE-CREATION

>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 916b235 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java 50db44c 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/Optimizer.java 590d69a 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteParseContextGenerator.java PRE-CREATION

>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteCanApplyCtx.java PRE-CREATION

>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteCanApplyProcFactory.java
PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteGBUsingIndex.java PRE-CREATION

>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteIndexSubqueryCtx.java
PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteIndexSubqueryProcFactory.java
PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteRemoveGroupbyCtx.java
PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteRemoveGroupbyProcFactory.java
PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 04f560f 
>   ql/src/test/queries/clientpositive/ql_rewrite_gbtoidx.q PRE-CREATION 
>   ql/src/test/results/clientpositive/ql_rewrite_gbtoidx.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/505/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Prajakta
> 
>


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