> On Oct. 11, 2014, 12:42 a.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/RelOptHiveTable.java, 
> > line 210
> > <https://reviews.apache.org/r/26577/diff/1/?file=717854#file717854line210>
> >
> >     Why this move?

This was one of the instances where the variable scope was too broad preventing 
the compiler from optimizing if needed.


> On Oct. 11, 2014, 12:42 a.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/stats/FilterSelectivityEstimator.java,
> >  line 147
> > <https://reviews.apache.org/r/26577/diff/1/?file=717871#file717871line147>
> >
> >     This move seems less performant; i know compiler might optimize it.

I'm pretty sure the compiler will optimize this and it makes the method shorter 
more readable as well as narrows the scope of the variable to where it's 
actually needed.


> On Oct. 11, 2014, 12:42 a.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/RelOptHiveTable.java, 
> > line 240
> > <https://reviews.apache.org/r/26577/diff/1/?file=717854#file717854line240>
> >
> >     This move mess up the code logic flow; i know you just wanted to do 
> > check true case first.
> >     
> >     Also the comment numbers are messed up.

Good catch, sorry, I've reverted the change.


- Lars


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


On Oct. 10, 2014, 9:44 p.m., Lars Francke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26577/
> -----------------------------------------------------------
> 
> (Updated Oct. 10, 2014, 9:44 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-8222
>     https://issues.apache.org/jira/browse/HIVE-8222
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Cleans up CBO code
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryProperties.java 5dab171 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/GroupByOperator.java 4632f08 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorizationContext.java 
> 34f5823 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/ColumnPrunerProcFactory.java 
> ba28bc7 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/HiveDefaultRelMetadataProvider.java
>  e9e052f 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/HiveOptiqUtil.java 
> 4bb99d9 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/HiveTypeSystemImpl.java 
> 1bc5a2c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/OptiqSemanticException.java
>  d2b08fa 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/RelOptHiveTable.java 
> 080d27f 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/TraitsUtil.java 
> 9b653d3 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/cost/HiveCost.java 
> 72fe5d6 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/cost/HiveCostUtil.java 
> 7436f12 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/cost/HiveVolcanoPlanner.java
>  15596bc 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/reloperators/HiveAggregateRel.java
>  fc19895 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/reloperators/HiveFilterRel.java
>  8b85046 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/reloperators/HiveJoinRel.java
>  3d6aa84 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/reloperators/HiveLimitRel.java
>  f8755d0 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/reloperators/HiveProjectRel.java
>  7b434ea 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/reloperators/HiveRel.java
>  4738c4a 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/reloperators/HiveSortRel.java
>  f85363d 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/reloperators/HiveTableScanRel.java
>  bd66459 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/reloperators/HiveUnionRel.java
>  d34fe95 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/rules/HivePartitionPrunerRule.java
>  ee19a6c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/rules/HivePushFilterPastJoinRule.java
>  da0f7a4 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/rules/PartitionPruner.java
>  a218eca 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/stats/FilterSelectivityEstimator.java
>  214a29f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/stats/HiveRelMdDistinctRowCount.java
>  4be57b1 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/stats/HiveRelMdRowCount.java
>  949eb19 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/stats/HiveRelMdSelectivity.java
>  49d2ee5 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/stats/HiveRelMdUniqueKeys.java
>  06ff584 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/ASTBuilder.java
>  98723a3 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/ASTConverter.java
>  f5a704f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/ExprNodeConverter.java
>  e6e491f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/JoinCondTypeCheckProcFactory.java
>  406c18e 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/JoinTypeCheckCtx.java
>  fdee66b 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/PlanModifierForASTConv.java
>  3d90ae7 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/RexNodeConverter.java
>  ec85603 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/SqlFunctionConverter.java
>  31f906a 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/TypeConverter.java
>  2c30e9d 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/RowResolver.java 9c55379 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5c7f148 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 
> e065983 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AggregationDesc.java 1a0cdf8 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeConstantDesc.java 
> 8a41577 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDescUtils.java 686befd 
> 
> Diff: https://reviews.apache.org/r/26577/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Lars Francke
> 
>

Reply via email to