Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11181 )
Change subject: IMPALA-7421. Static methods use wrong JNI call function ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/11181/1/be/src/util/jni-util.h File be/src/util/jni-util.h: http://gerrit.cloudera.org:8080/#/c/11181/1/be/src/util/jni-util.h@219 PS1, Line 219: JniCall > Just an idea: the correct usage of the fluent interface could be enforced b How about an even simpler one? I can make static methods JniCall::static(jclass, jmethod) and JniCall::instance(jobject, jmethod). Then the remaining member functions are all valid. Longer term I'd like to add a wrapper like: template<class Result, class Args...> struct JniFunction { JniFunction(const char* function_name); Result Call(Args... args); } which could handle generating the signature based on the template args and then ensure that the passed args match the expected types and count, etc. I didn't want to open that template can of worms for now, though (I was only fixing this because my tests started crashing since I run them with -Xcheck:jni by default) http://gerrit.cloudera.org:8080/#/c/11181/1/be/src/util/jni-util.h@455 PS1, Line 455: Sttring > possible typo: String good news is I got a new laptop yesterday that doesn't have this repeating key issue! -- To view, visit http://gerrit.cloudera.org:8080/11181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7cde6ca91613b63afe5307f4d819fb24cb17fd6 Gerrit-Change-Number: 11181 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Fri, 10 Aug 2018 18:01:09 +0000 Gerrit-HasComments: Yes
