Tim Armstrong has posted comments on this change.

Change subject: IMPALA-3337: fix "Cancelled" warnings when LIMIT clause is 
specified
......................................................................


Patch Set 1:

(4 comments)

There is some handling of abort_on_error in hbase-scanner - do we need to fix 
that too?

http://gerrit.cloudera.org:8080/#/c/2964/1/be/src/exec/hdfs-scanner.cc
File be/src/exec/hdfs-scanner.cc:

Line 651: Status HdfsScanner::CheckAbortOnError(const ErrorMsg& message) const {
The name doesn't really indicate that it logs the error. When I was reading the 
other files I was wondering if we were still logging the error.


Line 654:       message.error() == TErrorCode::MEM_LIMIT_EXCEEDED ||
Maybe explain why these two error codes are special?


http://gerrit.cloudera.org:8080/#/c/2964/1/be/src/exec/hdfs-scanner.h
File be/src/exec/hdfs-scanner.h:

Line 408: non
I had to think about this a bit but it seems correct. It's kind of confusing, 
since Status cannot have a non-OK error message, so it seemed redundant to say 
that the error message must be non-OK. It looks like we allow constructing "OK" 
ErrorMsg objects though, but I'm not sure if we do it anywhere.


http://gerrit.cloudera.org:8080/#/c/2964/1/tests/query_test/test_scanners.py
File tests/query_test/test_scanners.py:

Line 74:       limit = (iterations * 100) % 1000 + 1
What repro rate did this achieve?


-- 
To view, visit http://gerrit.cloudera.org:8080/2964
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4a91a22608e346ca21a23ea66c855eae54bbced6
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Dan Hecht <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to