Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13907 )
Change subject: IMPALA-8781: Result spooling tests to cover edge cases and cancellation ...................................................................... Patch Set 10: (5 comments) Just a few nits about the test dimensions after I saw your last comment. Specifying the database manually to match the test dimension seems fine. I forget the details, but I think it may only automatically switch databases to match the workload and dimension when running .test fiels. http://gerrit.cloudera.org:8080/#/c/13907/10/tests/query_test/test_result_spooling.py File tests/query_test/test_result_spooling.py: http://gerrit.cloudera.org:8080/#/c/13907/10/tests/query_test/test_result_spooling.py@26 PS10, Line 26: tpch tpch_parquet? http://gerrit.cloudera.org:8080/#/c/13907/10/tests/query_test/test_result_spooling.py@39 PS10, Line 39: v.get_value('table_format').file_format == 'parquet') Do we want to run it for all file formats in exhaustive? I think it makes sense just to run it on parquet. http://gerrit.cloudera.org:8080/#/c/13907/10/tests/query_test/test_result_spooling.py@46 PS10, Line 46: self.run_test_case('QueryTest/result-spooling', vector) Most or all of the queries explicitly specify "functional", which is text, which doesn't line up with the test dimensions. This is kinda confusing. I'd suggest either using functional_parquet or just leaving the database off (that should work in the .test files, at least). http://gerrit.cloudera.org:8080/#/c/13907/10/tests/query_test/test_result_spooling.py@58 PS10, Line 58: result = self.execute_query(query, exec_options) Maybe make a copy of exec_options, it's a little surprising that this function modifies its argument. http://gerrit.cloudera.org:8080/#/c/13907/10/tests/query_test/test_result_spooling.py@96 PS10, Line 96: v.get_value('table_format').file_format == 'parquet' and The same nit applies here that you've specified parquet but are querying text tables. -- To view, visit http://gerrit.cloudera.org:8080/13907 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib3b3a1539c4a5fa9b43c8ca315cea16c9701e283 Gerrit-Change-Number: 13907 Gerrit-PatchSet: 10 Gerrit-Owner: Sahil Takiar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Sun, 04 Aug 2019 21:25:25 +0000 Gerrit-HasComments: Yes
