impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-2399: Memory limit checks for all scanners.
Date Mon, 21 Mar 2016 22:56:07 GMT
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 <kwho@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message