impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Ho (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3079: Fix sequence file writer
Date Mon, 13 Mar 2017 17:31:11 GMT
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 <attilaj@cloudera.com>
Gerrit-Reviewer: Attila Jeges <attilaj@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message