Taras Bobrovytsky 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://github.mtv.cloudera.com/CDH/Impala/blob/a8f3c56fc6fff973046869d49429f5a6fe76796c/tests/comparison/data_generator.py#L37 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 make sense to throw this exception? Another alternative is to modify the comment above and say that any duplicates in the actual results are ignored. Would that still be considered set semantics? Line 213: actual_set = set(map(str, actual_results.rows)) This code looks very similar to verify_query_result_is_subset, only the assert at end is different. Do you think it would make sense to combine it somehow? -- 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
