Revert "IMPALA-5589: change "set" in impala-shell to show empty string for unset query options"
Due to re-use of connections in the test infrastructure, this commit is causing ASAN failures in the builds. That is being worked out as part of IMPALA-5908, but, in the meanwhile, it's prudent to revert. This reverts commit 387bde0639ffd8ef580ccbf727152954e62bacbe. Change-Id: I41bc8ab0f1df45bbd311030981a7c18989c2edc8 Reviewed-on: http://gerrit.cloudera.org:8080/8087 Reviewed-by: Dan Hecht <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/f0e79314 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/f0e79314 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/f0e79314 Branch: refs/heads/master Commit: f0e79314fe9d9d3e920ad65c3ca6a4ef279e68fc Parents: 5119ced Author: Philip Zeyliger <[email protected]> Authored: Fri Sep 15 16:55:02 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Sat Sep 16 04:06:53 2017 +0000 ---------------------------------------------------------------------- be/src/service/query-options-test.cc | 12 --- be/src/service/query-options.cc | 10 +- be/src/service/query-options.h | 4 +- .../functional-query/queries/QueryTest/set.test | 24 ++--- tests/hs2/hs2_test_suite.py | 10 +- tests/hs2/test_hs2.py | 104 ++++++++----------- tests/shell/test_shell_commandline.py | 2 - 7 files changed, 60 insertions(+), 106 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f0e79314/be/src/service/query-options-test.cc ---------------------------------------------------------------------- diff --git a/be/src/service/query-options-test.cc b/be/src/service/query-options-test.cc index 1bec770..397df5a 100644 --- a/be/src/service/query-options-test.cc +++ b/be/src/service/query-options-test.cc @@ -135,16 +135,4 @@ TEST(QueryOptions, ParseQueryOptions) { } } -TEST(QueryOptions, MapOptionalDefaultlessToEmptyString) { - TQueryOptions options; - map<string, string> map; - - TQueryOptionsToMap(options, &map); - // No default set - EXPECT_EQ(map["COMPRESSION_CODEC"], ""); - EXPECT_EQ(map["MT_DOP"], ""); - // Has defaults - EXPECT_EQ(map["EXPLAIN_LEVEL"], "1"); -} - IMPALA_TEST_MAIN(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f0e79314/be/src/service/query-options.cc ---------------------------------------------------------------------- diff --git a/be/src/service/query-options.cc b/be/src/service/query-options.cc index e950d31..21327f3 100644 --- a/be/src/service/query-options.cc +++ b/be/src/service/query-options.cc @@ -67,13 +67,9 @@ void impala::TQueryOptionsToMap(const TQueryOptions& query_options, map<string, string>* configuration) { #define QUERY_OPT_FN(NAME, ENUM)\ {\ - if (query_options.__isset.NAME) { \ - stringstream val;\ - val << query_options.NAME;\ - (*configuration)[#ENUM] = val.str();\ - } else { \ - (*configuration)[#ENUM] = ""; \ - }\ + stringstream val;\ + val << query_options.NAME;\ + (*configuration)[#ENUM] = val.str();\ } QUERY_OPTS_TABLE #undef QUERY_OPT_FN http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f0e79314/be/src/service/query-options.h ---------------------------------------------------------------------- diff --git a/be/src/service/query-options.h b/be/src/service/query-options.h index 3dada7d..bb8c301 100644 --- a/be/src/service/query-options.h +++ b/be/src/service/query-options.h @@ -99,9 +99,7 @@ class TQueryOptions; ; -/// Converts a TQueryOptions struct into a map of key, value pairs. Options that -/// aren't set and lack defaults in common/thrift/ImpalaInternalService.thrift are -/// mapped to the empty string. +/// Converts a TQueryOptions struct into a map of key, value pairs. void TQueryOptionsToMap(const TQueryOptions& query_options, std::map<std::string, std::string>* configuration); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f0e79314/testdata/workloads/functional-query/queries/QueryTest/set.test ---------------------------------------------------------------------- diff --git a/testdata/workloads/functional-query/queries/QueryTest/set.test b/testdata/workloads/functional-query/queries/QueryTest/set.test index 45c8343..a7004e0 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/set.test +++ b/testdata/workloads/functional-query/queries/QueryTest/set.test @@ -20,13 +20,13 @@ set 'MEM_LIMIT','0' 'NUM_NODES','0' 'NUM_SCANNER_THREADS','0' -'COMPRESSION_CODEC','' +'COMPRESSION_CODEC','NONE' 'PARQUET_FILE_SIZE','0' 'REQUEST_POOL','' -'RESERVATION_REQUEST_TIMEOUT','' +'RESERVATION_REQUEST_TIMEOUT','0' 'RM_INITIAL_MEM','0' 'SYNC_DDL','0' -'V_CPU_CORES','' +'V_CPU_CORES','0' ---- TYPES STRING, STRING ==== @@ -52,13 +52,13 @@ set; 'MEM_LIMIT','0' 'NUM_NODES','0' 'NUM_SCANNER_THREADS','0' -'COMPRESSION_CODEC','' +'COMPRESSION_CODEC','NONE' 'PARQUET_FILE_SIZE','0' 'REQUEST_POOL','' -'RESERVATION_REQUEST_TIMEOUT','' +'RESERVATION_REQUEST_TIMEOUT','0' 'RM_INITIAL_MEM','0' 'SYNC_DDL','0' -'V_CPU_CORES','' +'V_CPU_CORES','0' ---- TYPES STRING, STRING ==== @@ -84,13 +84,13 @@ set; 'MEM_LIMIT','0' 'NUM_NODES','0' 'NUM_SCANNER_THREADS','0' -'COMPRESSION_CODEC','' +'COMPRESSION_CODEC','NONE' 'PARQUET_FILE_SIZE','0' 'REQUEST_POOL','' -'RESERVATION_REQUEST_TIMEOUT','' +'RESERVATION_REQUEST_TIMEOUT','0' 'RM_INITIAL_MEM','0' 'SYNC_DDL','0' -'V_CPU_CORES','' +'V_CPU_CORES','0' ---- TYPES STRING, STRING ==== @@ -117,13 +117,13 @@ set; 'MEM_LIMIT','0' 'NUM_NODES','0' 'NUM_SCANNER_THREADS','0' -'COMPRESSION_CODEC','' +'COMPRESSION_CODEC','NONE' 'PARQUET_FILE_SIZE','1610612736' 'REQUEST_POOL','' -'RESERVATION_REQUEST_TIMEOUT','' +'RESERVATION_REQUEST_TIMEOUT','0' 'RM_INITIAL_MEM','0' 'SYNC_DDL','0' -'V_CPU_CORES','' +'V_CPU_CORES','0' ---- TYPES STRING, STRING ==== http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f0e79314/tests/hs2/hs2_test_suite.py ---------------------------------------------------------------------- diff --git a/tests/hs2/hs2_test_suite.py b/tests/hs2/hs2_test_suite.py index 1b2f89f..2c2cd51 100644 --- a/tests/hs2/hs2_test_suite.py +++ b/tests/hs2/hs2_test_suite.py @@ -124,6 +124,7 @@ class HS2TestSuite(ImpalaTestSuite): fetch_results_req.maxRows = size fetch_results_resp = self.hs2_client.FetchResults(fetch_results_req) HS2TestSuite.check_response(fetch_results_resp) + num_rows = size if expected_num_rows is not None: assert self.get_num_rows(fetch_results_resp.results) == expected_num_rows return fetch_results_resp @@ -227,12 +228,3 @@ class HS2TestSuite(ImpalaTestSuite): sleep(interval) assert False, 'Did not reach expected operation state %s in time, actual state was ' \ '%s' % (expected_state, get_operation_status_resp.operationState) - - def execute_statement(self, statement): - """Executes statement and returns response, which is checked.""" - execute_statement_req = TCLIService.TExecuteStatementReq() - execute_statement_req.sessionHandle = self.session_handle - execute_statement_req.statement = statement - execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req) - HS2TestSuite.check_response(execute_statement_resp) - return execute_statement_resp http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f0e79314/tests/hs2/test_hs2.py ---------------------------------------------------------------------- diff --git a/tests/hs2/test_hs2.py b/tests/hs2/test_hs2.py index 2f984b1..f42b29a 100644 --- a/tests/hs2/test_hs2.py +++ b/tests/hs2/test_hs2.py @@ -45,48 +45,6 @@ class TestHS2(HS2TestSuite): for k, v in open_session_req.configuration.items(): assert open_session_resp.configuration[k] == v - @needs_session() - def test_session_options_via_set(self): - """ - Tests that session options are returned by a SET - query and can be updated by a "SET k=v" query. - """ - def get_session_options(): - """Returns dictionary of query options.""" - execute_statement_resp = self.execute_statement("SET") - - fetch_results_req = TCLIService.TFetchResultsReq() - fetch_results_req.operationHandle = execute_statement_resp.operationHandle - fetch_results_req.maxRows = 1000 - fetch_results_resp = self.hs2_client.FetchResults(fetch_results_req) - TestHS2.check_response(fetch_results_resp) - - # Close the query - close_operation_req = TCLIService.TCloseOperationReq() - close_operation_req.operationHandle = execute_statement_resp.operationHandle - TestHS2.check_response(self.hs2_client.CloseOperation(close_operation_req)) - - # Results are returned in a columnar way: - cols = fetch_results_resp.results.columns - assert len(cols) == 2 - vals = dict(zip(cols[0].stringVal.values, cols[1].stringVal.values)) - return vals - - vals = get_session_options() - - # No default; should be empty string. - assert vals["COMPRESSION_CODEC"] == "" - # Has default of 0 - assert vals["SYNC_DDL"] == "0" - - # Set an option using SET - self.execute_statement("SET COMPRESSION_CODEC=gzip") - - vals2 = get_session_options() - assert vals2["COMPRESSION_CODEC"] == "GZIP" - # Should be unchanged - assert vals2["SYNC_DDL"] == "0" - def test_open_session_http_addr(self): """Check that OpenSession returns the coordinator's http address.""" open_session_req = TCLIService.TOpenSessionReq() @@ -214,8 +172,11 @@ class TestHS2(HS2TestSuite): @needs_session() def test_get_operation_status(self): """Tests that GetOperationStatus returns a valid result for a running query""" - statement = "SELECT COUNT(*) FROM functional.alltypes" - execute_statement_resp = self.execute_statement(statement) + execute_statement_req = TCLIService.TExecuteStatementReq() + execute_statement_req.sessionHandle = self.session_handle + execute_statement_req.statement = "SELECT COUNT(*) FROM functional.alltypes" + execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req) + TestHS2.check_response(execute_statement_resp) get_operation_status_resp = \ self.get_operation_status(execute_statement_resp.operationHandle) @@ -252,8 +213,11 @@ class TestHS2(HS2TestSuite): def test_get_operation_status_error(self): """Tests that GetOperationStatus returns a valid result for a query that encountered an error""" - statement = "SELECT * FROM functional.alltypeserror" - execute_statement_resp = self.execute_statement(statement) + execute_statement_req = TCLIService.TExecuteStatementReq() + execute_statement_req.sessionHandle = self.session_handle + execute_statement_req.statement = "SELECT * FROM functional.alltypeserror" + execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req) + TestHS2.check_response(execute_statement_resp) get_operation_status_resp = self.wait_for_operation_state( \ execute_statement_resp.operationHandle, TCLIService.TOperationState.ERROR_STATE) @@ -368,15 +332,17 @@ class TestHS2(HS2TestSuite): assert "Sql Statement: GET_SCHEMAS" in profile_page assert "Query Type: DDL" in profile_page - @needs_session(conf_overlay={"idle_session_timeout": "5"}) def test_get_operation_status_session_timeout(self): """Regression test for IMPALA-4488: GetOperationStatus() would not keep a session alive""" + execute_statement_req = TCLIService.TExecuteStatementReq() + execute_statement_req.sessionHandle = self.session_handle # Choose a long-running query so that it can't finish before the session timeout. - statement = """select * from functional.alltypes a + execute_statement_req.statement = """select * from functional.alltypes a join functional.alltypes b join functional.alltypes c""" - execute_statement_resp = self.execute_statement(statement) + execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req) + TestHS2.check_response(execute_statement_resp) now = time.time() # Loop until the session would be timed-out if IMPALA-4488 had not been fixed. @@ -388,7 +354,11 @@ class TestHS2(HS2TestSuite): time.sleep(0.1) def get_log(self, query_stmt): - execute_statement_resp = self.execute_statement(query_stmt) + execute_statement_req = TCLIService.TExecuteStatementReq() + execute_statement_req.sessionHandle = self.session_handle + execute_statement_req.statement = query_stmt + execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req) + TestHS2.check_response(execute_statement_resp) # Fetch results to make sure errors are generated. Errors are only guaranteed to be # seen by the coordinator after FetchResults() returns eos. @@ -419,8 +389,11 @@ class TestHS2(HS2TestSuite): @needs_session() def test_get_exec_summary(self): - statement = "SELECT COUNT(1) FROM functional.alltypes" - execute_statement_resp = self.execute_statement(statement) + execute_statement_req = TCLIService.TExecuteStatementReq() + execute_statement_req.sessionHandle = self.session_handle + execute_statement_req.statement = "SELECT COUNT(1) FROM functional.alltypes" + execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req) + TestHS2.check_response(execute_statement_resp) exec_summary_req = ImpalaHiveServer2Service.TGetExecSummaryReq() exec_summary_req.operationHandle = execute_statement_resp.operationHandle @@ -442,15 +415,18 @@ class TestHS2(HS2TestSuite): @needs_session() def test_get_profile(self): - statement = "SELECT COUNT(2) FROM functional.alltypes" - execute_statement_resp = self.execute_statement(statement) + execute_statement_req = TCLIService.TExecuteStatementReq() + execute_statement_req.sessionHandle = self.session_handle + execute_statement_req.statement = "SELECT COUNT(2) FROM functional.alltypes" + execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req) + TestHS2.check_response(execute_statement_resp) get_profile_req = ImpalaHiveServer2Service.TGetRuntimeProfileReq() get_profile_req.operationHandle = execute_statement_resp.operationHandle get_profile_req.sessionHandle = self.session_handle get_profile_resp = self.hs2_client.GetRuntimeProfile(get_profile_req) TestHS2.check_response(get_profile_resp) - assert statement in get_profile_resp.profile + assert execute_statement_req.statement in get_profile_resp.profile # If ExecuteStatement() has completed but the results haven't been fetched yet, the # query must have at least reached RUNNING. assert "Query State: RUNNING" in get_profile_resp.profile or \ @@ -463,7 +439,7 @@ class TestHS2(HS2TestSuite): get_profile_resp = self.hs2_client.GetRuntimeProfile(get_profile_req) TestHS2.check_response(get_profile_resp) - assert statement in get_profile_resp.profile + assert execute_statement_req.statement in get_profile_resp.profile # After fetching the results, we must be in state FINISHED. assert "Query State: FINISHED" in get_profile_resp.profile, get_profile_resp.profile @@ -473,20 +449,26 @@ class TestHS2(HS2TestSuite): get_profile_resp = self.hs2_client.GetRuntimeProfile(get_profile_req) TestHS2.check_response(get_profile_resp) - assert statement in get_profile_resp.profile + assert execute_statement_req.statement in get_profile_resp.profile assert "Query State: FINISHED" in get_profile_resp.profile, get_profile_resp.profile @needs_session(conf_overlay={"use:database": "functional"}) def test_change_default_database(self): - statement = "SELECT 1 FROM alltypes LIMIT 1" + execute_statement_req = TCLIService.TExecuteStatementReq() + execute_statement_req.sessionHandle = self.session_handle + execute_statement_req.statement = "SELECT 1 FROM alltypes LIMIT 1" + execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req) # Will fail if there's no table called 'alltypes' in the database - self.execute_statement(statement) + TestHS2.check_response(execute_statement_resp) @needs_session(conf_overlay={"use:database": "FUNCTIONAL"}) def test_change_default_database_case_insensitive(self): - statement = "SELECT 1 FROM alltypes LIMIT 1" + execute_statement_req = TCLIService.TExecuteStatementReq() + execute_statement_req.sessionHandle = self.session_handle + execute_statement_req.statement = "SELECT 1 FROM alltypes LIMIT 1" + execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req) # Will fail if there's no table called 'alltypes' in the database - self.execute_statement(statement) + TestHS2.check_response(execute_statement_resp) @needs_session(conf_overlay={"use:database": "doesnt-exist"}) def test_bad_default_database(self): http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/f0e79314/tests/shell/test_shell_commandline.py ---------------------------------------------------------------------- diff --git a/tests/shell/test_shell_commandline.py b/tests/shell/test_shell_commandline.py index 0602e77..488de49 100644 --- a/tests/shell/test_shell_commandline.py +++ b/tests/shell/test_shell_commandline.py @@ -248,8 +248,6 @@ class TestImpalaShell(ImpalaTestSuite): assert 'MEM_LIMIT: [0]' in result_set.stdout # test to check that explain_level is 1 assert 'EXPLAIN_LEVEL: [1]' in result_set.stdout - # test to check that configs without defaults show up as [] - assert 'COMPRESSION_CODEC: []' in result_set.stdout # test values displayed after setting value args = '-q "set mem_limit=1g;set"' result_set = run_impala_shell_cmd(args)
