Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21153 )
Change subject: IMPALA-12913: Refactor Workload Management Custom Cluser Tests ...................................................................... Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/21153/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21153/1//COMMIT_MSG@7 PS1, Line 7: IMPALA-12913: Refactor Workload Management Custom Cluser Tests > typo in the commit message: "Cluster" Done http://gerrit.cloudera.org:8080/#/c/21153/1/testdata/datasets/functional/functional_schema_template.sql File testdata/datasets/functional/functional_schema_template.sql: http://gerrit.cloudera.org:8080/#/c/21153/1/testdata/datasets/functional/functional_schema_template.sql@1804 PS1, Line 1804: CREATE EXTERNAL TABLE IF NOT EXISTS {db_name}{db_suffix}.{table_name} ( > Agree, using COLUMNS section might be better. No reason, just new to this file. Updated to use the existing features to avoid the create table. http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py File tests/custom_cluster/test_query_log.py: http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py@44 PS1, Line 44: super(TestQueryLogTableBase, cls).add_test_dimensions() > Add the following after L44 Done http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py@438 PS1, Line 438: client = self.create_impala_client(protocol=vector.get_value('protocol')) > ImpalaTestSuite and its subclasses initialize 3 clients for each test: clie I switched to using the default clients to avoid the unnecessary creation of extra clients. A few of the tests do use a second client to ensure that different coordinators can see the rows in the completed queries table, and those clients are created/closed explicitly. http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py@614 PS1, Line 614: @pytest.mark.parametrize("buffer_pool_limit", [None, "14.97MB"]) : def test_query_log_table_query_select(self, vector, buffer_pool_limit): > Contain this test in its own test class, and define buffer_pool_limit test Done http://gerrit.cloudera.org:8080/#/c/21153/1/tests/custom_cluster/test_query_log.py@635 PS1, Line 635: client.set_configuration_option("SPOOL_QUERY_RESULTS", "TRUE") > Can probably remove this, since it is True by default. https://impala.apache.org/docs/build/html/topics/impala_spool_query_results.html says default is false. Code defaults it to true: https://github.com/apache/impala/blob/1e67480ba6cd4a6f01b369eb11cc2b7330554bde/common/thrift/Query.thrift#L411 Filed https://issues.apache.org/jira/browse/IMPALA-12932 to determine how to handle this discrepency. I'm going to leave this line in here since I'm not sure what is going to happen with the spool_query_results options (if the default will be changed or if the docs will be changed). -- To view, visit http://gerrit.cloudera.org:8080/21153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e3249a8f306cf43de0d6f6586711c779399e83b Gerrit-Change-Number: 21153 Gerrit-PatchSet: 2 Gerrit-Owner: Jason Fehr <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Thu, 21 Mar 2024 18:43:17 +0000 Gerrit-HasComments: Yes
