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 <[email protected]> Gerrit-Reviewer: Greg Rahn <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Ivanfi <[email protected]> Gerrit-HasComments: Yes
