----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67803/#review205746 -----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 2649 (patched) <https://reviews.apache.org/r/67803/#comment288612> Let me know your thoughts on this: So when we drop privileges we drop for both grantOption = true and false cases. So what if we then set it to TSentryGrantOption.UNSET, and in method getMSentryPrivilege, grantOption variable will continue to be null and it can then be excluded from the query. Like below ==================================================== private MSentryPrivilege getMSentryPrivilege(TSentryPrivilege tPriv, PersistenceManager pm) { Boolean grantOption = null; if (tPriv.getGrantOption().equals(TSentryGrantOption.TRUE)) { grantOption = true; } else if (tPriv.getGrantOption().equals(TSentryGrantOption.FALSE)) { grantOption = false; } QueryParamBuilder paramBuilder = QueryParamBuilder.newQueryParamBuilder(); paramBuilder.add(SERVER_NAME, tPriv.getServerName()) .add(DB_NAME, tPriv.getDbName()) .add(TABLE_NAME, tPriv.getTableName()) .add(COLUMN_NAME, tPriv.getColumnName()) .add(URI, tPriv.getURI(), true); if(grantOption != null) { paramBuilder.addObject(GRANT_OPTION, grantOption); } paramBuilder.add(ACTION, tPriv.getAction()); Query query = pm.newQuery(MSentryPrivilege.class); query.setUnique(true); query.setFilter(paramBuilder.toString()); return (MSentryPrivilege)query.executeWithMap(paramBuilder.getArguments()); } ==================================================== - Arjun Mishra On July 5, 2018, 4:03 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67803/ > ----------------------------------------------------------- > > (Updated July 5, 2018, 4:03 p.m.) > > > Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Sergio > Pena. > > > Bugs: sentry-2247 > https://issues.apache.org/jira/browse/sentry-2247 > > > Repository: sentry > > > Description > ------- > > add test case to verify the owner privileges are added at db and table > creation. Some testing cases have to be ignore until sentry-2291 is done > > Fix multiple issues found by the new test cases: > 1. The server name of owner privilege was not set at server side > 2. When dropping privilege, if a privilege has no role, it is not dropped > 3. When dropping privilege, if it has grant option being true, it is not > dropped. > > > Diffs > ----- > > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java > 98f2e29 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > ff0b4c0 > > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java > 14a0fd8 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivileges.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestOwnerPrivilegesWithGrantOption.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java > 100219b > > > Diff: https://reviews.apache.org/r/67803/diff/4/ > > > Testing > ------- > > test cases in TestOwnerPrivileges and TestOwnerPrivilegesWithGrantOption > passed > > > Thanks, > > Na Li > >
