impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-1659: Netezza compatibility functions: metadata
Date Tue, 23 Aug 2016 17:32:18 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-1659: Netezza compatibility functions: metadata
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4063/2/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

Line 2702:   TestStringValue("current_user()", "impala_test_user");
We should extend tests/custom_cluster/test_delegation.py to test that these return the right
results when delegation is enabled.


http://gerrit.cloudera.org:8080/#/c/4063/2/be/src/exprs/utility-functions-ir.cc
File be/src/exprs/utility-functions-ir.cc:

Line 121:   return AnyValUtil::FromString(
This should fit in a single 90-character line.


http://gerrit.cloudera.org:8080/#/c/4063/2/common/function-registry/impala_functions.py
File common/function-registry/impala_functions.py:

Line 550:   [['current_sid'], 'STRING', [], 'impala::UtilityFunctions::CurrentSid'],
I spoke with Greg offline and he pointed out that there wasn't a clear use-case for current_sid,
since there's nothing you can really do with the session id once you have it, e.g. cancel
the session.

Greg, do you see any risks in adding this now?

My feeling is that it's ok to add it, but that we should call it current_session/CurrentSession
and make current_sid an alias for it only.


Line 551:   [['user', 'current_user'], 'STRING', [], 'impala::UtilityFunctions::User'],
I think you have current_user and session_user the wrong way around. session_user would be
the user that initiated the session, so session_user doesn't change with impersonation, but
current_user does change.

E.g. see http://www.ibm.com/support/knowledgecenter/SSULQD_7.1.0/com.ibm.nz.adv.doc/c_advsec_masquerading.html


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9b5d1009bbf42acc175a942d2df484e1c64822ca
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zoltan Ivanfi <zi@cloudera.com>
Gerrit-Reviewer: Greg Rahn <grahn@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Zoltan Ivanfi <zi@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message