impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3713,IMPALA-4439: Fix Kudu DML shell reporting
Date Wed, 16 Nov 2016 04:56:55 GMT
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 <mj@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message