----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24962/#review51257 -----------------------------------------------------------
Hi Xiaomeng, patch looks good, just had some style comments. ql/src/java/org/apache/hadoop/hive/ql/hooks/ReadEntity.java <https://reviews.apache.org/r/24962/#comment89359> Can we make this final, and not have a setter? The caller can just add to the list. It'll make the code a bit simpler. Also should it be set? ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java <https://reviews.apache.org/r/24962/#comment89360> No need for '==true' part. ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java <https://reviews.apache.org/r/24962/#comment89362> Can we indent this code block inside {}? - Szehon Ho On Aug. 22, 2014, 6:01 a.m., Xiaomeng Huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24962/ > ----------------------------------------------------------- > > (Updated Aug. 22, 2014, 6:01 a.m.) > > > Review request for hive, Prasad Mujumdar and Szehon Ho. > > > Repository: hive-git > > > Description > ------- > > External authorization model can not get accessed columns from query. Hive > should store accessed columns to ReadEntity > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/hooks/ReadEntity.java 7ed50b4 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java b05d3b4 > > Diff: https://reviews.apache.org/r/24962/diff/ > > > Testing > ------- > > > Thanks, > > Xiaomeng Huang > >