From eb2ed1b123f76be4d23e61a97e6a958cecee2e36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Mar=C3=ADn?= Date: Fri, 3 Mar 2023 13:40:16 +0100 Subject: [PATCH] Add support for different expected errors --- src/Client/ClientBase.cpp | 43 +++++++------- src/Client/TestHint.cpp | 45 +++++++++++---- src/Client/TestHint.h | 57 +++++++++++++++++-- .../01470_columns_transformers.sql | 8 +-- 4 files changed, 111 insertions(+), 42 deletions(-) diff --git a/src/Client/ClientBase.cpp b/src/Client/ClientBase.cpp index 53eb5080130..b5c662b4a80 100644 --- a/src/Client/ClientBase.cpp +++ b/src/Client/ClientBase.cpp @@ -1834,7 +1834,7 @@ bool ClientBase::executeMultiQuery(const String & all_queries_text) { /// disable logs if expects errors TestHint test_hint(all_queries_text); - if (test_hint.clientError() || test_hint.serverError()) + if (!test_hint.clientErrors().empty() || !test_hint.serverErrors().empty()) processTextAsSingleQuery("SET send_logs_level = 'fatal'"); } @@ -1876,17 +1876,18 @@ bool ClientBase::executeMultiQuery(const String & all_queries_text) // the query ends because we failed to parse it, so we consume // the entire line. TestHint hint(String(this_query_begin, this_query_end - this_query_begin)); - if (hint.serverError()) + if (!hint.serverErrors().empty()) { // Syntax errors are considered as client errors - current_exception->addMessage("\nExpected server error '{}'.", hint.serverError()); + current_exception->addMessage("\nExpected server error: {}.", hint.serverErrors()); current_exception->rethrow(); } - if (hint.clientError() != current_exception->code()) + if (std::find(hint.clientErrors().begin(), hint.clientErrors().end(), current_exception->code()) + == hint.clientErrors().end()) { - if (hint.clientError()) - current_exception->addMessage("\nExpected client error: " + std::to_string(hint.clientError())); + if (!hint.clientErrors().empty()) + current_exception->addMessage("\nExpected client error: {}.", hint.clientErrors()); current_exception->rethrow(); } @@ -1935,37 +1936,41 @@ bool ClientBase::executeMultiQuery(const String & all_queries_text) bool error_matches_hint = true; if (have_error) { - if (test_hint.serverError()) + if (!test_hint.serverErrors().empty()) { 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); + test_hint.serverErrors(), full_query); } - else if (server_exception->code() != test_hint.serverError()) + else if ( + std::find(test_hint.serverErrors().begin(), test_hint.serverErrors().end(), server_exception->code()) + == test_hint.serverErrors().end()) { error_matches_hint = false; fmt::print(stderr, "Expected server error code: {} but got: {} (query: {}).\n", - test_hint.serverError(), server_exception->code(), full_query); + test_hint.serverErrors(), server_exception->code(), full_query); } } - if (test_hint.clientError()) + if (!test_hint.clientErrors().empty()) { 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); + test_hint.clientErrors(), full_query); } - else if (client_exception->code() != test_hint.clientError()) + else if ( + std::find(test_hint.clientErrors().begin(), test_hint.clientErrors().end(), client_exception->code()) + == test_hint.clientErrors().end()) { error_matches_hint = false; fmt::print(stderr, "Expected client error code '{}' but got '{}' (query: {}).\n", - test_hint.clientError(), client_exception->code(), full_query); + test_hint.clientErrors(), client_exception->code(), full_query); } } - if (!test_hint.clientError() && !test_hint.serverError()) + if (test_hint.clientErrors().empty() && test_hint.serverErrors().empty()) { // No error was expected but it still occurred. This is the // default case without test hint, doesn't need additional @@ -1975,19 +1980,19 @@ bool ClientBase::executeMultiQuery(const String & all_queries_text) } else { - if (test_hint.clientError()) + if (!test_hint.clientErrors().empty()) { error_matches_hint = false; fmt::print(stderr, "The query succeeded but the client error '{}' was expected (query: {}).\n", - test_hint.clientError(), full_query); + test_hint.clientErrors(), full_query); } - if (test_hint.serverError()) + if (!test_hint.serverErrors().empty()) { error_matches_hint = false; fmt::print(stderr, "The query succeeded but the server error '{}' was expected (query: {}).\n", - test_hint.serverError(), full_query); + test_hint.serverErrors(), full_query); } } diff --git a/src/Client/TestHint.cpp b/src/Client/TestHint.cpp index f6d1e5d73c3..adaae5fe5ee 100644 --- a/src/Client/TestHint.cpp +++ b/src/Client/TestHint.cpp @@ -6,25 +6,46 @@ #include #include +namespace DB::ErrorCodes +{ +extern const int CANNOT_PARSE_TEXT; +} + namespace { /// Parse error as number or as a string (name of the error code const) -int parseErrorCode(DB::ReadBufferFromString & in) +DB::TestHint::error_vector parseErrorCode(DB::ReadBufferFromString & in) { - int code = -1; - String code_name; + DB::TestHint::error_vector error_codes{}; - auto * pos = in.position(); - tryReadText(code, in); - if (pos != in.position()) + while (!in.eof()) { - return code; + int code = -1; + String code_name; + auto * pos = in.position(); + + tryReadText(code, in); + if (pos == in.position()) + { + readStringUntilWhitespace(code_name, in); + code = DB::ErrorCodes::getErrorCodeByName(code_name); + } + error_codes.push_back(code); + + if (in.eof()) + break; + skipWhitespaceIfAny(in); + if (in.eof()) + break; + char c; + in.readStrict(c); + if (c != '|') + throw DB::Exception(DB::ErrorCodes::CANNOT_PARSE_TEXT, "Expected separator '|'. Got '{}'", c); + skipWhitespaceIfAny(in); } - /// Try parse as string - readStringUntilWhitespace(code_name, in); - return DB::ErrorCodes::getErrorCodeByName(code_name); + return error_codes; } } @@ -85,9 +106,9 @@ void TestHint::parse(const String & hint, bool is_leading_hint) if (!is_leading_hint) { if (item == "serverError") - server_error = parseErrorCode(in); + server_errors = parseErrorCode(in); else if (item == "clientError") - client_error = parseErrorCode(in); + client_errors = parseErrorCode(in); } if (item == "echo") diff --git a/src/Client/TestHint.h b/src/Client/TestHint.h index 7fa4e86c025..30b3cacd3cb 100644 --- a/src/Client/TestHint.h +++ b/src/Client/TestHint.h @@ -1,6 +1,10 @@ #pragma once #include +#include + +#include + #include @@ -12,10 +16,13 @@ namespace DB /// The following comment hints are supported: /// /// - "-- { serverError 60 }" -- in case of you are expecting server error. +/// - "-- { serverError 16 | 36 }" -- in case of you are expecting one of the 2 errors /// /// - "-- { clientError 20 }" -- in case of you are expecting client error. +/// - "-- { clientError 20 | 60 | 92 }" -- It's expected that the client will return one of the 3 errors. /// /// - "-- { serverError FUNCTION_THROW_IF_VALUE_IS_NON_ZERO }" -- by error name. +/// - "-- { serverError NO_SUCH_COLUMN_IN_TABLE | BAD_ARGUMENTS }" -- by error name. /// /// - "-- { clientError FUNCTION_THROW_IF_VALUE_IS_NON_ZERO }" -- by error name. /// @@ -43,29 +50,67 @@ namespace DB class TestHint { public: + using error_vector = std::vector; TestHint(const String & query_); - int serverError() const { return server_error; } - int clientError() const { return client_error; } + const auto & serverErrors() const { return server_errors; } + const auto & clientErrors() const { return client_errors; } std::optional echoQueries() const { return echo; } private: const String & query; - int server_error = 0; - int client_error = 0; + error_vector server_errors{}; + error_vector client_errors{}; std::optional echo; void parse(const String & hint, bool is_leading_hint); bool allErrorsExpected(int actual_server_error, int actual_client_error) const { - return (server_error || client_error) && (server_error == actual_server_error) && (client_error == actual_client_error); + if (actual_server_error && std::find(server_errors.begin(), server_errors.end(), actual_server_error) == server_errors.end()) + return false; + if (!actual_server_error && server_errors.size()) + return false; + + if (actual_client_error && std::find(client_errors.begin(), client_errors.end(), actual_client_error) == client_errors.end()) + return false; + if (!actual_client_error && client_errors.size()) + return false; + + return true; } bool lostExpectedError(int actual_server_error, int actual_client_error) const { - return (server_error && !actual_server_error) || (client_error && !actual_client_error); + return (server_errors.size() && !actual_server_error) || (client_errors.size() && !actual_client_error); } }; } + +template <> +struct fmt::formatter +{ + static constexpr auto parse(format_parse_context & ctx) + { + const auto * it = ctx.begin(); + const auto * end = ctx.end(); + + /// Only support {}. + if (it != end && *it != '}') + throw format_error("Invalid format"); + + return it; + } + + template + auto format(const DB::TestHint::error_vector & error_vector, FormatContext & ctx) + { + if (error_vector.empty()) + return format_to(ctx.out(), "{}", 0); + else if (error_vector.size() == 1) + return format_to(ctx.out(), "{}", error_vector[0]); + else + return format_to(ctx.out(), "One of [{}]", fmt::join(error_vector, ", ")); + } +}; diff --git a/tests/queries/0_stateless/01470_columns_transformers.sql b/tests/queries/0_stateless/01470_columns_transformers.sql index 22c30ed36bf..7cff1920a4e 100644 --- a/tests/queries/0_stateless/01470_columns_transformers.sql +++ b/tests/queries/0_stateless/01470_columns_transformers.sql @@ -1,5 +1,3 @@ -SET allow_experimental_analyzer = 1; - DROP TABLE IF EXISTS columns_transformers; CREATE TABLE columns_transformers (i Int64, j Int16, k Int64) Engine=TinyLog; @@ -19,15 +17,15 @@ SELECT a.* APPLY(toDate) EXCEPT(i, j) APPLY(any) from columns_transformers a; SELECT * EXCEPT STRICT i from columns_transformers; SELECT * EXCEPT STRICT (i, j) from columns_transformers; SELECT * EXCEPT STRICT i, j1 from columns_transformers; -- { serverError 47 } -SELECT * EXCEPT STRICT(i, j1) from columns_transformers; -- { serverError 36 } +SELECT * EXCEPT STRICT(i, j1) from columns_transformers; -- { serverError NO_SUCH_COLUMN_IN_TABLE | BAD_ARGUMENTS } SELECT * REPLACE STRICT i + 1 AS i from columns_transformers; -SELECT * REPLACE STRICT(i + 1 AS col) from columns_transformers; -- { serverError 36 } +SELECT * REPLACE STRICT(i + 1 AS col) from columns_transformers; -- { serverError NO_SUCH_COLUMN_IN_TABLE | BAD_ARGUMENTS } SELECT * REPLACE(i + 1 AS i) APPLY(sum) from columns_transformers; SELECT columns_transformers.* REPLACE(j + 2 AS j, i + 1 AS i) APPLY(avg) from columns_transformers; SELECT columns_transformers.* REPLACE(j + 1 AS j, j + 2 AS j) APPLY(avg) from columns_transformers; -- { serverError 43 } -- REPLACE after APPLY will not match anything SELECT a.* APPLY(toDate) REPLACE(i + 1 AS i) APPLY(any) from columns_transformers a; -SELECT a.* APPLY(toDate) REPLACE STRICT(i + 1 AS i) APPLY(any) from columns_transformers a; -- { serverError 36 } +SELECT a.* APPLY(toDate) REPLACE STRICT(i + 1 AS i) APPLY(any) from columns_transformers a; -- { serverError NO_SUCH_COLUMN_IN_TABLE | BAD_ARGUMENTS } EXPLAIN SYNTAX SELECT * APPLY(sum) from columns_transformers; EXPLAIN SYNTAX SELECT columns_transformers.* APPLY(avg) from columns_transformers;