Michael Ho has posted comments on this change.

Change subject: IMPALA-3756: Fix wrong argument type in HiveStringsTest
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3617/5/fe/src/test/java/com/cloudera/impala/hive/executor/UdfExecutorTest.java
File fe/src/test/java/com/cloudera/impala/hive/executor/UdfExecutorTest.java:

PS5, Line 235: null
> Does it work with null? Please ignore if it does. Else I think it can be "d
Yes, it works. The DB is not used in this case and the constructor of 
FunctionName also handles this argument being NULL.


PS5, Line 237: TFunction fn = scalar_fn.toThrift()
> nit:Remove and directly call scalar_fn.toThrift() below? Seems like we are 
I personally find the code easier to read if we explicitly declare it here. The 
call to THiveUdfExecutorCtorParam() has a pretty long argument list already.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I662e6286dac601ae0e45f18545ef149724aa047e
Gerrit-PatchSet: 5
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Michael Ho <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-HasComments: Yes

Reply via email to