Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11314 )
Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER ...................................................................... Patch Set 29: (8 comments) http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3165 PS29, Line 3165: rolePrivileges > are these meant to be 'added'? Done http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3207 PS29, Line 3207: privileges.get(0) > what's special about the first one? is there some homogeneity assumption he They are homogeneous. The only time there are ever multiple privileges is in the case of column level select privileges. http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3211 PS29, Line 3211: updatedPrivs.size() - 1 > what's special about the version of the last one? perhaps a helper method w Done http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/util/SentryProxy.java File fe/src/main/java/org/apache/impala/util/SentryProxy.java: http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/util/SentryProxy.java@382 PS29, Line 382: * modified. Returns the removed privileges. Throws an exception if there was any error > update the comment to describe the newly added return parameter. Done http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/util/SentryProxy.java@400 PS29, Line 400: new ones added > make this more specific: "these same privileges are added back to the catal Done http://gerrit.cloudera.org:8080/#/c/11314/29/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test File testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test: http://gerrit.cloudera.org:8080/#/c/11314/29/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test@798 PS29, Line 798: 'functional','' > perhaps a regex can be applied here to succeed when anything but 'grant_rev Disabled object ownership by default the dev environment. Caused other tests to fail. This will be reverted. http://gerrit.cloudera.org:8080/#/c/11314/29/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test@859 PS29, Line 859: 'functional','' > same suggestion here to rely on a regex filter Disabled object ownership by default the dev environment. Caused other tests to fail. This will be reverted. http://gerrit.cloudera.org:8080/#/c/11314/26/tests/authorization/test_owner_privileges.py File tests/authorization/test_owner_privileges.py: http://gerrit.cloudera.org:8080/#/c/11314/26/tests/authorization/test_owner_privileges.py@132 PS26, Line 132: @CustomClusterTestSuite.with_ar > unused, pls remove. Done -- To view, visit http://gerrit.cloudera.org:8080/11314 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0 Gerrit-Change-Number: 11314 Gerrit-PatchSet: 29 Gerrit-Owner: Adam Holley <[email protected]> Gerrit-Reviewer: Adam Holley <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Mon, 10 Sep 2018 20:40:58 +0000 Gerrit-HasComments: Yes
