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 <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-HasComments: Yes

Reply via email to