Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10919 )
Change subject: IMPALA-7275: Create table authorization error should not show table name ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/10919/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10919/3//COMMIT_MSG@7 PS3, Line 7: tbl auth > nit: table authorization (we shouldn't be abbreviating it) Done http://gerrit.cloudera.org:8080/#/c/10919/3//COMMIT_MSG@7 PS3, Line 7: table > nit: table name Done http://gerrit.cloudera.org:8080/#/c/10919/3//COMMIT_MSG@11 PS3, Line 11: remove : the table name. > Give an explanation why having a table name in the exception for CREATE TAB Done http://gerrit.cloudera.org:8080/#/c/10919/3/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java File fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java: http://gerrit.cloudera.org:8080/#/c/10919/3/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java@122 PS3, Line 122: List<DBModelAuthorizable> authorizeables = privilegeRequest.getAuthorizeable(). : getHiveAuthorizeableHierarchy(); : if (privilege == Privilege.CREATE && authorizeables.size() > 1 && : !(privilegeRequest.getAuthorizeable() instanceof AuthorizeableFn)) { : // CREATE on an object requires CREATE on the parent and we shouldn't : // expose the table name. Since we don't get back the modified : // authorizable list, we need to check again and modify the error. : authorizeables.remove(authorizeables.size() - 1); : StringBuilder sb = new StringBuilder(); : Iterator<DBModelAuthorizable> authIt = authorizeables.iterator(); : while (authIt.hasNext()) { : DBModelAuthorizable authorizeable = authIt.next(); : sb.append(authorizeable.getName()); : if (authIt.hasNext()) { : sb.append("."); : } : } : throw new AuthorizationException(String.format( : "User '%s' does not have privileges to execute '%s' on: %s", : user.getName(), privilege, sb.toString())); : > The logic here is way too complicated. You're mutating the list and also bu Done -- To view, visit http://gerrit.cloudera.org:8080/10919 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6711a744541abd5ff26574974ba8517b6e51c453 Gerrit-Change-Number: 10919 Gerrit-PatchSet: 3 Gerrit-Owner: Adam Holley <ahol...@cloudera.com> Gerrit-Reviewer: Adam Holley <ahol...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Comment-Date: Wed, 11 Jul 2018 16:49:44 +0000 Gerrit-HasComments: Yes
