Matthew Jacobs has posted comments on this change. Change subject: Improve Kudu UPSERT test coverage ......................................................................
Patch Set 4: Code-Review+1 (4 comments) http://gerrit.cloudera.org:8080/#/c/4953/4//COMMIT_MSG Commit Message: PS4, Line 9: In preparation for the public release of Kudu integration in : 2.8, we need to make sure that we've covered as much of the : Kudu related functionality with tests as possible. let's remove this; sorry if my previous comments were confusing I think the line above covers this. http://gerrit.cloudera.org:8080/#/c/4953/4/testdata/workloads/functional-query/queries/QueryTest/kudu_upsert.test File testdata/workloads/functional-query/queries/QueryTest/kudu_upsert.test: PS4, Line 23: row_regex: .*NumModifiedRows: 1.* I realized in my own change that all of these regex checks can be simplified: we don't need the regex we can just do NumModifiedRows: 1 Can you clean these up here and below? http://gerrit.cloudera.org:8080/#/c/4953/4/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: PS4, Line 341: ssert 'DML_RESULTS' not in test_section add a comment that this isn't supported for now because __verify_results_and_errors calls verify_raw_results which always checks ERRORS, TYPES, LABELS, ... and it doesn't make sense to do so for this as well as for DML_RESULTS below. PS4, Line 357: limit 1000 Please add a comment about the limit, e.g .to make sure the queries aren't unbounded; the tests shouldn't be checking tables this big anyway -- To view, visit http://gerrit.cloudera.org:8080/4953 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib9e7afbef60186edb00a9d11fbe5a8c64931add6 Gerrit-PatchSet: 4 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
