Jim Apple has posted comments on this change. Change subject: Fix undefined calls to __builtin_ctz. ......................................................................
Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/5004/3/be/src/udf_samples/hyperloglog-uda.cc File be/src/udf_samples/hyperloglog-uda.cc: PS3, Line 78: uint > uint64_t? Seems clearer to specify the type and it's not too verbose. Done PS3, Line 80: (hash_top_bit > hash_top_bits != 0 would make the intent clearer (it's a matter of taste, b Done http://gerrit.cloudera.org:8080/#/c/5004/3/be/src/util/bit-util.h File be/src/util/bit-util.h: Line 32: #include <boost/type_traits/make_unsigned.hpp> > Is this used? Yes, but I was just moving it in the #include list, not using it. It's used elsewhere. Line 247: /// zero. > We don't use the optional argument, so it seems like we should just remove We do use the optional argument in be/src/exprs/aggregate-functions-ir.cc. Explained the undefinedness in a comment. -- To view, visit http://gerrit.cloudera.org:8080/5004 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8460bc3f7e510ce07b8673387c9440accc432abe Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple <jbapple-imp...@apache.org> Gerrit-Reviewer: Amos Bird <amosb...@gmail.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
