Sailesh Mukil has posted comments on this change. Change subject: IMPALA-5489: Improve Sentry authorization for Kudu tables ......................................................................
Patch Set 1: (2 comments) Just a couple of nits. http://gerrit.cloudera.org:8080/#/c/7307/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java: Line 131: formatArgs)); Add a test case for all privileges? http://gerrit.cloudera.org:8080/#/c/7307/1/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test File testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test: Line 696: create role grant_revoke_test_KUDU Would it be preferable to create a separate file for Kudu grant revoke tests? We do that for other tests specific to Kudu IIRC. The upside is that we can disable kudu tests while running against other platforms and reduce test time. But the downside is too many test files. I'm okay with either as long as we've considered both the alternatives. -- To view, visit http://gerrit.cloudera.org:8080/7307 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib12d2b32fa3e142e69bd8b0f24f53f9e5cbf7460 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-HasComments: Yes
