> On July 22, 2014, 12:40 a.m., Sravya Tirukkovalur wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java, > > line 94 > > <https://reviews.apache.org/r/23721/diff/1/?file=636611#file636611line94> > > > > Not sure what we are doing here by setting authorizer to null?
Hive 0.13 sets the authorizer by default to builtin authorizer implementation. This conflicts with Sentry implementation. > On July 22, 2014, 12:40 a.m., Sravya Tirukkovalur wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java, > > line 565 > > <https://reviews.apache.org/r/23721/diff/1/?file=636611#file636611line565> > > > > Can you please file a follow on to track this Prasad? Create https://issues.apache.org/jira/browse/SENTRY-351 to track it. Thanks! > On July 22, 2014, 12:40 a.m., Sravya Tirukkovalur wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java, > > line 50 > > <https://reviews.apache.org/r/23721/diff/1/?file=636612#file636612line50> > > > > Wonder, what was this hive_extended_entity_capture being used for > > earlier? It was used for capturing URI and UDF info which is now done by default. > On July 22, 2014, 12:40 a.m., Sravya Tirukkovalur wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java, > > line 123 > > <https://reviews.apache.org/r/23721/diff/1/?file=636612#file636612line123> > > > > Is this required? yes, it's a bit hacky to deal with Hive authorization plugin framework. Ideally we should look into using that interface to integrate Sentry with Hive to make the Hive binding cleaner as well as upgrades simpler. Create https://issues.apache.org/jira/browse/SENTRY-352 to track that. > On July 22, 2014, 12:40 a.m., Sravya Tirukkovalur wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java, > > line 367 > > <https://reviews.apache.org/r/23721/diff/1/?file=636613#file636613line367> > > > > Wonder what is the semantics of this command? Can we file a follow on > > jira as this might lead to unexpected output as we are not explicitly > > handling this case and not throwing not supported exception? I haven't looked into that yet. Created https://issues.apache.org/jira/browse/SENTRY-353 to track it. > On July 22, 2014, 12:40 a.m., Sravya Tirukkovalur wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java, > > line 50 > > <https://reviews.apache.org/r/23721/diff/1/?file=636615#file636615line50> > > > > Do we need this output privilege for the alter table add partition? yes. Hive 0.13 adds URI entities to either input and output depending on statement semantics. Hence we need to look into both cases. > On July 22, 2014, 12:40 a.m., Sravya Tirukkovalur wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java, > > line 257 > > <https://reviews.apache.org/r/23721/diff/1/?file=636618#file636618line257> > > > > Is this a hive 0.13 bug? It's a behavior change to restrict replace columns to a small subset of serdes. > On July 22, 2014, 12:40 a.m., Sravya Tirukkovalur wrote: > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java, > > line 162 > > <https://reviews.apache.org/r/23721/diff/1/?file=636621#file636621line162> > > > > Nit: Can we remove these commented out lines? Done > On July 22, 2014, 12:40 a.m., Sravya Tirukkovalur wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryHivePrivilegeObjectDesc.java, > > line 27 > > <https://reviews.apache.org/r/23721/diff/1/?file=636610#file636610line27> > > > > Nit: reset table type which is on by default Done - Prasad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23721/#review48301 ----------------------------------------------------------- On July 21, 2014, 10:50 p.m., Prasad Mujumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23721/ > ----------------------------------------------------------- > > (Updated July 21, 2014, 10:50 p.m.) > > > Review request for sentry, Brock Noland and Sravya Tirukkovalur. > > > Bugs: SENTRY-326 > https://issues.apache.org/jira/browse/SENTRY-326 > > > Repository: sentry > > > Description > ------- > > - Removed the resultset filter hook > - Added a metastore client wrapper that filter out the metadata > - Additional 0.13 compatibility changes > - Includes SENTRY-324 and SENTRY-325 changes > > > Diffs > ----- > > pom.xml 05943c6 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/SentryHiveConstants.java > 26eea91 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java > 27a10ee > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryHivePrivilegeObjectDesc.java > 0447e26 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java > 44c0d20 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java > 1bd0bec > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java > 991d734 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java > 5a8ca6f > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java > 4b2723a > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java > 5c43095 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java > PRE-CREATION > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestOperations.java > 23e91f0 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestRuntimeMetadataRetrieval.java > dac0b92 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestServerConfiguration.java > 436e842 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java > 44331f6 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalHiveServer.java > 02d8024 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java > b376c37 > > Diff: https://reviews.apache.org/r/23721/diff/ > > > Testing > ------- > > Full regression run passes with the patch > > > Thanks, > > Prasad Mujumdar > >
