Michael Ho has posted comments on this change.

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


Patch Set 2:

(4 comments)

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

PS2, Line 215:  loading the class and validating it
             :    * has the proper function.
> This statement in the comment will be removed in the next patch.
Done


PS2, Line 224: createUdfExecutor
> nit: static method?
Doesn't quite work as this function invokes some non-static functions.


PS2, Line 237: TFunction fn = new TFunction(new TFunctionName("fn"), 
TFunctionBinaryType.JAVA,
             :         argTypes, retType.toThrift(), false);
             :     fn.setScalar_fn(new TScalarFunction(udfPath));
> nit: I think you can use the factory method, ScalarFunction.createForTestin
Didn't know there is a factory method for it. Good point. Done.


PS2, Line 249: byte[] params_byte_array = serializer.serialize(params);
             :     return new UdfExecutor(params_byte_array);
> nit: return new UdfExecutor(serialize.serialize(params)) ?
Done


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