Michael Ho has posted comments on this change. Change subject: IMPALA-1619: Support 64-bit allocations. ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/2781/1/be/src/exec/delimited-text-parser.inline.h File be/src/exec/delimited-text-parser.inline.h: Line 58: // Only support up to 1GB in length for a slot. Truncate if necessary. > Yeah I'm a little concerned about silently truncating data - not sure if th In the old code, if len >= 2GB, we will have negative length (which may be incorrectly treated as the case with escape character) so it's unclear what the proper behavior for the old code is either. May be it's more sane to check against std::numeric_limits<int>::max() instead ? What do you think ? http://gerrit.cloudera.org:8080/#/c/2781/1/be/src/runtime/string-buffer.h File be/src/runtime/string-buffer.h: Line 67: buffer_size_ = 0; > Will you add a comment to explain this? Not your change but it would be goo Sure. buffer_ is set to NULL in the new patch so we know we cannot free the old buffer. -- To view, visit http://gerrit.cloudera.org:8080/2781 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7ed28083d809a86d801a9c063a0aa32c50d32b20 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
