Tim Armstrong has posted comments on this change. Change subject: IMPALA-3729: batch_size=1 coverage for avro scanner ......................................................................
Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/3413/2/tests/common/test_result_verifier.py File tests/common/test_result_verifier.py: PS2, Line 12: \ > I don't think this is needed, put these in round brackets. Similar to: http I didn't know you could do that. I guess it makes sense that it's a tuple. Line 202: raise Exception("Duplicate rows in expected results: %s" % str(expected_results.rows)) > If you added the comment saying that duplicates are ignored, does it still My thought was to prevent mistakes in writing test files. E.g. if someone writes a RESULTS: VERIFY_IS_SUBSET section with duplicate rows, it's almost certainly a mistake, since the duplicate rows will be ignored. I'm ok with removing if you think it's unnecessary. Line 213: actual_set = set(map(str, actual_results.rows)) > This code looks very similar to verify_query_result_is_subset, only the ass Good point - done. -- To view, visit http://gerrit.cloudera.org:8080/3413 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia007e558d92c7e4ce30be90446fdbb1f50a0ebc4 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
