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

Reply via email to