----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16847/#review31813 -----------------------------------------------------------
Thejas, thank you very much for the interface annotations! Glad to see this being included more in Hive. Say, DDLTask is one ugly class. Is there any reason we cannot create AuthorizationTask or similar? ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java <https://reviews.apache.org/r/16847/#comment60526> There should be an assertion that dbTab.length is 1? ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java <https://reviews.apache.org/r/16847/#comment60527> nit: trailing ws ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/DefaultHiveAuthorizerFactory.java <https://reviews.apache.org/r/16847/#comment60528> ? ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizer.java <https://reviews.apache.org/r/16847/#comment60523> We should state this is the v2 interface ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveMetastoreClientFactoryImpl.java <https://reviews.apache.org/r/16847/#comment60524> Do we have to do this? ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java <https://reviews.apache.org/r/16847/#comment60525> Shouldn't we also assert that both cannot be not null? ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java <https://reviews.apache.org/r/16847/#comment60522> else is unneeded and I think it should throw java.lang.AssertionError. ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveOperationType.java <https://reviews.apache.org/r/16847/#comment60521> nit: spacing ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveOperationType.java <https://reviews.apache.org/r/16847/#comment60519> We should only catch IllegalArgException there and the spacing is off - Brock Noland On Jan. 14, 2014, 3:51 a.m., Thejas Nair wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16847/ > ----------------------------------------------------------- > > (Updated Jan. 14, 2014, 3:51 a.m.) > > > Review request for hive, Ashutosh Chauhan and Brock Noland. > > > Bugs: HIVE-5928 > https://issues.apache.org/jira/browse/HIVE-5928 > > > Repository: hive-git > > > Description > ------- > > The existing HiveAuthorizationProvider interface implementations can be used > to support custom authorization models. > But this interface limits the customization for these reasons - > 1. It has assumptions about the privileges required for an action. > 2. It does have not functions that you can implement for having custom ways > of doing the actions of access control statements. > > This jira proposes a new interface HiveAuthorizer that does not make > assumptions of the privileges required for the actions. The authorize() > functions will be equivalent of authorize(<operation type>, <input objects>, > <output objects>). It will also have functions that will be called from the > access control statements. > > The current HiveAuthorizationProvider will continue to be supported for > backward compatibility. > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/Driver.java 72c04d3 > ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java b36a4ca > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java dc45ea2 > ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java 143c0a6 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 52d7c75 > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/DefaultHiveAuthorizerFactory.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAccessController.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizationValidator.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizer.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizerFactory.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizerImpl.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveMetastoreClientFactory.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveMetastoreClientFactoryImpl.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveOperationType.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrincipal.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilege.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java ef35f1a > > ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/TestHiveOperationType.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/16847/diff/ > > > Testing > ------- > > > Thanks, > > Thejas Nair > >
