Jim Apple has posted comments on this change. Change subject: IMPALA-2107: Add Base64 encoder/decoder ......................................................................
Patch Set 4: (8 comments) http://gerrit.cloudera.org:8080/#/c/2633/4/be/src/exprs/string-functions.cc File be/src/exprs/string-functions.cc: Line 799: characters > how about saying "bytes" here to help differentiate between input (which ca Done Line 817: AnyValUtil::ToString(str) > Might be really long. I'm a bit worried about printing the whole thing. wha Done Line 829: characters > likewise, how about saying "bytes" here. Done Line 830: > (which represents 6-bits) Done Line 831: . > (for a total of 8 bits, so there is one less byte of output). Done Line 832: character > byte Done Line 836: ss << "Invalid base64 string \"" << AnyValUtil::ToString(str) << "\""; > maybe be more specific about the reason (length not a multiple of 4). Also, Done Line 854: AnyValUtil::ToString(str) > same thing. this might be really long, right? Done -- To view, visit http://gerrit.cloudera.org:8080/2633 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I911451c5d68e8ae9d352abfcf4d5ff36484f0bf3 Gerrit-PatchSet: 4 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Jim Apple <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: Yes
