Fix tests

programs/client/Client.cpp

@@ -89,6 +89,7 @@ namespace ErrorCodes
     extern const int DEADLOCK_AVOIDED;
     extern const int SYNTAX_ERROR;
     extern const int TOO_DEEP_RECURSION;
+    extern const int NETWORK_ERROR;
 }
 
 
@@ -330,55 +331,71 @@ void Client::loadSuggestionDataIfPossible()
 
 int Client::mainImpl()
 {
-    registerFormats();
-    registerFunctions();
-    registerAggregateFunctions();
-
-    connect();
-
-    if (is_interactive)
+    try
     {
-        /// Load Warnings at the beginning of connection
-        if (!config().has("no-warnings"))
+        registerFormats();
+        registerFunctions();
+        registerAggregateFunctions();
+
+        processConfig();
+        connect();
+
+        if (is_interactive)
         {
-            try
+            /// Load Warnings at the beginning of connection
+            if (!config().has("no-warnings"))
             {
-                std::vector<String> messages = loadWarningMessages();
-                if (!messages.empty())
+                try
                 {
-                    std::cout << "Warnings:" << std::endl;
-                    for (const auto & message : messages)
-                        std::cout << " * " << message << std::endl;
-                    std::cout << std::endl;
+                    std::vector<String> messages = loadWarningMessages();
+                    if (!messages.empty())
+                    {
+                        std::cout << "Warnings:" << std::endl;
+                        for (const auto & message : messages)
+                            std::cout << " * " << message << std::endl;
+                        std::cout << std::endl;
+                    }
+                }
+                catch (...)
+                {
+                    /// Ignore exception
                 }
             }
-            catch (...)
+
+            runInteractive();
+        }
+        else
+        {
+            runNonInteractive();
+
+            // If exception code isn't zero, we should return non-zero return
+            // code anyway.
+            const auto * exception = server_exception ? server_exception.get() : client_exception.get();
+
+            if (exception)
             {
-                /// Ignore exception
+                return exception->code() != 0 ? exception->code() : -1;
+            }
+
+            if (have_error)
+            {
+                // Shouldn't be set without an exception, but check it just in
+                // case so that at least we don't lose an error.
+                return -1;
             }
         }
-
-        runInteractive();
     }
-    else
+    catch (const Exception & e)
     {
-        runNonInteractive();
-
-        // If exception code isn't zero, we should return non-zero return
-        // code anyway.
-        const auto * exception = server_exception ? server_exception.get() : client_exception.get();
-
-        if (exception)
-        {
-            return exception->code() != 0 ? exception->code() : -1;
-        }
-
-        if (have_error)
-        {
-            // Shouldn't be set without an exception, but check it just in
-            // case so that at least we don't lose an error.
-            return -1;
-        }
+        bool print_stack_trace = config().getBool("stacktrace", false) && e.code() != ErrorCodes::NETWORK_ERROR;
+        std::cerr << getExceptionMessage(e, print_stack_trace, true) << std::endl << std::endl;
+        /// If exception code isn't zero, we should return non-zero return code anyway.
+        return e.code() ? e.code() : -1;
+    }
+    catch (...)
+    {
+        std::cerr << getCurrentExceptionMessage(false) << std::endl;
+        return getCurrentExceptionCode();
     }
 
     return 0;

programs/local/LocalServer.cpp

@@ -242,8 +242,8 @@ void LocalServer::executeParsedQueryImpl()
         if (!config().hasOption("ignore-error"))
             throw;
 
-        if (!exception)
-            exception = std::current_exception();
+        if (!local_server_exception)
+            local_server_exception = std::current_exception();
 
         std::cerr << getCurrentExceptionMessage(config().hasOption("stacktrace")) << '\n';
     }
@@ -304,66 +304,93 @@ void LocalServer::setupUsers()
 
 int LocalServer::mainImpl()
 {
-    ThreadStatus thread_status;
-
-    if (is_interactive)
+    try
     {
-        std::map<String, String> prompt_substitutions{{"display_name", server_display_name}};
-        for (const auto & [key, value] : prompt_substitutions)
-            boost::replace_all(prompt_by_server_display_name, "{" + key + "}", value);
-    }
+        ThreadStatus thread_status;
 
-    /// We will terminate process on error
-    static KillingErrorHandler error_handler;
-    Poco::ErrorHandler::set(&error_handler);
-
-    /// Don't initialize DateLUT
-    registerFunctions();
-    registerAggregateFunctions();
-    registerTableFunctions();
-    registerStorages();
-    registerDictionaries();
-    registerDisks();
-    registerFormats();
-
-    /// we can't mutate global_context (can lead to races, as it was already passed to some background threads)
-    /// so we can't reuse it safely as a query context and need a copy here
-    query_context = Context::createCopy(global_context);
-    query_context->makeSessionContext();
-    query_context->makeQueryContext();
-    query_context->setUser("default", "", Poco::Net::SocketAddress{});
-    query_context->setCurrentQueryId("");
-
-    applyCmdSettings(query_context);
-    /// Use the same query_id (and thread group) for all queries
-    CurrentThread::QueryScope query_scope_holder(query_context);
-
-    if (need_render_progress)
-    {
-        /// Set progress callback, which can be run from multiple threads.
-        query_context->setProgressCallback([&](const Progress & value)
+        if (is_interactive)
         {
-            /// Write progress only if progress was updated
-            if (progress_indication.updateProgress(value))
-                progress_indication.writeProgress();
-        });
+            std::map<String, String> prompt_substitutions{{"display_name", server_display_name}};
+            for (const auto & [key, value] : prompt_substitutions)
+                boost::replace_all(prompt_by_server_display_name, "{" + key + "}", value);
+        }
 
-        /// Set callback for file processing progress.
-        progress_indication.setFileProgressCallback(query_context);
+        /// We will terminate process on error
+        static KillingErrorHandler error_handler;
+        Poco::ErrorHandler::set(&error_handler);
+
+        /// Don't initialize DateLUT
+        registerFunctions();
+        registerAggregateFunctions();
+        registerTableFunctions();
+        registerStorages();
+        registerDictionaries();
+        registerDisks();
+        registerFormats();
+
+        processConfig();
+
+        /// we can't mutate global_context (can lead to races, as it was already passed to some background threads)
+        /// so we can't reuse it safely as a query context and need a copy here
+        query_context = Context::createCopy(global_context);
+        query_context->makeSessionContext();
+        query_context->makeQueryContext();
+        query_context->setUser("default", "", Poco::Net::SocketAddress{});
+        query_context->setCurrentQueryId("");
+
+        applyCmdSettings(query_context);
+        /// Use the same query_id (and thread group) for all queries
+        CurrentThread::QueryScope query_scope_holder(query_context);
+
+        if (need_render_progress)
+        {
+            /// Set progress callback, which can be run from multiple threads.
+            query_context->setProgressCallback([&](const Progress & value)
+            {
+                /// Write progress only if progress was updated
+                if (progress_indication.updateProgress(value))
+                    progress_indication.writeProgress();
+            });
+
+            /// Set callback for file processing progress.
+            progress_indication.setFileProgressCallback(query_context);
+        }
+
+        if (is_interactive)
+        {
+            runInteractive();
+        }
+        else
+        {
+            runNonInteractive();
+
+            if (local_server_exception)
+                std::rethrow_exception(local_server_exception);
+        }
+
+        global_context->shutdown();
+        global_context.reset();
+
+        status.reset();
+        cleanup();
+
+        return Application::EXIT_OK;
     }
+    catch (const Exception & e)
+    {
+        try
+        {
+            cleanup();
+        }
+        catch (...)
+        {
+            tryLogCurrentException(__PRETTY_FUNCTION__);
+        }
 
-    if (is_interactive)
-        runInteractive();
-    else
-        runNonInteractive();
-
-    global_context->shutdown();
-    global_context.reset();
-
-    status.reset();
-    cleanup();
-
-    return Application::EXIT_OK;
+        std::cerr << getCurrentExceptionMessage(config().hasOption("stacktrace")) << '\n';
+        /// If exception code isn't zero, we should return non-zero return code anyway.
+        return e.code() ? e.code() : -1;
+    }
 }
 
 

programs/local/LocalServer.h

@@ -90,12 +90,6 @@ protected:
         return processMultiQuery(text);
     }
 
-    void checkExceptions() override
-    {
-        if (exception)
-            std::rethrow_exception(exception);
-    }
-
 private:
     /** Composes CREATE subquery based on passed arguments (--structure --file --table and --input-format)
       * This query will be executed first, before queries passed through --query argument
@@ -119,7 +113,7 @@ private:
 
     std::optional<StatusFile> status;
 
-    std::exception_ptr exception;
+    std::exception_ptr local_server_exception;
 
     void processQuery(const String & query, std::exception_ptr exception);
 

src/Client/ClientBase.cpp

@@ -49,7 +49,6 @@ namespace ErrorCodes
 {
     extern const int UNRECOGNIZED_ARGUMENTS;
     extern const int BAD_ARGUMENTS;
-    extern const int NETWORK_ERROR;
 }
 
 
@@ -571,8 +570,6 @@ void ClientBase::runNonInteractive()
         processWithFuzzing(text);
     else
         processQueryText(text);
-
-    checkExceptions();
 }
 
 
@@ -595,38 +592,18 @@ static void showClientVersion()
 
 int ClientBase::main(const std::vector<std::string> & /*args*/)
 {
-    try
+    UseSSL use_ssl;
+
+    std::cout << std::fixed << std::setprecision(3);
+    std::cerr << std::fixed << std::setprecision(3);
+
+    if (is_interactive)
     {
-        UseSSL use_ssl;
-
-        processConfig();
-
-        std::cout << std::fixed << std::setprecision(3);
-        std::cerr << std::fixed << std::setprecision(3);
-
-        if (is_interactive)
-        {
-            clearTerminal();
-            showClientVersion();
-        }
-
-        return mainImpl();
+        clearTerminal();
+        showClientVersion();
     }
-    catch (const Exception & e)
-    {
-        shutdown();
 
-        bool print_stack_trace = config().getBool("stacktrace", false) && e.code() != ErrorCodes::NETWORK_ERROR;
-        std::cerr << getExceptionMessage(e, print_stack_trace, true) << std::endl << std::endl;
-
-        /// If exception code isn't zero, we should return non-zero return code anyway.
-        return e.code() ? e.code() : -1;
-    }
-    catch (...)
-    {
-        std::cerr << getCurrentExceptionMessage(false) << std::endl;
-        return getCurrentExceptionCode();
-    }
+    return mainImpl();
 }
 
 

src/Client/ClientBase.h

@@ -64,6 +64,7 @@ protected:
 
     virtual bool validateParsedOptions() { return false; }
 
+
     void runInteractive();
 
     void runNonInteractive();
@@ -74,11 +75,13 @@ protected:
      * - 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.
     **/
-    virtual bool processMultiQuery(const String & all_queries_text) = 0;
     bool processMultiQueryImpl(const String & all_queries_text,
                                std::function<void(const String &)> process_single_query,
                                std::function<void(const String &, Exception &)> process_parse_query_error = {});
 
+    /// Method to implement multi-query processing. Must call processMultiQueryImpl.
+    virtual bool processMultiQuery(const String & all_queries_text) = 0;
+
     /// Process single file (with queries) from non-interactive mode.
     virtual bool processMultiQueryFromFile(const String & file) = 0;
 
@@ -102,8 +105,6 @@ protected:
 
     void resetOutput();
 
-    virtual void checkExceptions() {}
-
     virtual void reportQueryError() const = 0;
 
     virtual void loadSuggestionDataIfPossible() {}