----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19344/#review41668 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java <https://reviews.apache.org/r/19344/#comment75228> Same issue with this change. It will break Sql std authorization - in Operation2Privilege - // in alter-table-add-partition, the table is output, and location is input op2Priv.put(HiveOperationType.ALTERTABLE_ADDPARTS, new InOutPrivs(OWNER_INS_SEL_DEL_NOGRANT_AR, INS_NOGRANT_AR)); Can we hold off on this part of the changes, that add uri along with other types such as table and database? Let me take a look at how I can fix the sql std auth to handle this. ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java <https://reviews.apache.org/r/19344/#comment75227> This is already there on line 232 ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java <https://reviews.apache.org/r/19344/#comment75224> This would break the new sql standard authorization. This will add both URI and database to output. See Operation2Privilege.java, it uses only one type of privilege check for output. I agree that is a limitation with the implementation (which is similar to the default authorization in this case). So different kind of check for Database and URI will not be possible. There was another jira where you also had changes for refactoring ReadEntity/WriteEntity to include more information. But I was unable to find that jira now. I wonder if those changes will help improve the code to avoid this issue. ql/src/test/results/clientnegative/authorization_uri_create_table1.q.out <https://reviews.apache.org/r/19344/#comment75225> This is a strange change in output. Not sure why the changes are resulting in ParseException of all things! ql/src/test/results/clientnegative/deletejar.q.out <https://reviews.apache.org/r/19344/#comment75223> This change looks unrelated. - Thejas Nair On March 18, 2014, 7:57 a.m., Navis Ryu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19344/ > ----------------------------------------------------------- > > (Updated March 18, 2014, 7:57 a.m.) > > > Review request for hive and Thejas Nair. > > > Bugs: HIVE-6692 > https://issues.apache.org/jira/browse/HIVE-6692 > > > Repository: hive-git > > > Description > ------- > > Locations for "create table" and "alter table add partition"should be write > entities. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/common/FileUtils.java 16d7c80 > ql/src/java/org/apache/hadoop/hive/ql/Driver.java 7dbb8be > ql/src/java/org/apache/hadoop/hive/ql/hooks/Entity.java 2a38aad > ql/src/java/org/apache/hadoop/hive/ql/hooks/ReadEntity.java 6d7c4f6 > ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java 44a3924 > ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java > db9fa74 > ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java > e642919 > ql/src/java/org/apache/hadoop/hive/ql/parse/ImportSemanticAnalyzer.java > 92ec334 > ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java > 6c53447 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java e1e427f > ql/src/test/results/clientnegative/archive_multi7.q.out a8eee2f > ql/src/test/results/clientnegative/authorization_droppartition.q.out > 1da250a > ql/src/test/results/clientnegative/authorization_uri_alterpart_loc.q.out > 39a4e4f > ql/src/test/results/clientnegative/authorization_uri_create_table1.q.out > 0b8182a > ql/src/test/results/clientnegative/authorization_uri_create_table_ext.q.out > 0b8182a > ql/src/test/results/clientnegative/deletejar.q.out 91560ee > > ql/src/test/results/clientnegative/exim_20_managed_location_over_existing.q.out > fd4a418 > ql/src/test/results/clientnegative/external1.q.out 696beaa > ql/src/test/results/clientnegative/external2.q.out a604885 > ql/src/test/results/clientnegative/insertexternal1.q.out 3df5013 > > Diff: https://reviews.apache.org/r/19344/diff/ > > > Testing > ------- > > > Thanks, > > Navis Ryu > >
