Improvements based on the PR

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.clientErrors().empty() || !test_hint.serverErrors().empty())
+        if (test_hint.hasClientErrors() || test_hint.hasServerErrors())
             processTextAsSingleQuery("SET send_logs_level = 'fatal'");
     }
 
@@ -1876,7 +1876,7 @@ 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.serverErrors().empty())
+                if (hint.hasServerErrors())
                 {
                     // Syntax errors are considered as client errors
                     current_exception->addMessage("\nExpected server error: {}.", hint.serverErrors());
@@ -1886,7 +1886,7 @@ bool ClientBase::executeMultiQuery(const String & all_queries_text)
                 if (std::find(hint.clientErrors().begin(), hint.clientErrors().end(), current_exception->code())
                     == hint.clientErrors().end())
                 {
-                    if (!hint.clientErrors().empty())
+                    if (hint.hasClientErrors())
                         current_exception->addMessage("\nExpected client error: {}.", hint.clientErrors());
 
                     current_exception->rethrow();
@@ -1936,7 +1936,7 @@ bool ClientBase::executeMultiQuery(const String & all_queries_text)
                 bool error_matches_hint = true;
                 if (have_error)
                 {
-                    if (!test_hint.serverErrors().empty())
+                    if (test_hint.hasServerErrors())
                     {
                         if (!server_exception)
                         {
@@ -1953,7 +1953,7 @@ bool ClientBase::executeMultiQuery(const String & all_queries_text)
                                        test_hint.serverErrors(), server_exception->code(), full_query);
                         }
                     }
-                    if (!test_hint.clientErrors().empty())
+                    if (test_hint.hasClientErrors())
                     {
                         if (!client_exception)
                         {
@@ -1970,7 +1970,7 @@ bool ClientBase::executeMultiQuery(const String & all_queries_text)
                                        test_hint.clientErrors(), client_exception->code(), full_query);
                         }
                     }
-                    if (test_hint.clientErrors().empty() && test_hint.serverErrors().empty())
+                    if (!test_hint.hasClientErrors() && !test_hint.hasServerErrors())
                     {
                         // No error was expected but it still occurred. This is the
                         // default case without test hint, doesn't need additional
@@ -1980,14 +1980,14 @@ bool ClientBase::executeMultiQuery(const String & all_queries_text)
                 }
                 else
                 {
-                    if (!test_hint.clientErrors().empty())
+                    if (test_hint.hasClientErrors())
                     {
                         error_matches_hint = false;
                         fmt::print(stderr,
                                    "The query succeeded but the client error '{}' was expected (query: {}).\n",
                                    test_hint.clientErrors(), full_query);
                     }
-                    if (!test_hint.serverErrors().empty())
+                    if (test_hint.hasServerErrors())
                     {
                         error_matches_hint = false;
                         fmt::print(stderr,

src/Client/TestHint.cpp

@@ -9,7 +9,7 @@
 
 namespace DB::ErrorCodes
 {
-extern const int CANNOT_PARSE_TEXT;
+    extern const int CANNOT_PARSE_TEXT;
 }
 
 namespace DB
@@ -54,9 +54,9 @@ TestHint::TestHint(const String & query_)
 
 void TestHint::parse(Lexer & comment_lexer, bool is_leading_hint)
 {
-    std::unordered_set<String> commands{"echo", "echoOn", "echoOff"};
+    std::unordered_set<std::string_view> commands{"echo", "echoOn", "echoOff"};
 
-    std::unordered_set<String> command_errors{
+    std::unordered_set<std::string_view> command_errors{
         "serverError",
         "clientError",
     };
@@ -76,7 +76,7 @@ void TestHint::parse(Lexer & comment_lexer, bool is_leading_hint)
         else if (!is_leading_hint && token.type == TokenType::BareWord && command_errors.contains(item))
         {
             /// Everything after this must be a list of errors separated by comma
-            error_vector error_codes;
+            ErrorVector error_codes;
             while (!token.isEnd())
             {
                 token = comment_lexer.nextToken();

src/Client/TestHint.h

@@ -18,7 +18,7 @@ class Lexer;
 /// 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
+/// - "-- { 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.
@@ -52,17 +52,20 @@ class Lexer;
 class TestHint
 {
 public:
-    using error_vector = std::vector<int>;
+    using ErrorVector = std::vector<int>;
     TestHint(const String & query_);
 
     const auto & serverErrors() const { return server_errors; }
     const auto & clientErrors() const { return client_errors; }
     std::optional<bool> echoQueries() const { return echo; }
 
+    bool hasClientErrors() { return !client_errors.empty(); }
+    bool hasServerErrors() { return !server_errors.empty(); }
+
 private:
     const String & query;
-    error_vector server_errors{};
+    ErrorVector server_errors{};
-    error_vector client_errors{};
+    ErrorVector client_errors{};
     std::optional<bool> echo;
 
     void parse(Lexer & comment_lexer, bool is_leading_hint);
@@ -91,7 +94,7 @@ private:
 }
 
 template <>
-struct fmt::formatter<DB::TestHint::error_vector>
+struct fmt::formatter<DB::TestHint::ErrorVector>
 {
     static constexpr auto parse(format_parse_context & ctx)
     {
@@ -106,13 +109,13 @@ struct fmt::formatter<DB::TestHint::error_vector>
     }
 
     template <typename FormatContext>
-    auto format(const DB::TestHint::error_vector & error_vector, FormatContext & ctx)
+    auto format(const DB::TestHint::ErrorVector & ErrorVector, FormatContext & ctx)
     {
-        if (error_vector.empty())
+        if (ErrorVector.empty())
             return format_to(ctx.out(), "{}", 0);
-        else if (error_vector.size() == 1)
+        else if (ErrorVector.size() == 1)
-            return format_to(ctx.out(), "{}", error_vector[0]);
+            return format_to(ctx.out(), "{}", ErrorVector[0]);
         else
-            return format_to(ctx.out(), "One of [{}]", fmt::join(error_vector, ", "));
+            return format_to(ctx.out(), "[{}]", fmt::join(ErrorVector, ", "));
     }
 };