Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16741 )
Change subject: IMPALA-10237: Support Bucket and Truncate partition transforms as built-in functions ...................................................................... Patch Set 13: (2 comments) I had some comments about simplifying IntToByteArray, other than that the change lgtm. http://gerrit.cloudera.org:8080/#/c/16741/13/be/src/util/bit-util.h File be/src/util/bit-util.h: http://gerrit.cloudera.org:8080/#/c/16741/13/be/src/util/bit-util.h@355 PS13, Line 355: void Maybe we could just return the vector. http://gerrit.cloudera.org:8080/#/c/16741/12/be/src/util/bit-util.h File be/src/util/bit-util.h: http://gerrit.cloudera.org:8080/#/c/16741/12/be/src/util/bit-util.h@357 PS12, Line 357: T value = input; > This is needed because negative numbers are stored in two's complement form I think we don't need the negation at L357 and the multiplication with -1 at L361 because the right shift introduces ones on the left for negative numbers. I tested the following algorithm and it worked well: template<typename T> vector<char> IntToByteArray(T input) { vector<char> ret; T value = input; for (int i = 0; i < sizeof(value); ++i) { // Applies a mask for a byte range on the input. char value_to_save = value & 0XFF; ret.push_back(value_to_save); value >>= 8; if (value == 0 && value_to_save >= 0) break; if (value == -1 && value_to_save < 0) break; } std::reverse(ret.begin(), ret.end()); return ret; } -- To view, visit http://gerrit.cloudera.org:8080/16741 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I485680cf79d96d578dd8cfbfd554bec468fe84bd Gerrit-Change-Number: 16741 Gerrit-PatchSet: 13 Gerrit-Owner: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Wed, 16 Dec 2020 10:50:53 +0000 Gerrit-HasComments: Yes
