IMPALA-5908: Allow SET to unset modified query options. The query 'SET <option>=""' will now unset an option within the session, reverting it to its default state.
This change became necessary when "SET" started returning an empty string for unset options which don't have a default. The test infrastructure (impala_test_suite.py) resets options to what it thinks is its defaults, and, when this broke, some ASAN builds started to fail, presumably due to a timing issue with how we re-use connections between tests. Previously, SessionState copied over the default options from the server when the session was created and then mutated that. To support unsetting options at the session layer, this change keeps a pointer to the default server settings, keeps separately the mutations, and overlays the options each time they're requested. Similarly, for configuration overlays that happen per-query, the overlay is now done explicitly, because empty per-query overlay values (key=..., value="") now have no effect. Because "set key=''" is ambiguous between "set to the empty string" and "unset", it's now impossible to set to the empty string, at the session layer, an option that is configured at a previous layer. In practice, this is just debug_action and request_pool. debug_action is essentially an internal tool. For request_pool, this means that setting the default request_pool via impalad command line is now a bad idea, as it can't be cleared at a per-session level. For request_pool, the correct course of action for users is to use placement rules, and to have a default placement rule. Testing: * Added a simple test that triggered this side-effect without this code. Specifically, "impala-python infra/python/env/bin/py.test tests/metadata/test_set.py -s" with the modified set.test triggers. * Amended tests/custom_cluster/test_admission_controller.py; it was useful for testing these code paths. * Added cases to query-options-test to check behavior for both defaulted and non-defaulted values. * Added a custom cluster test that checks that overlays are working against * Ran an ASAN build where this was triggering previously. Change-Id: Ia8c383e68064f839cb5000118901dff77b4e5cb9 Reviewed-on: http://gerrit.cloudera.org:8080/8070 Reviewed-by: Tim Armstrong <[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/c9740b43 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/c9740b43 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/c9740b43 Branch: refs/heads/master Commit: c9740b43d1493a8249ad7497430e5bfbcc6ebf64 Parents: 226c99e Author: Philip Zeyliger <[email protected]> Authored: Mon Sep 11 15:50:16 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Thu Oct 5 03:04:38 2017 +0000 ---------------------------------------------------------------------- be/src/service/client-request-state.cc | 4 +- be/src/service/impala-beeswax-server.cc | 9 +- be/src/service/impala-hs2-server.cc | 14 ++- be/src/service/impala-server.cc | 8 +- be/src/service/impala-server.h | 16 +++- be/src/service/query-options-test.cc | 67 +++++++++++++ be/src/service/query-options.cc | 25 ++++- be/src/service/query-options.h | 2 +- common/thrift/ImpalaInternalService.thrift | 19 +++- .../functional-query/queries/QueryTest/set.test | 6 ++ tests/common/impala_test_suite.py | 2 +- .../custom_cluster/test_admission_controller.py | 18 +++- tests/custom_cluster/test_set_and_unset.py | 98 ++++++++++++++++++++ tests/hs2/hs2_test_suite.py | 4 +- 14 files changed, 266 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c9740b43/be/src/service/client-request-state.cc ---------------------------------------------------------------------- diff --git a/be/src/service/client-request-state.cc b/be/src/service/client-request-state.cc index ef6a69d..cb542d0 100644 --- a/be/src/service/client-request-state.cc +++ b/be/src/service/client-request-state.cc @@ -199,14 +199,14 @@ Status ClientRequestState::Exec(TExecRequest* exec_request) { RETURN_IF_ERROR(SetQueryOption( exec_request_.set_query_option_request.key, exec_request_.set_query_option_request.value, - &session_->default_query_options, + &session_->set_query_options, &session_->set_query_options_mask)); SetResultSet({}, {}); } else { // "SET" returns a table of all query options. map<string, string> config; TQueryOptionsToMap( - session_->default_query_options, &config); + session_->QueryOptions(), &config); vector<string> keys, values; map<string, string>::const_iterator itr = config.begin(); for (; itr != config.end(); ++itr) { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c9740b43/be/src/service/impala-beeswax-server.cc ---------------------------------------------------------------------- diff --git a/be/src/service/impala-beeswax-server.cc b/be/src/service/impala-beeswax-server.cc index bcf76b6..eb011a3 100644 --- a/be/src/service/impala-beeswax-server.cc +++ b/be/src/service/impala-beeswax-server.cc @@ -436,7 +436,7 @@ Status ImpalaServer::QueryToTQueryContext(const Query& query, // set yet, set it now. lock_guard<mutex> l(session->lock); if (session->connected_user.empty()) session->connected_user = query.hadoop_user; - query_ctx->client_request.query_options = session->default_query_options; + query_ctx->client_request.query_options = session->QueryOptions(); set_query_options_mask = session->set_query_options_mask; } session->ToThrift(session_id, &query_ctx->session); @@ -444,10 +444,13 @@ Status ImpalaServer::QueryToTQueryContext(const Query& query, // Override default query options with Query.Configuration if (query.__isset.configuration) { + TQueryOptions overlay; + QueryOptionsMask overlay_mask; for (const string& option: query.configuration) { - RETURN_IF_ERROR(ParseQueryOptions(option, &query_ctx->client_request.query_options, - &set_query_options_mask)); + RETURN_IF_ERROR(ParseQueryOptions(option, &overlay, &overlay_mask)); } + OverlayQueryOptions(overlay, overlay_mask, &query_ctx->client_request.query_options); + set_query_options_mask |= overlay_mask; } // Only query options not set in the session or confOverlay can be overridden by the http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c9740b43/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 da8d606..20608b7 100644 --- a/be/src/service/impala-hs2-server.cc +++ b/be/src/service/impala-hs2-server.cc @@ -241,11 +241,13 @@ Status ImpalaServer::TExecuteStatementReqToTQueryContext( RETURN_IF_ERROR(GetSessionState(session_id, &session_state)); session_state->ToThrift(session_id, &query_ctx->session); lock_guard<mutex> l(session_state->lock); - query_ctx->client_request.query_options = session_state->default_query_options; + query_ctx->client_request.query_options = session_state->QueryOptions(); set_query_options_mask = session_state->set_query_options_mask; } if (execute_request.__isset.confOverlay) { + TQueryOptions overlay; + QueryOptionsMask overlay_mask; map<string, string>::const_iterator conf_itr = execute_request.confOverlay.begin(); for (; conf_itr != execute_request.confOverlay.end(); ++conf_itr) { if (conf_itr->first == IMPALA_RESULT_CACHING_OPT) continue; @@ -256,8 +258,10 @@ Status ImpalaServer::TExecuteStatementReqToTQueryContext( continue; } RETURN_IF_ERROR(SetQueryOption(conf_itr->first, conf_itr->second, - &query_ctx->client_request.query_options, &set_query_options_mask)); + &overlay, &overlay_mask)); } + OverlayQueryOptions(overlay, overlay_mask, &query_ctx->client_request.query_options); + set_query_options_mask |= overlay_mask; } // Only query options not set in the session or confOverlay can be overridden by the // pool options. @@ -313,7 +317,7 @@ void ImpalaServer::OpenSession(TOpenSessionResp& return_val, // Process the supplied configuration map. state->database = "default"; state->session_timeout = FLAGS_idle_session_timeout; - state->default_query_options = default_query_options_; + state->server_default_query_options = &default_query_options_; if (request.__isset.configuration) { typedef map<string, string> ConfigurationMap; for (const ConfigurationMap::value_type& v: request.configuration) { @@ -340,13 +344,13 @@ void ImpalaServer::OpenSession(TOpenSessionResp& return_val, } else { // Normal configuration key. Use it to set session default query options. // Ignore failure (failures will be logged in SetQueryOption()). - discard_result(SetQueryOption(v.first, v.second, &state->default_query_options, + discard_result(SetQueryOption(v.first, v.second, &state->set_query_options, &state->set_query_options_mask)); } } } RegisterSessionTimeout(state->session_timeout); - TQueryOptionsToMap(state->default_query_options, &return_val.configuration); + TQueryOptionsToMap(state->QueryOptions(), &return_val.configuration); // OpenSession() should return the coordinator's HTTP server address. const string& http_addr = lexical_cast<string>( http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c9740b43/be/src/service/impala-server.cc ---------------------------------------------------------------------- diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc index 6ce20e9..d9d2629 100644 --- a/be/src/service/impala-server.cc +++ b/be/src/service/impala-server.cc @@ -1241,6 +1241,12 @@ void ImpalaServer::SessionState::ToThrift(const TUniqueId& session_id, state->__set_kudu_latest_observed_ts(kudu_latest_observed_ts); } +TQueryOptions ImpalaServer::SessionState::QueryOptions() { + TQueryOptions ret = *server_default_query_options; + OverlayQueryOptions(set_query_options, set_query_options_mask, &ret); + return ret; +} + void ImpalaServer::CancelFromThreadPool(uint32_t thread_id, const CancellationWork& cancellation_work) { if (cancellation_work.unregister()) { @@ -1730,7 +1736,7 @@ void ImpalaServer::ConnectionStart( session_state->session_timeout = FLAGS_idle_session_timeout; session_state->session_type = TSessionType::BEESWAX; session_state->network_address = connection_context.network_address; - session_state->default_query_options = default_query_options_; + session_state->server_default_query_options = &default_query_options_; session_state->kudu_latest_observed_ts = 0; // If the username was set by a lower-level transport, use it. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c9740b43/be/src/service/impala-server.h ---------------------------------------------------------------------- diff --git a/be/src/service/impala-server.h b/be/src/service/impala-server.h index baca128..dbd2877 100644 --- a/be/src/service/impala-server.h +++ b/be/src/service/impala-server.h @@ -360,13 +360,17 @@ class ImpalaServer : public ImpalaServiceIf, /// The default database (changed as a result of 'use' query execution). std::string database; - /// The default query options of this session. When the session is created, the - /// session inherits the global defaults from ImpalaServer::default_query_options_. - TQueryOptions default_query_options; + /// Reference to the ImpalaServer's query options + TQueryOptions* server_default_query_options; - /// BitSet indicating which query options in default_query_options have been + /// Query options that have been explicitly set in this session. + TQueryOptions set_query_options; + + /// BitSet indicating which query options in set_query_options have been /// explicitly set in the session. Updated when a query option is specified using a /// SET command: the bit corresponding to the TImpalaQueryOptions enum is set. + /// If the option is subsequently reset via a SET with an empty value, the bit + /// is cleared. QueryOptionsMask set_query_options_mask; /// For HS2 only, the protocol version this session is expecting. @@ -399,6 +403,10 @@ class ImpalaServer : public ImpalaServiceIf, /// Builds a Thrift representation of this SessionState for serialisation to /// the frontend. void ToThrift(const TUniqueId& session_id, TSessionState* session_state); + + /// Builds the overlay of the default server query options and the options + /// explicitly set in this session. + TQueryOptions QueryOptions(); }; private: http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c9740b43/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..75b1567 100644 --- a/be/src/service/query-options-test.cc +++ b/be/src/service/query-options-test.cc @@ -147,4 +147,71 @@ TEST(QueryOptions, MapOptionalDefaultlessToEmptyString) { EXPECT_EQ(map["EXPLAIN_LEVEL"], "1"); } +/// Overlay a with b. batch_size is set in both places. +/// num_nodes is set in a and is missing from the bit +/// mask. mt_dop is only set in b. +TEST(QueryOptions, TestOverlay) { + TQueryOptions a; + TQueryOptions b; + QueryOptionsMask mask; + + // overlay + a.__set_batch_size(1); + b.__set_batch_size(2); + mask.set(TImpalaQueryOptions::BATCH_SIZE); + + // no overlay + a.__set_num_nodes(3); + + // missing mask on overlay + b.__set_debug_action("ignored"); // no mask set + + // overlay; no original value + b.__set_mt_dop(4); + mask.set(TImpalaQueryOptions::MT_DOP); + + TQueryOptions dst = a; + OverlayQueryOptions(b, mask, &dst); + + EXPECT_EQ(2, dst.batch_size); + EXPECT_TRUE(dst.__isset.batch_size); + EXPECT_EQ(3, dst.num_nodes); + EXPECT_EQ(a.debug_action, dst.debug_action); + EXPECT_EQ(4, dst.mt_dop); + EXPECT_TRUE(dst.__isset.mt_dop); +} + +TEST(QueryOptions, ResetToDefaultViaEmptyString) { + // MT_DOP has no default; resetting should do nothing. + { + TQueryOptions options; + EXPECT_TRUE(SetQueryOption("MT_DOP", "", &options, NULL).ok()); + EXPECT_FALSE(options.__isset.mt_dop); + } + + // Set and then reset; check mask too. + { + TQueryOptions options; + QueryOptionsMask mask; + + EXPECT_FALSE(options.__isset.mt_dop); + EXPECT_TRUE(SetQueryOption("MT_DOP", "3", &options, &mask).ok()); + EXPECT_TRUE(mask[TImpalaQueryOptions::MT_DOP]); + EXPECT_TRUE(SetQueryOption("MT_DOP", "", &options, &mask).ok()); + EXPECT_FALSE(options.__isset.mt_dop); + // After reset, mask should be clear. + EXPECT_FALSE(mask[TImpalaQueryOptions::MT_DOP]); + } + + // Reset should reset to defaults for something that has set defaults. + { + TQueryOptions options; + + EXPECT_TRUE(SetQueryOption("EXPLAIN_LEVEL", "3", &options, NULL).ok()); + EXPECT_TRUE(SetQueryOption("EXPLAIN_LEVEL", "", &options, NULL).ok()); + EXPECT_TRUE(options.__isset.explain_level); + EXPECT_EQ(options.explain_level, 1); + } +} + IMPALA_TEST_MAIN(); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c9740b43/be/src/service/query-options.cc ---------------------------------------------------------------------- diff --git a/be/src/service/query-options.cc b/be/src/service/query-options.cc index e950d31..d4f1f73 100644 --- a/be/src/service/query-options.cc +++ b/be/src/service/query-options.cc @@ -58,7 +58,7 @@ void impala::OverlayQueryOptions(const TQueryOptions& src, const QueryOptionsMas DCHECK_GT(mask.size(), _TImpalaQueryOptions_VALUES_TO_NAMES.size()) << "Size of QueryOptionsMask must be increased."; #define QUERY_OPT_FN(NAME, ENUM)\ - if (src.__isset.NAME && mask[TImpalaQueryOptions::ENUM]) dst->NAME = src.NAME; + if (src.__isset.NAME && mask[TImpalaQueryOptions::ENUM]) dst->__set_##NAME(src.NAME); QUERY_OPTS_TABLE #undef QUERY_OPT_FN } @@ -79,6 +79,20 @@ void impala::TQueryOptionsToMap(const TQueryOptions& query_options, #undef QUERY_OPT_FN } +// Resets query_options->option to its default value. +static void ResetQueryOption(const int option, TQueryOptions* query_options) { + const static TQueryOptions defaults; + switch (option) { +#define QUERY_OPT_FN(NAME, ENUM)\ + case TImpalaQueryOptions::ENUM:\ + query_options->__isset.NAME = defaults.__isset.NAME;\ + query_options->NAME = defaults.NAME;\ + break; + QUERY_OPTS_TABLE +#undef QUERY_OPT_FN + } +} + string impala::DebugQueryOptions(const TQueryOptions& query_options) { const static TQueryOptions defaults; int i = 0; @@ -96,7 +110,7 @@ string impala::DebugQueryOptions(const TQueryOptions& query_options) { // Returns the TImpalaQueryOptions enum for the given "key". Input is case insensitive. // Return -1 if the input is an invalid option. -int GetQueryOptionForKey(const string& key) { +static int GetQueryOptionForKey(const string& key) { map<int, const char*>::const_iterator itr = _TImpalaQueryOptions_VALUES_TO_NAMES.begin(); for (; itr != _TImpalaQueryOptions_VALUES_TO_NAMES.end(); ++itr) { @@ -115,6 +129,12 @@ Status impala::SetQueryOption(const string& key, const string& value, int option = GetQueryOptionForKey(key); if (option < 0) { return Status(Substitute("Invalid query option: $0", key)); + } else if (value == "") { + ResetQueryOption(option, query_options); + if (set_query_options_mask != nullptr) { + DCHECK_LT(option, set_query_options_mask->size()); + set_query_options_mask->reset(option); + } } else { switch (option) { case TImpalaQueryOptions::ABORT_ON_ERROR: @@ -176,7 +196,6 @@ Status impala::SetQueryOption(const string& key, const string& value, break; } case TImpalaQueryOptions::COMPRESSION_CODEC: { - if (value.empty()) break; if (iequals(value, "none")) { query_options->__set_compression_codec(THdfsCompression::NONE); } else if (iequals(value, "gzip")) { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c9740b43/be/src/service/query-options.h ---------------------------------------------------------------------- diff --git a/be/src/service/query-options.h b/be/src/service/query-options.h index 3dada7d..5ca8c5f 100644 --- a/be/src/service/query-options.h +++ b/be/src/service/query-options.h @@ -124,7 +124,7 @@ void OverlayQueryOptions(const TQueryOptions& src, const QueryOptionsMask& mask, /// Set the key/value pair in TQueryOptions. It will override existing setting in /// query_options. The bit corresponding to query option 'key' in set_query_options_mask -/// is set. +/// is set. An empty string value will reset the key to its default value. Status SetQueryOption(const std::string& key, const std::string& value, TQueryOptions* query_options, QueryOptionsMask* set_query_options_mask); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c9740b43/common/thrift/ImpalaInternalService.thrift ---------------------------------------------------------------------- diff --git a/common/thrift/ImpalaInternalService.thrift b/common/thrift/ImpalaInternalService.thrift index db6ffbb..cda5083 100644 --- a/common/thrift/ImpalaInternalService.thrift +++ b/common/thrift/ImpalaInternalService.thrift @@ -72,9 +72,22 @@ enum TJoinDistributionMode { // // (1) and (2) are set by administrators and provide the default query options for a // session, in that order, so options set in (2) override those in (1). The user -// can specify query options with (3) to override the defaults, which are stored in the -// SessionState. Finally, the client can pass a config 'overlay' (4) in the request -// metadata which overrides everything else. +// can specify query options with (3) to override the preceding layers; these +// overrides are stored in SessionState. Finally, the client can pass a config +// 'overlay' (4) in the request metadata which overrides everything else. +// +// Session options (level 3, above) can be set by the user with SET <key>=<value> +// or in the OpenSession RPC. They can be unset with SET <key>="". When unset, +// it's unset in that level, and the values as specified by the defaults, +// and levels 1 and 2 above take hold. +// +// Because of the ambiguity between null and the empty string here, string-typed +// options where the empty string is a valid value can cause problems as follows: +// * If their default is not the empty string, a user can't set it to the +// empty string with SET. +// * Even if their default is the empty string, they may be set to something +// else via process defaults or resource pool defaults, and the user +// may not be able to override them back to the empty string. struct TQueryOptions { 1: optional bool abort_on_error = 0 2: optional i32 max_errors = 100 http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c9740b43/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..364a01e 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/set.test +++ b/testdata/workloads/functional-query/queries/QueryTest/set.test @@ -1,11 +1,17 @@ ==== ---- QUERY +# Set an option explicitly; the test infrastructure should clear it before the next test. +# The next test tests that buffer_pool_limit is unset (""). +set buffer_pool_limit=7; +==== +---- QUERY set ---- RESULTS: VERIFY_IS_SUBSET 'ABORT_ON_DEFAULT_LIMIT_EXCEEDED','0' 'ABORT_ON_ERROR','0' 'ALLOW_UNSUPPORTED_FORMATS','0' 'BATCH_SIZE','0' +'BUFFER_POOL_LIMIT','' 'DEBUG_ACTION','' 'DEFAULT_ORDER_BY_LIMIT','-1' 'DISABLE_CACHED_READS','0' http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c9740b43/tests/common/impala_test_suite.py ---------------------------------------------------------------------- diff --git a/tests/common/impala_test_suite.py b/tests/common/impala_test_suite.py index 0732695..6c23aa8 100644 --- a/tests/common/impala_test_suite.py +++ b/tests/common/impala_test_suite.py @@ -211,7 +211,7 @@ class ImpalaTestSuite(BaseTestSuite): if not query_option in self.default_query_options: continue default_val = self.default_query_options[query_option] - query_str = 'SET '+ query_option + '="' + default_val + '"' + query_str = 'SET ' + query_option + '="' + default_val + '"' try: impalad_client.execute(query_str) except Exception as e: http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c9740b43/tests/custom_cluster/test_admission_controller.py ---------------------------------------------------------------------- diff --git a/tests/custom_cluster/test_admission_controller.py b/tests/custom_cluster/test_admission_controller.py index de97e7c..712ce57 100644 --- a/tests/custom_cluster/test_admission_controller.py +++ b/tests/custom_cluster/test_admission_controller.py @@ -156,8 +156,10 @@ class TestAdmissionController(TestAdmissionControllerBase, HS2TestSuite): assert len(confs) == len(expected_query_options) confs = map(str.lower, confs) for expected in expected_query_options: - assert expected.lower() in confs,\ - "Expected query options '%s' to be set" % (",".join(expected_query_options)) + if expected.lower() not in confs: + expected = ",".join(sorted(expected_query_options)) + actual = ",".join(sorted(confs)) + assert False, "Expected query options %s, got %s." % (expected, actual) def __check_hs2_query_opts(self, pool_name, mem_limit=None, expected_options=None): """ Submits a query via HS2 (optionally with a mem_limit in the confOverlay) @@ -254,6 +256,18 @@ class TestAdmissionController(TestAdmissionControllerBase, HS2TestSuite): self.__check_query_options(result.runtime_profile,\ ['MEM_LIMIT=12345', 'QUERY_TIMEOUT_S=5', 'REQUEST_POOL=root.queueA',\ 'ABORT_ON_ERROR=1', 'MAX_IO_BUFFERS=100']) + + # Once options are reset to their defaults, the queue + # configuration should kick back in. We'll see the + # queue-configured mem_limit, and we won't see + # abort on error, because it's back to being the default. + client.execute('set mem_limit=""') + client.execute('set abort_on_error=""') + client.set_configuration({ 'request_pool': 'root.queueA' }) + result = client.execute("select 1") + self.__check_query_options(result.runtime_profile, + [queueA_mem_limit, 'REQUEST_POOL=root.queueA', 'QUERY_TIMEOUT_S=5']) + finally: client.close() http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c9740b43/tests/custom_cluster/test_set_and_unset.py ---------------------------------------------------------------------- diff --git a/tests/custom_cluster/test_set_and_unset.py b/tests/custom_cluster/test_set_and_unset.py new file mode 100644 index 0000000..3347ca7 --- /dev/null +++ b/tests/custom_cluster/test_set_and_unset.py @@ -0,0 +1,98 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import pytest + +from tests.common.custom_cluster_test_suite import CustomClusterTestSuite +from tests.hs2.hs2_test_suite import HS2TestSuite, needs_session +from TCLIService import TCLIService +from ImpalaService import ImpalaHiveServer2Service + +class TestSetAndUnset(CustomClusterTestSuite, HS2TestSuite): + """ + Test behavior of SET and UNSET within a session, and how + SET/UNSET override options configured at the impalad level. + """ + @pytest.mark.execute_serially + @CustomClusterTestSuite.with_args( + impalad_args="--default_query_options=debug_action=custom") + @needs_session(TCLIService.TProtocolVersion.HIVE_CLI_SERVICE_PROTOCOL_V6) + def test_set_and_unset(self): + """ + Starts Impala cluster with a custom query option, and checks that option + overlaying works correctly. + + The Beeswax API and the HiveServer2 implementations are slightly different, + so the same test is run in both contexts. + """ + # Beeswax API: + result = self.execute_query_expect_success(self.client, "set") + assert "DEBUG_ACTION\tcustom" in result.data, "baseline" + self.execute_query_expect_success(self.client, "set debug_action=hey") + assert "DEBUG_ACTION\they" in \ + self.execute_query_expect_success(self.client, "set").data, "session override" + self.execute_query_expect_success(self.client, 'set debug_action=""') + assert "DEBUG_ACTION\tcustom" in \ + self.execute_query_expect_success(self.client, "set").data, "reset" + self.execute_query_expect_success(self.client, 'set batch_size=123') + # Use a "request overlay" to change the option for a specific + # request within a session. We run a real query and check its + # runtime profile, as SET shows session options without applying + # the request overlays to them. + assert "BATCH_SIZE=100" in self.execute_query_expect_success(self.client, 'select 1', + query_options=dict(batch_size="100")).runtime_profile, "request overlay" + + # Overlaying an empty string (unset) has no effect; the session option + # takes hold and the "request overlay" is considered blank. + assert "BATCH_SIZE=123" in self.execute_query_expect_success(self.client, 'select 1', + query_options=dict(batch_size="")).runtime_profile, "null request overlay" + + # Same dance, but with HS2: + assert ("DEBUG_ACTION", "custom") in self.get_set_results(), "baseline" + self.execute_statement("set debug_action='hey'") + assert ("DEBUG_ACTION", "hey") in self.get_set_results(), "session override" + self.execute_statement("set debug_action=''") + assert ("DEBUG_ACTION", "custom") in self.get_set_results(), "reset" + + # Request Overlay + self.execute_statement("set batch_size=123") + execute_statement_resp = self.execute_statement("select 1", conf_overlay=dict(batch_size="100")) + get_profile_req = ImpalaHiveServer2Service.TGetRuntimeProfileReq() + get_profile_req.operationHandle = execute_statement_resp.operationHandle + get_profile_req.sessionHandle = self.session_handle + assert "BATCH_SIZE=100" in self.hs2_client.GetRuntimeProfile(get_profile_req).profile + + # Null request overlay + self.execute_statement("set batch_size=999") + execute_statement_resp = self.execute_statement("select 1", conf_overlay=dict(batch_size="")) + get_profile_req = ImpalaHiveServer2Service.TGetRuntimeProfileReq() + get_profile_req.operationHandle = execute_statement_resp.operationHandle + get_profile_req.sessionHandle = self.session_handle + assert "BATCH_SIZE=999" in self.hs2_client.GetRuntimeProfile(get_profile_req).profile + + def get_set_results(self): + """ + Executes a "SET" HiveServer2 query and returns a list + of key-value tuples. + """ + execute_statement_resp = self.execute_statement("set") + fetch_results_req = TCLIService.TFetchResultsReq() + fetch_results_req.operationHandle = execute_statement_resp.operationHandle + fetch_results_req.maxRows = 100 + fetch_results_resp = self.hs2_client.FetchResults(fetch_results_req) + return zip(fetch_results_resp.results.columns[0].stringVal.values, + fetch_results_resp.results.columns[1].stringVal.values) http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/c9740b43/tests/hs2/hs2_test_suite.py ---------------------------------------------------------------------- diff --git a/tests/hs2/hs2_test_suite.py b/tests/hs2/hs2_test_suite.py index 1b2f89f..da9f68b 100644 --- a/tests/hs2/hs2_test_suite.py +++ b/tests/hs2/hs2_test_suite.py @@ -228,11 +228,13 @@ class HS2TestSuite(ImpalaTestSuite): 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): + def execute_statement(self, statement, conf_overlay=None): """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 + if conf_overlay: + execute_statement_req.confOverlay = conf_overlay execute_statement_resp = self.hs2_client.ExecuteStatement(execute_statement_req) HS2TestSuite.check_response(execute_statement_resp) return execute_statement_resp
