Alex Behm has posted comments on this change.

Change subject: IMPALA-3713,IMPALA-4439: Fix Kudu DML shell reporting
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5103/1/be/src/service/impala-beeswax-server.cc
File be/src/service/impala-beeswax-server.cc:

Line 532:         insert_result->__set_num_row_errors(num_row_errors);
Can we leave this unset for DML where reporting the num_row_errors doesn't make 
sense? Based on the code here, I think we'll report them for all DML (even 
non-Kudu), right?


http://gerrit.cloudera.org:8080/#/c/5103/1/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:

Line 260:   // Number of rows that weren't modified due to errors. Only applies 
to Kudu tables.
Can you qualify errors more precisely? These are constraint violations correct, 
and not really Kudu "errors", right?


http://gerrit.cloudera.org:8080/#/c/5103/1/shell/impala_shell.py
File shell/impala_shell.py:

Line 928:         errors_stmt = ", %d row error(s)" % (num_row_errors)
Let's avoid "stmt" since that is a somewhat overloaded abbreviation (aka SQL 
statement). error_report?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3d3d7aa8d176e03ea58fb00f2a81fb3e34965aa1
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-HasComments: Yes

Reply via email to