Michael Ho has posted comments on this change. Change subject: IMPALA-3079: Fix sequence file writer ......................................................................
Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/6107/1/be/src/exec/read-write-util.h File be/src/exec/read-write-util.h: PS1, Line 240: buf[i] = (val >> (8 * (num_bytes - i - 1))) & 0xFF; : } > Yes, it is part of the sequence file standard and it can be read back by Hi Thanks for the pointers. Would you mind adding the definitionamd the URLs above as function header: Serializes a long to a binary stream with zero-compressed encoding. For -112 <= i <= 127, only one byte is used with the actual value. For other values of i, the first byte value indicates whether the long is positive or negative, and the number of bytes that follow. If the first byte value v is between -113 and -120, the following long is positive, with number of bytes that follow are -(v+112). If the first byte value v is between -121 and -128, the following long is negative, with number of bytes that follow are -(v+120). Bytes are stored in the high-non-zero-byte-first order. http://gerrit.cloudera.org:8080/#/c/6107/3/be/src/exec/read-write-util.h File be/src/exec/read-write-util.h: PS3, Line 215: // returns size of the encoded long value, including the 1 byte for length nit: consider putting it at line 213 instead, as a function header. PS3, Line 218: (9 - __builtin_clzll(val)/8); nit: unnecessary parenthesis. PS3, Line 233: val = ~val; May also want to state that the source of this behavior. -- To view, visit http://gerrit.cloudera.org:8080/6107 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0db642ad35132a9a5a6611810a6cafbbe26e7487 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-HasComments: Yes
