impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5799: Kudu DML can crash if schema has changed
Date Tue, 22 Aug 2017 00:28:50 GMT
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-5799: Kudu DML can crash if schema has changed
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7688/3/be/src/exec/kudu-table-sink.cc
File be/src/exec/kudu-table-sink.cc:

PS3, Line 147: <=
'<' ?


PS3, Line 147:     if (table_->schema().num_columns() <= col_idx) {
             :       return Status(strings::Substitute(
             :           "Table $0 has fewer columns than expected.", table_desc_->name()));
             :     }
while the new test is really great new coverage, I think there are some functional test cases
we aren't covering with that test. e.g. I don't think we could have caught the off-by-one
error in the comment on l147. Obviously getting good coverage of these cases is kinda tricky.
I'm not sure yet if there's some reasonable way to do it. E.g. maybe we can inject a delay
between planning and the BE exec by making it queue in admission control.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9fd6bf164310df0041144f75f5ee722665e9f587
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message