impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zach Amsden (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4927: Impala should handle invalid input from Sentry
Date Tue, 21 Nov 2017 00:57:57 GMT
Zach Amsden has posted comments on this change. ( )

Change subject: IMPALA-4927: Impala should handle invalid input from Sentry

Patch Set 4:


What version of Sentry was causing this issue?  Also, what was the exact exception being throw?
File fe/src/main/java/org/apache/impala/util/
PS4, Line 141:               // need to be removed.
It's fine to leave this as is, the 90 column limit is not exceeded.
PS4, Line 146:                   sentryPolicyService_.listRolePrivileges(processUser_, role.getName()))
I think it would be better to call listRolePrivileges directly and catch exceptions from that
instead of at the end of the function.  If the role doesn't exist anymore, we definitely want
to remove all its privileges from the catalog instead of going on to exception handling.
PS4, Line 168:             } catch (Exception e) {
Can you catch a more specific exception?  For example, we probably wouldn't want to catch
a NullPointerException here.

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b
Gerrit-Change-Number: 8588
Gerrit-PatchSet: 4
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-Reviewer: Philip Zeyliger <>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Zach Amsden <>
Gerrit-Comment-Date: Tue, 21 Nov 2017 00:57:57 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message