impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4965: Authorize access to runtime profile and exec summary
Date Thu, 08 Jun 2017 00:14:21 GMT
Dan Hecht has posted comments on this change.

Change subject: IMPALA-4965: Authorize access to runtime profile and exec summary
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7064/7/be/src/service/client-request-state.h
File be/src/service/client-request-state.h:

Line 218:       const ClientRequestState& request_state);
> We need two implementations of CheckProfileAccess(), one that works against
The real logic is already consolidated in the common CheckProfileAccess() function that takes
both users and bool. These friend functions just pull out  So how about we just get rid of
these two friend functions and call directly? They don't seem to really add much (there are
only two callsites each and from the same file) and it seems clearer anyway to see where the
two user values come.

If you really want to keep the wrappers, why do they need to be 'friend'? Isn't the state
they access public?

The real problem here is that we have two classes to represent the same info. Down the road
we should get rid of QueryStateRecord.


-- 
To view, visit http://gerrit.cloudera.org:8080/7064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2255d587367c2d328590ae8534a5406c4b0c9b15
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message