> On Oct. 15, 2014, 9:39 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/reloperators/HiveProjectRel.java,
> >  line 71
> > <https://reviews.apache.org/r/26721/diff/1/?file=721199#file721199line71>
> >
> >     Why are we recomputing the digest here?

that updates toString; not needed except for debugging


> On Oct. 15, 2014, 9:39 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/PlanModifierForASTConv.java,
> >  line 151
> > <https://reviews.apache.org/r/26721/diff/1/?file=721201#file721201line151>
> >
> >     This change seems just inversion of logic.
> >     Does Hive Coding convention proposes this?

IMHO the code is easier to read this way, because it always stays linear so 
there's no need to refer back to what is nested in what


> On Oct. 15, 2014, 9:39 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/RowResolver.java, line 354
> > <https://reviews.apache.org/r/26721/diff/1/?file=721202#file721202line354>
> >
> >     Instead of introducing a new class why don't you return a Pair<Boolean, 
> > Integer>

Only one caller needs the int; Pair will make it more cumbersome for all other 
callers


- Sergey


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


On Oct. 15, 2014, 12:44 a.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26721/
> -----------------------------------------------------------
> 
> (Updated Oct. 15, 2014, 12:44 a.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/reloperators/HiveProjectRel.java
>  7b434ea 
>   
> 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/PlanModifierForASTConv.java
>  4f96d02 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/RowResolver.java 9c55379 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java e1eca13 
>   ql/src/test/queries/clientpositive/select_same_col.q PRE-CREATION 
>   ql/src/test/results/clientpositive/select_same_col.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/26721/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>

Reply via email to