Taras Bobrovytsky 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 t
I think it makes sense to remove tpcds-q22a. Removed it.


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?
Done


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?
Done


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?
That's how we run the normal TPCDS workload. Are you suggesting to run this one 
differently? This has to also match the TPCDS that is loaded. (For example, we 
only load snap compressed tpcds data).

Can you please clarify what you mean here?


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?
Simlar response to above. Are you suggesting to run this workload differently 
(for example with a different batch size)?


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
Done



--
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: Wed, 17 Jan 2018 02:20:18 +0000
Gerrit-HasComments: Yes

Reply via email to