Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/8985 )
Change subject: IMPALA-5478: Run TPCDS queries with decimal_v2 enabled ...................................................................... Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/8985/2/testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22a.test File testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22a.test: http://gerrit.cloudera.org:8080/#/c/8985/2/testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22a.test@2 PS2, Line 2: ---- QUERY: TPCDS-Q22A Do we just straight up remove tpcds-q22a.test then? It was set previously to force decimal_v2, directly in the .test file. It was my understanding the query wouldn't work without decimal_v2. http://gerrit.cloudera.org:8080/#/c/8985/2/testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22a.test@5 PS2, Line 5: set decimal_v2=1; Should this be removed? http://gerrit.cloudera.org:8080/#/c/8985/2/tests/query_test/test_tpcds_queries.py File tests/query_test/test_tpcds_queries.py: http://gerrit.cloudera.org:8080/#/c/8985/2/tests/query_test/test_tpcds_queries.py@108 PS2, Line 108: def test_tpcds_q22a(self, vector): : self.run_test_case(self.get_workload() + '-q22a', vector) Remove? http://gerrit.cloudera.org:8080/#/c/8985/2/tests/query_test/test_tpcds_queries.py@279 PS2, Line 279: cls.ImpalaTestMatrix.add_constraint(lambda v:\ : v.get_value('table_format').file_format not in ['rc', 'hbase', 'kudu'] and\ : v.get_value('table_format').compression_codec in ['none', 'snap'] and\ : v.get_value('table_format').compression_type != 'record') Are these lines necessary? http://gerrit.cloudera.org:8080/#/c/8985/2/tests/query_test/test_tpcds_queries.py@285 PS2, Line 285: if cls.exploration_strategy() != 'exhaustive': : # Cut down on the execution time for these tests in core by running only : # against parquet. : cls.ImpalaTestMatrix.add_constraint(lambda v:\ : v.get_value('table_format').file_format in ['parquet']) : : cls.ImpalaTestMatrix.add_constraint(lambda v:\ : v.get_value('exec_option')['batch_size'] == 0) Are these lines necessary? http://gerrit.cloudera.org:8080/#/c/8985/2/tests/query_test/test_tpcds_queries.py@294 PS2, Line 294: @pytest.mark.execute_serially : # Marked serially to make sure it runs first. : def test_tpcds_count(self, vector): : self.run_test_case('count', vector) This seems a redundant test with the one in TestTpcdsQuery. Is it necessary to run it here? It doesn't seem like it to me. -- To view, visit http://gerrit.cloudera.org:8080/8985 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib867c51a521ec4a087bc127d99aee4b95ba97733 Gerrit-Change-Number: 8985 Gerrit-PatchSet: 2 Gerrit-Owner: Taras Bobrovytsky <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-Reviewer: Zach Amsden <[email protected]> Gerrit-Comment-Date: Tue, 16 Jan 2018 17:28:18 +0000 Gerrit-HasComments: Yes
