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




ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsAutoGatherContext.java 
(line 102)
<https://reviews.apache.org/r/47419/#comment198467>

    For static partitions no need to specify part-spec ?



ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsAutoGatherContext.java 
(line 151)
<https://reviews.apache.org/r/47419/#comment198468>

    Will this trigger new calls to metastore to collect metadata ?



ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsAutoGatherContext.java 
(line 157)
<https://reviews.apache.org/r/47419/#comment198469>

    Might make sense to throw an exception if values().size() > 1



ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsAutoGatherContext.java 
(line 215)
<https://reviews.apache.org/r/47419/#comment198470>

    Can you add a comment in what case srcType != destType



ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java 
(line 399)
<https://reviews.apache.org/r/47419/#comment198471>

    Better name: rewriteAST() ?



ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java (lines 66 - 67)
<https://reviews.apache.org/r/47419/#comment198473>

    Can you please add comments describing purpose of these new data structures?



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java (lines 7052 - 
7062)
<https://reviews.apache.org/r/47419/#comment198466>

    This will add latency in query compilation because this makes new calls to 
metastore to get table and partition objects. 
    An alternative could be to always add CS pipeline and then delete it later 
(if needed) either in FetchOptimizer where ParseContext already have these 
objects or even later in TaskCompiler.



ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java (lines 267 - 277)
<https://reviews.apache.org/r/47419/#comment198474>

    Wanna move this utility BFS search method in some utils class. Might be 
useful for other purposes as well.


Thanks for nicely commenting the patch.
Overall looks good, my major concern is around how much compile latency this 
patch may increase?

- Ashutosh Chauhan


On May 16, 2016, 6:11 p.m., pengcheng xiong wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47419/
> -----------------------------------------------------------
> 
> (Updated May 16, 2016, 6:11 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HIVE-13566
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 541af57 
>   itests/src/test/resources/testconfiguration.properties c891d40 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FetchOperator.java d8ac6ae 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java 
> d213731 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/GenMRSkewJoinProcessor.java
>  9fbbd4c 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java 
> 02c5a89 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsAutoGatherContext.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java 
> 3b6cbce 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 96ef20d 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java 3a226e7 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 7162c08 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 4049f40 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBridge.java 
> 7433263 
>   ql/src/test/queries/clientpositive/autoColumnStats_1.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/autoColumnStats_2.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/autoColumnStats_3.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/autoColumnStats_4.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/autoColumnStats_5.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/autoColumnStats_6.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/autoColumnStats_7.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/autoColumnStats_8.q PRE-CREATION 
>   ql/src/test/results/clientpositive/autoColumnStats_1.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/autoColumnStats_2.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/autoColumnStats_3.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/autoColumnStats_4.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/autoColumnStats_5.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/autoColumnStats_6.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/autoColumnStats_7.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/autoColumnStats_8.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/tez/autoColumnStats_2.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47419/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> pengcheng xiong
> 
>

Reply via email to