----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24973/#review51447 -----------------------------------------------------------
Nice work! Mostly looks good to me. Minor comments below. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java <https://reviews.apache.org/r/24973/#comment89783> sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift <https://reviews.apache.org/r/24973/#comment89826> As this also allows to add mutliple privileges at table/db scope. Can we add tests for that? sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java <https://reviews.apache.org/r/24973/#comment89788> Nit: Fix the assert message, only on t1.c1? sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java <https://reviews.apache.org/r/24973/#comment89814> Looks like we do not have a use case in Hive where we will need insert privileges at Column scope. If so, can we notify users trying to do "GRANT INSERT on (c1,c2) on TABLE tb1 to .." that it is not supported? - Sravya Tirukkovalur On Aug. 22, 2014, 10:12 a.m., Sun Dapeng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24973/ > ----------------------------------------------------------- > > (Updated Aug. 22, 2014, 10:12 a.m.) > > > Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya > Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > The jira include 6 SENTRY subtask and 1 HIVE jira: > > 1. SENTRY-389 Database implementation for column > 2. SENTRY-391 Extend sentrystore query for column level privilege > 3. SENTRY-390 Extend Thrift API to support column-level privilege > 4. SENTRY-393 Grant/Revoke and Show Grant info support for column level > privilege > 5. HIVE-7730 Extend ReadEntity to add accessed columns from query > (https://reviews.apache.org/r/24962/) > 6. SENTRY-392 Authorization for column level security > 7. SENTRY-394 PolicyFile and ConfigImport support for column level privilege > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java > 49922f9 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java > 0b26806 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryHivePrivilegeObjectDesc.java > 1b5f557 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java > 2df741c > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java > f38ee91 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java > 761082a > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java > 1c32946 > > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java > 6e40a5c > > sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/Column.java > PRE-CREATION > > sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/DBModelAuthorizable.java > de35bfa > > sentry-policy/sentry-policy-db/src/main/java/org/apache/sentry/policy/db/DBModelAuthorizables.java > 873f789 > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java > 51d4248 > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TAlterSentryRoleGrantPrivilegeRequest.java > 62b6b31 > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TAlterSentryRoleRevokePrivilegeRequest.java > bbd9536 > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesForProviderRequest.java > 10ab56b > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesForProviderResponse.java > 4c571c2 > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesResponse.java > d34205a > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryRolesResponse.java > 13f22ff > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryActiveRoleSet.java > 573dc26 > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryAuthorizable.java > 59418a3 > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java > 54b6204 > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryRole.java > f43a6d5 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/AuditMetadataLogEntity.java > e1d8a9e > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/entity/JsonLogEntityFactory.java > 2cc8194 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/CommandUtil.java > 841eeb3 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/log/util/Constants.java > 4b1d7de > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java > 5328fff > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo > b39cb18 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 33600e9 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java > d4c5806 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > f227a02 > sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql > c1a2778 > > sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql > c1a2778 > > sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql > d7ce57c > > sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql > 3f01cda > > sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql > 186c968 > > sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift > b14616b > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestAuditMetadataLogEntity.java > cd0a435 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/entity/TestJsonLogEntityFactory.java > fc9c716 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/log/util/TestCommandUtil.java > 5da8591 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryPrivilege.java > 91d3171 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 985a73d > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreToAuthorizable.java > d77786a > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java > 5244094 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java > 8b83859 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegesAtColumnScope.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java > 3a7aa41 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestConfigTool.java > c186a42 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestExportImportPrivileges.java > eaf3816 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPolicyImport.java > c238361 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtColumnScope.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java > 4c66ffe > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/SentryPolicyProviderForDb.java > c60d0d5 > > Diff: https://reviews.apache.org/r/24973/diff/ > > > Testing > ------- > > unit test in local > > > Thanks, > > Sun Dapeng > >
