Dan Hecht has posted comments on this change. Change subject: IMPALA-2399: Memory limit checks for all scanners. ......................................................................
Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/2568/1/be/src/exec/hdfs-avro-scanner.h File be/src/exec/hdfs-avro-scanner.h: Line 181: /// comments below for ReadAvroChar(). indicate parse_status_ is set with the error Line 236: /// new char buffer. It returns true otherwise. indicate that parse_status_ is set with the error http://gerrit.cloudera.org:8080/#/c/2568/1/be/src/exec/hdfs-scanner.cc File be/src/exec/hdfs-scanner.cc: Line 185: RETURN_IF_ERROR(state_->GetQueryStatus()); how about leaving a comment until it's renamed: // Check for UDF errors. (assuming this is the only remaining reason we need it, is that right?) http://gerrit.cloudera.org:8080/#/c/2568/1/be/src/exec/text-converter.inline.h File be/src/exec/text-converter.inline.h: Line 71: // In other words, 'copy_string' and 'need_escape' will always be false. What about the type.IsVarLenStringType() false case? And what's the significance of this comment? Does the code depend on not taking this path in codegen for correctness in some way? http://gerrit.cloudera.org:8080/#/c/2568/1/be/src/runtime/string-buffer.h File be/src/runtime/string-buffer.h: Line 106: $0 does this work for normal string? I thought this was Substitute() magic. -- To view, visit http://gerrit.cloudera.org:8080/2568 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3e5a56501967a58513888917db5ce66dec4fb5ce Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-HasComments: Yes
