Skye Wanderman-Milne has posted comments on this change.

Change subject: IMPALA-3378/IMPALA-3379: fix various JNI issues
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/2820/1/be/src/exec/hbase-table-scanner.cc
File be/src/exec/hbase-table-scanner.cc:

Line 544: (
> parenthesis unnecessary
Done


Line 545: // cells_ = result.raw();
> is this a comment or is it stale code ?
It looks like it's supposed to be a comment for the below line.


http://gerrit.cloudera.org:8080/#/c/2820/1/be/src/exprs/hive-udf-call.h
File be/src/exprs/hive-udf-call.h:

Line 60:   /// Must be called before creating any HiveUdfCall instances.
> Please also mention that it's called once at Impalad init time.
Done


http://gerrit.cloudera.org:8080/#/c/2820/1/be/src/util/jni-util.h
File be/src/util/jni-util.h:

Line 182:  LocalToGlobalRef
> is this one actually used ? If it's for jobject subclasses, the one above s
>From http://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/types.html:

class _jobject {}; 
class _jclass : public _jobject {}; 
... 
typedef _jobject *jobject; 
typedef _jclass *jclass; 

So the above doesn't work because jobject* is actually _jobject**, and so you 
can't use a subclass like _jclass** without the reinterpret_cast (this 
generally wouldn't be safe but it is in this case, since the classes are empty 
and we know the exact subclass being returned).

I'll add a note explaining this to the comment.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8cd089e355d2ee2d5ace81f05b214272c05cf941
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Skye Wanderman-Milne <[email protected]>
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Skye Wanderman-Milne <[email protected]>
Gerrit-HasComments: Yes

Reply via email to