----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23373/#review47565 -----------------------------------------------------------
metastore/if/hive_metastore.thrift <https://reviews.apache.org/r/23373/#comment83571> Making these fields optional can be confusing for external users. Should we just update this unit test to set some dummy values ? metastore/if/hive_metastore.thrift <https://reviews.apache.org/r/23373/#comment83572> Can you also add comments saying that grant_role and revoke_role functions are deprecated ? Unfortunately, thrift does not seem have built-in proper support for deprecation. (https://issues.apache.org/jira/browse/THRIFT-640) ql/src/test/results/clientnegative/authorization_role_grant2.q.out <https://reviews.apache.org/r/23373/#comment83663> This not related to your changes, but can you make this minor correction to the error message ? "ADMIN privileges on role" is better worded as "ADMIN OPTION on role" - Thejas Nair On July 10, 2014, 2:53 a.m., Jason Dere wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23373/ > ----------------------------------------------------------- > > (Updated July 10, 2014, 2:53 a.m.) > > > Review request for hive and Thejas Nair. > > > Bugs: HIVE-6252 > https://issues.apache.org/jira/browse/HIVE-6252 > > > Repository: hive-git > > > Description > ------- > > Parser changes - support REVOKE ADMIN ROLE FOR > New grant_revoke_role() thrift metastore method > > > Diffs > ----- > > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestAuthorizationApiAuthorizer.java > 6b2f28e > metastore/if/hive_metastore.thrift d425d2b > metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 2a1b4d7 > metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 9567874 > metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp > b18009c > metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h a0f208a > metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp a6cd09a > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddPartitionsRequest.java > 791c46b > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddPartitionsResult.java > 2471690 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ColumnStatistics.java > aa647d4 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/DropPartitionsResult.java > b8d5a56 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Function.java > 4a24bbf > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetOpenTxnsInfoResponse.java > 427204e > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetOpenTxnsResponse.java > eda18ad > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPrincipalsInRoleResponse.java > 083699b > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetRoleGrantsForPrincipalResponse.java > f745c08 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/HeartbeatTxnRangeResponse.java > 0fc4310 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/HiveObjectRef.java > 997060f > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/LockRequest.java > c35aadd > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/OpenTxnsResponse.java > 3d47286 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Partition.java > 312807e > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionsByExprResult.java > ea8f0bb > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionsStatsRequest.java > a46bdc8 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionsStatsResult.java > 27f654d > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PrincipalPrivilegeSet.java > eea86e5 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PrivilegeBag.java > a4687ad > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/RequestPartsSpec.java > 5119b83 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Schema.java > d91ca2d > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ShowCompactResponse.java > a9f9f7c > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ShowLocksResponse.java > d2657e0 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/SkewedInfo.java > 83438c7 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/StorageDescriptor.java > d0b9843 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Table.java > 229a819 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/TableStatsRequest.java > 48d16b7 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/TableStatsResult.java > b25c6c2 > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java > 4f051af > > metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Type.java > bb81e3c > metastore/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php c79624f > metastore/src/gen/thrift/gen-php/metastore/Types.php 3db3ded > metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote > fdedb57 > metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py > 23679be > metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py 43a498a > metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb feb99db > metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 56c23e6 > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > acef599 > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java > 0595b09 > metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java > 0c2209b > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java > 911c997 > metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java e0de0e0 > > metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java > 5c00aa1 > > metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java > 5025b83 > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java bbf89ef > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java fea1e47 > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 5ac6452 > > ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactoryImpl.java > 419117c > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLStdHiveAccessController.java > 6ede03c > ql/src/test/queries/clientnegative/authorization_role_grant2.q PRE-CREATION > ql/src/test/queries/clientpositive/authorization_role_grant1.q 051bdee > ql/src/test/results/clientnegative/authorization_role_grant2.q.out > PRE-CREATION > ql/src/test/results/clientpositive/authorization_role_grant1.q.out cdbcb26 > > Diff: https://reviews.apache.org/r/23373/diff/ > > > Testing > ------- > > unit tests added > > > Thanks, > > Jason Dere > >