> 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.
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! - Szehon ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24962/#review51257 ----------------------------------------------------------- On Aug. 22, 2014, 6:47 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:47 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 > >
