Repository: incubator-impala Updated Branches: refs/heads/master 9162d5d05 -> 457ee684c
IMPALA-3829: OpenSession() logs errors on valid configuration keys Refactored OpenSession() to process the supplied configuration map in one loop. Call SetQueryOption() on normal configuration keys only. Other changes: - Compare config keys to "impala.doas.user" in case-insensitive manner. - New E2E test to check that setting query options still works after the change. Change-Id: Ifa9b823abc39ba9809a35a6f0844fa3436f1e025 Reviewed-on: http://gerrit.cloudera.org:8080/3808 Tested-by: Internal Jenkins Reviewed-by: Michael Ho <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/457ee684 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/457ee684 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/457ee684 Branch: refs/heads/master Commit: 457ee684c8baee7ab0deff49e07c0d8dd5da370d Parents: 9162d5d Author: Attila Jeges <[email protected]> Authored: Thu Jul 28 13:41:51 2016 -0700 Committer: Michael Ho <[email protected]> Committed: Fri Aug 12 00:58:27 2016 +0000 ---------------------------------------------------------------------- be/src/service/impala-hs2-server.cc | 58 +++++++++++++++----------------- tests/hs2/test_hs2.py | 10 ++++++ 2 files changed, 37 insertions(+), 31 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/457ee684/be/src/service/impala-hs2-server.cc ---------------------------------------------------------------------- diff --git a/be/src/service/impala-hs2-server.cc b/be/src/service/impala-hs2-server.cc index d41d8da..ee79b4b 100644 --- a/be/src/service/impala-hs2-server.cc +++ b/be/src/service/impala-hs2-server.cc @@ -610,46 +610,42 @@ void ImpalaServer::OpenSession(TOpenSessionResp& return_val, state->connected_user = request.username; } + // Process the supplied configuration map. state->database = "default"; state->session_timeout = FLAGS_idle_session_timeout; - typedef map<string, string> ConfigurationMap; - for (const ConfigurationMap::value_type& v: request.configuration) { - if (iequals(v.first, "use:database")) { - state->database = v.second; - } else if (iequals(v.first, "idle_session_timeout")) { - int32_t requested_timeout = atoi(v.second.c_str()); - if (requested_timeout > 0) { - if (FLAGS_idle_session_timeout > 0) { - state->session_timeout = min(FLAGS_idle_session_timeout, requested_timeout); - } else { - state->session_timeout = requested_timeout; - } - } - VLOG_QUERY << "OpenSession(): idle_session_timeout=" - << PrettyPrinter::Print(state->session_timeout, TUnit::TIME_S); - } - } - RegisterSessionTimeout(state->session_timeout); - - // Convert request.configuration to session default query options. state->default_query_options = default_query_options_; if (request.__isset.configuration) { - map<string, string>::const_iterator conf_itr = request.configuration.begin(); - for (; conf_itr != request.configuration.end(); ++conf_itr) { - // If the current user is a valid proxy user, he/she can optionally perform - // authorization requests on behalf of another user. This is done by setting the - // 'impala.doas.user' Hive Server 2 configuration property. - if (conf_itr->first == "impala.doas.user") { - state->do_as_user = conf_itr->second; + typedef map<string, string> ConfigurationMap; + for (const ConfigurationMap::value_type& v: request.configuration) { + if (iequals(v.first, "impala.doas.user")) { + // If the current user is a valid proxy user, he/she can optionally perform + // authorization requests on behalf of another user. This is done by setting + // the 'impala.doas.user' Hive Server 2 configuration property. + state->do_as_user = v.second; Status status = AuthorizeProxyUser(state->connected_user, state->do_as_user); HS2_RETURN_IF_ERROR(return_val, status, SQLSTATE_GENERAL_ERROR); - continue; + } else if (iequals(v.first, "use:database")) { + state->database = v.second; + } else if (iequals(v.first, "idle_session_timeout")) { + int32_t requested_timeout = atoi(v.second.c_str()); + if (requested_timeout > 0) { + if (FLAGS_idle_session_timeout > 0) { + state->session_timeout = min(FLAGS_idle_session_timeout, requested_timeout); + } else { + state->session_timeout = requested_timeout; + } + } + VLOG_QUERY << "OpenSession(): idle_session_timeout=" + << PrettyPrinter::Print(state->session_timeout, TUnit::TIME_S); + } else { + // Normal configuration key. Use it to set session default query options. + // Ignore failure (failures will be logged in SetQueryOption()). + SetQueryOption(v.first, v.second, &state->default_query_options, + &state->set_query_options_mask); } - // Ignore failure to set query options (will be logged) - SetQueryOption(conf_itr->first, conf_itr->second, &state->default_query_options, - &state->set_query_options_mask); } } + RegisterSessionTimeout(state->session_timeout); TQueryOptionsToMap(state->default_query_options, &return_val.configuration); // OpenSession() should return the coordinator's HTTP server address. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/457ee684/tests/hs2/test_hs2.py ---------------------------------------------------------------------- diff --git a/tests/hs2/test_hs2.py b/tests/hs2/test_hs2.py index fbaa580..20bc9c7 100644 --- a/tests/hs2/test_hs2.py +++ b/tests/hs2/test_hs2.py @@ -35,6 +35,16 @@ class TestHS2(HS2TestSuite): open_session_req = TCLIService.TOpenSessionReq() TestHS2.check_response(self.hs2_client.OpenSession(open_session_req)) + def test_open_sesssion_query_options(self): + """Check that OpenSession sets query options""" + open_session_req = TCLIService.TOpenSessionReq() + open_session_req.configuration = {'MAX_ERRORS': '45678', + 'NUM_NODES': '1234', 'MAX_NUM_RUNTIME_FILTERS': '333'} + open_session_resp = self.hs2_client.OpenSession(open_session_req) + TestHS2.check_response(open_session_resp) + for k, v in open_session_req.configuration.items(): + assert open_session_resp.configuration[k] == v + def test_open_session_http_addr(self): """Check that OpenSession returns the coordinator's http address.""" open_session_req = TCLIService.TOpenSessionReq()
