Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-5489: Improve Sentry authorization for Kudu tables ......................................................................
Patch Set 1: (4 comments) 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: PS1, Line 712: ---- QUERY : insert into grant_rev_db.test_kudu_tbl values (1, "foo"); : ==== : ---- QUERY : upsert into grant_rev_db.test_kudu_tbl values (1, "bar"); Maybe you also want to run these statements before granting insert to show that authz fails. PS1, Line 737: SELECT hm, why 'SELECT'? PS1, Line 740: GRANT SELECT(a) ON TABLE grant_rev_db.test_kudu_tbl TO grant_revoke_test_KUDU : ---- RESULTS : ==== : ---- QUERY : update grant_rev_db.test_kudu_tbl set a = "zzz" This is kind of weird to me. What if you had a where clause in the update statement on column i? Would that work or would it require select privs on i as well? My point is that insert + select (on something) = UPDATE is not intuitive. Maybe it would be simpler to just allow UPDATE only on ALL. What do you think? Line 804: drop role grant_revoke_test_ROOT; Don't you need to clean up the role here? -- 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 <m...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-HasComments: Yes