Zoltan Ivanfi has posted comments on this change.

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


Patch Set 3:

(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
I modified the test but I couldn't run it. I keep trying.


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(ctx, 
PrintId(ctx->impl()->state()->session_id()));
> This should fit in a single 90-character line.
Done


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

Line 550:   [['current_session', 'current_sid'], 'STRING', [], 
'impala::UtilityFunctions::CurrentSession'],
> I spoke with Greg offline and he pointed out that there wasn't a clear use-
Done


Line 551:   [['user', 'session_user'], 'STRING', [], 
'impala::UtilityFunctions::User'],
> I think you have current_user and session_user the wrong way around. sessio
The PDF attachment of the feature request contained this definition:

CURRENT_USER
alias for user()? function already exists

But the link you sent convinced me, so I fixed it to reflect that definition.


-- 
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: 3
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

Reply via email to