From 199188be080217053fcd02f8a9bb79e41bac36dd Mon Sep 17 00:00:00 2001 From: kssenii Date: Mon, 14 Mar 2022 12:00:47 +0100 Subject: [PATCH] Support test mode for clickhouse-local --- programs/client/CMakeLists.txt | 1 - programs/client/Client.cpp | 211 ----------------- programs/client/Client.h | 6 +- programs/local/LocalServer.cpp | 86 ------- programs/local/LocalServer.h | 6 +- src/Client/ClientBase.cpp | 219 ++++++++++++++++++ src/Client/ClientBase.h | 2 +- {programs/client => src/Client}/TestTags.cpp | 0 {programs/client => src/Client}/TestTags.h | 0 ...02234_clickhouse_local_test_mode.reference | 1 + .../02234_clickhouse_local_test_mode.sh | 10 + 11 files changed, 240 insertions(+), 302 deletions(-) rename {programs/client => src/Client}/TestTags.cpp (100%) rename {programs/client => src/Client}/TestTags.h (100%) create mode 100644 tests/queries/0_stateless/02234_clickhouse_local_test_mode.reference create mode 100755 tests/queries/0_stateless/02234_clickhouse_local_test_mode.sh diff --git a/programs/client/CMakeLists.txt b/programs/client/CMakeLists.txt index 97616e9b69f..d212da59908 100644 --- a/programs/client/CMakeLists.txt +++ b/programs/client/CMakeLists.txt @@ -1,6 +1,5 @@ set (CLICKHOUSE_CLIENT_SOURCES Client.cpp - TestTags.cpp ) set (CLICKHOUSE_CLIENT_LINK diff --git a/programs/client/Client.cpp b/programs/client/Client.cpp index a3f5c0ab1c7..bee2d501a3b 100644 --- a/programs/client/Client.cpp +++ b/programs/client/Client.cpp @@ -21,7 +21,6 @@ #include #include -#include #include #include @@ -43,7 +42,6 @@ #include #include #include -#include "TestTags.h" #ifndef __clang__ #pragma GCC optimize("-fno-var-tracking-assignments") @@ -102,212 +100,6 @@ void Client::processError(const String & query) const } -bool Client::executeMultiQuery(const String & all_queries_text) -{ - // It makes sense not to base any control flow on this, so that it is - // the same in tests and in normal usage. The only difference is that in - // normal mode we ignore the test hints. - const bool test_mode = config().has("testmode"); - if (test_mode) - { - /// disable logs if expects errors - TestHint test_hint(test_mode, all_queries_text); - if (test_hint.clientError() || test_hint.serverError()) - processTextAsSingleQuery("SET send_logs_level = 'fatal'"); - } - - bool echo_query = echo_queries; - - /// Test tags are started with "--" so they are interpreted as comments anyway. - /// But if the echo is enabled we have to remove the test tags from `all_queries_text` - /// because we don't want test tags to be echoed. - size_t test_tags_length = test_mode ? getTestTagsLength(all_queries_text) : 0; - - /// Several queries separated by ';'. - /// INSERT data is ended by the end of line, not ';'. - /// An exception is VALUES format where we also support semicolon in - /// addition to end of line. - const char * this_query_begin = all_queries_text.data() + test_tags_length; - const char * this_query_end; - const char * all_queries_end = all_queries_text.data() + all_queries_text.size(); - - String full_query; // full_query is the query + inline INSERT data + trailing comments (the latter is our best guess for now). - String query_to_execute; - ASTPtr parsed_query; - std::optional current_exception; - - while (true) - { - auto stage = analyzeMultiQueryText(this_query_begin, this_query_end, all_queries_end, - query_to_execute, parsed_query, all_queries_text, current_exception); - switch (stage) - { - case MultiQueryProcessingStage::QUERIES_END: - case MultiQueryProcessingStage::PARSING_FAILED: - { - return true; - } - case MultiQueryProcessingStage::CONTINUE_PARSING: - { - continue; - } - case MultiQueryProcessingStage::PARSING_EXCEPTION: - { - this_query_end = find_first_symbols<'\n'>(this_query_end, all_queries_end); - - // Try to find test hint for syntax error. We don't know where - // the query ends because we failed to parse it, so we consume - // the entire line. - TestHint hint(test_mode, String(this_query_begin, this_query_end - this_query_begin)); - if (hint.serverError()) - { - // Syntax errors are considered as client errors - current_exception->addMessage("\nExpected server error '{}'.", hint.serverError()); - current_exception->rethrow(); - } - - if (hint.clientError() != current_exception->code()) - { - if (hint.clientError()) - current_exception->addMessage("\nExpected client error: " + std::to_string(hint.clientError())); - current_exception->rethrow(); - } - - /// It's expected syntax error, skip the line - this_query_begin = this_query_end; - current_exception.reset(); - - continue; - } - case MultiQueryProcessingStage::EXECUTE_QUERY: - { - full_query = all_queries_text.substr(this_query_begin - all_queries_text.data(), this_query_end - this_query_begin); - if (query_fuzzer_runs) - { - if (!processWithFuzzing(full_query)) - return false; - this_query_begin = this_query_end; - continue; - } - - // Now we know for sure where the query ends. - // Look for the hint in the text of query + insert data + trailing - // comments, - // e.g. insert into t format CSV 'a' -- { serverError 123 }. - // Use the updated query boundaries we just calculated. - TestHint test_hint(test_mode, full_query); - // Echo all queries if asked; makes for a more readable reference - // file. - echo_query = test_hint.echoQueries().value_or(echo_query); - try - { - processParsedSingleQuery(full_query, query_to_execute, parsed_query, echo_query, false); - } - catch (...) - { - // Surprisingly, this is a client error. A server error would - // have been reported w/o throwing (see onReceiveSeverException()). - client_exception = std::make_unique(getCurrentExceptionMessage(print_stack_trace), getCurrentExceptionCode()); - have_error = true; - } - // Check whether the error (or its absence) matches the test hints - // (or their absence). - bool error_matches_hint = true; - if (have_error) - { - if (test_hint.serverError()) - { - if (!server_exception) - { - error_matches_hint = false; - fmt::print(stderr, "Expected server error code '{}' but got no server error (query: {}).\n", - test_hint.serverError(), full_query); - } - else if (server_exception->code() != test_hint.serverError()) - { - error_matches_hint = false; - fmt::print(stderr, "Expected server error code: {} but got: {} (query: {}).\n", - test_hint.serverError(), server_exception->code(), full_query); - } - } - if (test_hint.clientError()) - { - if (!client_exception) - { - error_matches_hint = false; - fmt::print(stderr, "Expected client error code '{}' but got no client error (query: {}).\n", - test_hint.clientError(), full_query); - } - else if (client_exception->code() != test_hint.clientError()) - { - error_matches_hint = false; - fmt::print(stderr, "Expected client error code '{}' but got '{}' (query: {}).\n", - test_hint.clientError(), client_exception->code(), full_query); - } - } - if (!test_hint.clientError() && !test_hint.serverError()) - { - // No error was expected but it still occurred. This is the - // default case w/o test hint, doesn't need additional - // diagnostics. - error_matches_hint = false; - } - } - else - { - if (test_hint.clientError()) - { - fmt::print(stderr, "The query succeeded but the client error '{}' was expected (query: {}).\n", - test_hint.clientError(), full_query); - error_matches_hint = false; - } - if (test_hint.serverError()) - { - fmt::print(stderr, "The query succeeded but the server error '{}' was expected (query: {}).\n", - test_hint.serverError(), full_query); - error_matches_hint = false; - } - } - // If the error is expected, force reconnect and ignore it. - if (have_error && error_matches_hint) - { - client_exception.reset(); - server_exception.reset(); - have_error = false; - - if (!connection->checkConnected()) - connect(); - } - - // For INSERTs with inline data: use the end of inline data as - // reported by the format parser (it is saved in sendData()). - // This allows us to handle queries like: - // insert into t values (1); select 1 - // , where the inline data is delimited by semicolon and not by a - // newline. - auto * insert_ast = parsed_query->as(); - if (insert_ast && isSyncInsertWithData(*insert_ast, global_context)) - { - this_query_end = insert_ast->end; - adjustQueryEnd(this_query_end, all_queries_end, global_context->getSettingsRef().max_parser_depth); - } - - // Report error. - if (have_error) - processError(full_query); - - // Stop processing queries if needed. - if (have_error && !ignore_error) - return is_interactive; - - this_query_begin = this_query_end; - break; - } - } - } -} - - /// Make query to get all server warnings std::vector Client::loadWarningMessages() { @@ -1015,7 +807,6 @@ void Client::addOptions(OptionsDescription & options_description) ("password", po::value()->implicit_value("\n", ""), "password") ("ask-password", "ask-password") ("quota_key", po::value(), "A string to differentiate quotas when the user have keyed quotas configured on server") - ("testmode,T", "enable test hints in comments") ("max_client_network_bandwidth", po::value(), "the maximum speed of data exchange over the network for the client in bytes per second.") ("compression", po::value(), "enable or disable compression") @@ -1151,8 +942,6 @@ void Client::processOptions(const OptionsDescription & options_description, config().setBool("ask-password", true); if (options.count("quota_key")) config().setString("quota_key", options["quota_key"].as()); - if (options.count("testmode")) - config().setBool("testmode", true); if (options.count("max_client_network_bandwidth")) max_client_network_bandwidth = options["max_client_network_bandwidth"].as(); if (options.count("compression")) diff --git a/programs/client/Client.h b/programs/client/Client.h index 45bdd077351..d235da45139 100644 --- a/programs/client/Client.h +++ b/programs/client/Client.h @@ -16,20 +16,24 @@ public: int main(const std::vector & /*args*/) override; protected: - bool executeMultiQuery(const String & all_queries_text) override; bool processWithFuzzing(const String & full_query) override; void connect() override; + void processError(const String & query) const override; + String getName() const override { return "client"; } void printHelpMessage(const OptionsDescription & options_description) override; + void addOptions(OptionsDescription & options_description) override; + void processOptions( const OptionsDescription & options_description, const CommandLineOptions & options, const std::vector & external_tables_arguments, const std::vector & hosts_and_ports_arguments) override; + void processConfig() override; private: diff --git a/programs/local/LocalServer.cpp b/programs/local/LocalServer.cpp index eb562dfd9eb..e92d2f4099a 100644 --- a/programs/local/LocalServer.cpp +++ b/programs/local/LocalServer.cpp @@ -91,92 +91,6 @@ void LocalServer::processError(const String &) const } -bool LocalServer::executeMultiQuery(const String & all_queries_text) -{ - bool echo_query = echo_queries; - - /// Several queries separated by ';'. - /// INSERT data is ended by the end of line, not ';'. - /// An exception is VALUES format where we also support semicolon in - /// addition to end of line. - const char * this_query_begin = all_queries_text.data(); - const char * this_query_end; - const char * all_queries_end = all_queries_text.data() + all_queries_text.size(); - - String full_query; // full_query is the query + inline INSERT data + trailing comments (the latter is our best guess for now). - String query_to_execute; - ASTPtr parsed_query; - std::optional current_exception; - - while (true) - { - auto stage = analyzeMultiQueryText(this_query_begin, this_query_end, all_queries_end, - query_to_execute, parsed_query, all_queries_text, current_exception); - switch (stage) - { - case MultiQueryProcessingStage::QUERIES_END: - case MultiQueryProcessingStage::PARSING_FAILED: - { - return true; - } - case MultiQueryProcessingStage::CONTINUE_PARSING: - { - continue; - } - case MultiQueryProcessingStage::PARSING_EXCEPTION: - { - if (current_exception) - current_exception->rethrow(); - return true; - } - case MultiQueryProcessingStage::EXECUTE_QUERY: - { - full_query = all_queries_text.substr(this_query_begin - all_queries_text.data(), this_query_end - this_query_begin); - - try - { - processParsedSingleQuery(full_query, query_to_execute, parsed_query, echo_query, false); - } - catch (...) - { - if (!is_interactive && !ignore_error) - throw; - - // Surprisingly, this is a client error. A server error would - // have been reported w/o throwing (see onReceiveSeverException()). - client_exception = std::make_unique(getCurrentExceptionMessage(print_stack_trace), getCurrentExceptionCode()); - have_error = true; - } - - // For INSERTs with inline data: use the end of inline data as - // reported by the format parser (it is saved in sendData()). - // This allows us to handle queries like: - // insert into t values (1); select 1 - // , where the inline data is delimited by semicolon and not by a - // newline. - auto * insert_ast = parsed_query->as(); - if (insert_ast && insert_ast->data) - { - this_query_end = insert_ast->end; - adjustQueryEnd(this_query_end, all_queries_end, global_context->getSettingsRef().max_parser_depth); - } - - // Report error. - if (have_error) - processError(full_query); - - // Stop processing queries if needed. - if (have_error && !ignore_error) - return is_interactive; - - this_query_begin = this_query_end; - break; - } - } - } -} - - void LocalServer::initialize(Poco::Util::Application & self) { Poco::Util::Application::initialize(self); diff --git a/programs/local/LocalServer.h b/programs/local/LocalServer.h index cc186e343c1..969af7f1b77 100644 --- a/programs/local/LocalServer.h +++ b/programs/local/LocalServer.h @@ -31,17 +31,19 @@ public: int main(const std::vector & /*args*/) override; protected: - bool executeMultiQuery(const String & all_queries_text) override; - void connect() override; + void processError(const String & query) const override; + String getName() const override { return "local"; } void printHelpMessage(const OptionsDescription & options_description) override; void addOptions(OptionsDescription & options_description) override; + void processOptions(const OptionsDescription & options_description, const CommandLineOptions & options, const std::vector &, const std::vector &) override; + void processConfig() override; private: diff --git a/src/Client/ClientBase.cpp b/src/Client/ClientBase.cpp index a69acb2ed82..c575cd37a5f 100644 --- a/src/Client/ClientBase.cpp +++ b/src/Client/ClientBase.cpp @@ -36,6 +36,8 @@ #include #include +#include +#include "TestTags.h" #include #include @@ -1483,6 +1485,219 @@ MultiQueryProcessingStage ClientBase::analyzeMultiQueryText( } +bool ClientBase::executeMultiQuery(const String & all_queries_text) +{ + // It makes sense not to base any control flow on this, so that it is + // the same in tests and in normal usage. The only difference is that in + // normal mode we ignore the test hints. + const bool test_mode = config().has("testmode"); + if (test_mode) + { + /// disable logs if expects errors + TestHint test_hint(test_mode, all_queries_text); + if (test_hint.clientError() || test_hint.serverError()) + processTextAsSingleQuery("SET send_logs_level = 'fatal'"); + } + + bool echo_query = echo_queries; + + /// Test tags are started with "--" so they are interpreted as comments anyway. + /// But if the echo is enabled we have to remove the test tags from `all_queries_text` + /// because we don't want test tags to be echoed. + size_t test_tags_length = test_mode ? getTestTagsLength(all_queries_text) : 0; + + /// Several queries separated by ';'. + /// INSERT data is ended by the end of line, not ';'. + /// An exception is VALUES format where we also support semicolon in + /// addition to end of line. + const char * this_query_begin = all_queries_text.data() + test_tags_length; + const char * this_query_end; + const char * all_queries_end = all_queries_text.data() + all_queries_text.size(); + + String full_query; // full_query is the query + inline INSERT data + trailing comments (the latter is our best guess for now). + String query_to_execute; + ASTPtr parsed_query; + std::optional current_exception; + + while (true) + { + auto stage = analyzeMultiQueryText(this_query_begin, this_query_end, all_queries_end, + query_to_execute, parsed_query, all_queries_text, current_exception); + switch (stage) + { + case MultiQueryProcessingStage::QUERIES_END: + case MultiQueryProcessingStage::PARSING_FAILED: + { + return true; + } + case MultiQueryProcessingStage::CONTINUE_PARSING: + { + continue; + } + case MultiQueryProcessingStage::PARSING_EXCEPTION: + { + this_query_end = find_first_symbols<'\n'>(this_query_end, all_queries_end); + + // Try to find test hint for syntax error. We don't know where + // the query ends because we failed to parse it, so we consume + // the entire line. + TestHint hint(test_mode, String(this_query_begin, this_query_end - this_query_begin)); + if (hint.serverError()) + { + // Syntax errors are considered as client errors + current_exception->addMessage("\nExpected server error '{}'.", hint.serverError()); + current_exception->rethrow(); + } + + if (hint.clientError() != current_exception->code()) + { + if (hint.clientError()) + current_exception->addMessage("\nExpected client error: " + std::to_string(hint.clientError())); + + current_exception->rethrow(); + } + + /// It's expected syntax error, skip the line + this_query_begin = this_query_end; + current_exception.reset(); + + continue; + } + case MultiQueryProcessingStage::EXECUTE_QUERY: + { + full_query = all_queries_text.substr(this_query_begin - all_queries_text.data(), this_query_end - this_query_begin); + if (query_fuzzer_runs) + { + if (!processWithFuzzing(full_query)) + return false; + + this_query_begin = this_query_end; + continue; + } + + // Now we know for sure where the query ends. + // Look for the hint in the text of query + insert data + trailing + // comments, e.g. insert into t format CSV 'a' -- { serverError 123 }. + // Use the updated query boundaries we just calculated. + TestHint test_hint(test_mode, full_query); + + // Echo all queries if asked; makes for a more readable reference file. + echo_query = test_hint.echoQueries().value_or(echo_query); + + try + { + processParsedSingleQuery(full_query, query_to_execute, parsed_query, echo_query, false); + } + catch (...) + { + // Surprisingly, this is a client error. A server error would + // have been reported w/o throwing (see onReceiveSeverException()). + client_exception = std::make_unique(getCurrentExceptionMessage(print_stack_trace), getCurrentExceptionCode()); + have_error = true; + } + + // Check whether the error (or its absence) matches the test hints + // (or their absence). + bool error_matches_hint = true; + if (have_error) + { + if (test_hint.serverError()) + { + if (!server_exception) + { + error_matches_hint = false; + fmt::print(stderr, "Expected server error code '{}' but got no server error (query: {}).\n", + test_hint.serverError(), full_query); + } + else if (server_exception->code() != test_hint.serverError()) + { + error_matches_hint = false; + fmt::print(stderr, "Expected server error code: {} but got: {} (query: {}).\n", + test_hint.serverError(), server_exception->code(), full_query); + } + } + if (test_hint.clientError()) + { + if (!client_exception) + { + error_matches_hint = false; + fmt::print(stderr, "Expected client error code '{}' but got no client error (query: {}).\n", + test_hint.clientError(), full_query); + } + else if (client_exception->code() != test_hint.clientError()) + { + error_matches_hint = false; + fmt::print(stderr, "Expected client error code '{}' but got '{}' (query: {}).\n", + test_hint.clientError(), client_exception->code(), full_query); + } + } + if (!test_hint.clientError() && !test_hint.serverError()) + { + // No error was expected but it still occurred. This is the + // default case w/o test hint, doesn't need additional + // diagnostics. + error_matches_hint = false; + } + } + else + { + if (test_hint.clientError()) + { + error_matches_hint = false; + fmt::print(stderr, + "The query succeeded but the client error '{}' was expected (query: {}).\n", + test_hint.clientError(), full_query); + } + if (test_hint.serverError()) + { + error_matches_hint = false; + fmt::print(stderr, + "The query succeeded but the server error '{}' was expected (query: {}).\n", + test_hint.serverError(), full_query); + } + } + + // If the error is expected, force reconnect and ignore it. + if (have_error && error_matches_hint) + { + client_exception.reset(); + server_exception.reset(); + + have_error = false; + + if (!connection->checkConnected()) + connect(); + } + + // For INSERTs with inline data: use the end of inline data as + // reported by the format parser (it is saved in sendData()). + // This allows us to handle queries like: + // insert into t values (1); select 1 + // , where the inline data is delimited by semicolon and not by a + // newline. + auto * insert_ast = parsed_query->as(); + if (insert_ast && isSyncInsertWithData(*insert_ast, global_context)) + { + this_query_end = insert_ast->end; + adjustQueryEnd(this_query_end, all_queries_end, global_context->getSettingsRef().max_parser_depth); + } + + // Report error. + if (have_error) + processError(full_query); + + // Stop processing queries if needed. + if (have_error && !ignore_error) + return is_interactive; + + this_query_begin = this_query_end; + break; + } + } + } +} + + bool ClientBase::processQueryText(const String & text) { if (exit_strings.end() != exit_strings.find(trim(text, [](char c) { return isWhitespaceASCII(c) || c == ';'; }))) @@ -1967,6 +2182,8 @@ void ClientBase::init(int argc, char ** argv) ("suggestion_limit", po::value()->default_value(10000), "Suggestion limit for how many databases, tables and columns to fetch.") + ("testmode,T", "enable test hints in comments") + ("format,f", po::value(), "default output format") ("vertical,E", "vertical output format, same as --format=Vertical or FORMAT Vertical or \\G at end of command") ("highlight", po::value()->default_value(true), "enable or disable basic syntax highlight in interactive command line") @@ -2072,6 +2289,8 @@ void ClientBase::init(int argc, char ** argv) config().setBool("interactive", true); if (options.count("pager")) config().setString("pager", options["pager"].as()); + if (options.count("testmode")) + config().setBool("testmode", true); if (options.count("log-level")) Poco::Logger::root().setLevel(options["log-level"].as()); diff --git a/src/Client/ClientBase.h b/src/Client/ClientBase.h index e625d4a5c63..551f771c0e9 100644 --- a/src/Client/ClientBase.h +++ b/src/Client/ClientBase.h @@ -61,7 +61,6 @@ protected: throw Exception("Query processing with fuzzing is not implemented", ErrorCodes::NOT_IMPLEMENTED); } - virtual bool executeMultiQuery(const String & all_queries_text) = 0; virtual void connect() = 0; virtual void processError(const String & query) const = 0; virtual String getName() const = 0; @@ -77,6 +76,7 @@ protected: ASTPtr parseQuery(const char *& pos, const char * end, bool allow_multi_statements) const; static void setupSignalHandler(); + bool executeMultiQuery(const String & all_queries_text); MultiQueryProcessingStage analyzeMultiQueryText( const char *& this_query_begin, const char *& this_query_end, const char * all_queries_end, String & query_to_execute, ASTPtr & parsed_query, const String & all_queries_text, diff --git a/programs/client/TestTags.cpp b/src/Client/TestTags.cpp similarity index 100% rename from programs/client/TestTags.cpp rename to src/Client/TestTags.cpp diff --git a/programs/client/TestTags.h b/src/Client/TestTags.h similarity index 100% rename from programs/client/TestTags.h rename to src/Client/TestTags.h diff --git a/tests/queries/0_stateless/02234_clickhouse_local_test_mode.reference b/tests/queries/0_stateless/02234_clickhouse_local_test_mode.reference new file mode 100644 index 00000000000..d86bac9de59 --- /dev/null +++ b/tests/queries/0_stateless/02234_clickhouse_local_test_mode.reference @@ -0,0 +1 @@ +OK diff --git a/tests/queries/0_stateless/02234_clickhouse_local_test_mode.sh b/tests/queries/0_stateless/02234_clickhouse_local_test_mode.sh new file mode 100755 index 00000000000..6abe1e30334 --- /dev/null +++ b/tests/queries/0_stateless/02234_clickhouse_local_test_mode.sh @@ -0,0 +1,10 @@ +#!/usr/bin/env bash + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CURDIR"/../shell_config.sh + + +$CLICKHOUSE_LOCAL --query="SELECT n" 2>&1 | grep -q "Code: 47. DB::Exception: Missing columns:" && echo 'OK' || echo 'FAIL' ||: +$CLICKHOUSE_LOCAL --testmode --query="SELECT n -- { serverError 47 }" +