> On Aug. 22, 2014, 6:14 a.m., Szehon Ho wrote: > > ql/src/java/org/apache/hadoop/hive/ql/hooks/ReadEntity.java, line 54 > > <https://reviews.apache.org/r/24962/diff/1/?file=666753#file666753line54> > > > > 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? > > Xiaomeng Huang wrote: > Thanks, I think it better to be list. I get accessed columns from > tableToColumnAccessMap, which is a Map<String, List<String>>. Hive's native > authorization is use this list too. > I get the column list via a table name, then set it to readEntity > directly, don't need to add every one with a loop. so it is necessary to have > a setter. > BTW, I can also to add a API addAccessedColumn(String column) to add one > column to this column list. > > Szehon Ho wrote: > OK its fine if you think it should be list. > > For the other part, I was thinking to have just one method > getAccessedColumn() which returns list. > > Then caller (SemanticAnalyzer) can call: > entity.getAccessedColumns().addAll(...). The benefit to me is 1) the list > can be made final, and 2) make the calling code cleaner (no need to construct > lists and set them). Also its more consistent with the other collections in > this class. Hope that makes sense, thanks!
Yes, it fines to me. Fixed it, thanks! - Xiaomeng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24962/#review51257 ----------------------------------------------------------- On Aug. 25, 2014, 3:17 a.m., Xiaomeng Huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24962/ > ----------------------------------------------------------- > > (Updated Aug. 25, 2014, 3:17 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 > >
