Merge pull request #55193 from azat/clickhouse-client-pager-signals

Reset signals caught by clickhouse-client if a pager is in use

programs/client/Client.cpp

@@ -320,6 +320,7 @@ try
     registerAggregateFunctions();
 
     processConfig();
+    adjustSettings();
     initTtyBuffer(toProgressOption(config().getString("progress", "default")));
 
     {
@@ -1238,6 +1239,8 @@ void Client::processConfig()
     if (config().has("multiquery"))
         is_multiquery = true;
 
+    pager = config().getString("pager", "");
+
     is_default_format = !config().has("vertical") && !config().has("format");
     if (config().has("vertical"))
         format = config().getString("format", "Vertical");
@@ -1264,15 +1267,6 @@ void Client::processConfig()
     global_context->setQueryKindInitial();
     global_context->setQuotaClientKey(config().getString("quota_key", ""));
     global_context->setQueryKind(query_kind);
-
-    if (is_multiquery && !global_context->getSettingsRef().input_format_values_allow_data_after_semicolon.changed)
-    {
-        Settings settings = global_context->getSettings();
-        settings.input_format_values_allow_data_after_semicolon = true;
-        /// Do not send it to the server
-        settings.input_format_values_allow_data_after_semicolon.changed = false;
-        global_context->setSettings(settings);
-    }
 }
 
 

programs/local/LocalServer.cpp

@@ -494,6 +494,7 @@ try
     registerFormats();
 
     processConfig();
+    adjustSettings();
     initTtyBuffer(toProgressOption(config().getString("progress", "default")));
 
     applyCmdSettings(global_context);
@@ -577,6 +578,8 @@ void LocalServer::processConfig()
     if (config().has("multiquery"))
         is_multiquery = true;
 
+    pager = config().getString("pager", "");
+
     delayed_interactive = config().has("interactive") && (!queries.empty() || config().has("queries-file"));
     if (!is_interactive || delayed_interactive)
     {
@@ -783,15 +786,6 @@ void LocalServer::processConfig()
 
     global_context->setQueryKindInitial();
     global_context->setQueryKind(query_kind);
-
-    if (is_multiquery && !global_context->getSettingsRef().input_format_values_allow_data_after_semicolon.changed)
-    {
-        Settings settings = global_context->getSettings();
-        settings.input_format_values_allow_data_after_semicolon = true;
-        /// Do not send it to the server
-        settings.input_format_values_allow_data_after_semicolon.changed = false;
-        global_context->setSettings(settings);
-    }
 }
 
 

src/Client/ClientBase.cpp

@@ -72,6 +72,7 @@
 #include <boost/algorithm/string/replace.hpp>
 #include <iostream>
 #include <filesystem>
+#include <limits>
 #include <map>
 #include <memory>
 #include <unordered_map>
@@ -560,11 +561,19 @@ try
         }
 
         WriteBuffer * out_buf = nullptr;
-        String pager = config().getString("pager", "");
         if (!pager.empty())
         {
             if (SIG_ERR == signal(SIGPIPE, SIG_IGN))
-                throwFromErrno("Cannot set signal handler.", ErrorCodes::CANNOT_SET_SIGNAL_HANDLER);
+                throwFromErrno("Cannot set signal handler for SIGPIPE.", ErrorCodes::CANNOT_SET_SIGNAL_HANDLER);
+            /// We need to reset signals that had been installed in the
+            /// setupSignalHandler() since terminal will send signals to both
+            /// processes and so signals will be delivered to the
+            /// clickhouse-client/local as well, which will be terminated when
+            /// signal will be delivered second time.
+            if (SIG_ERR == signal(SIGINT, SIG_IGN))
+                throwFromErrno("Cannot set signal handler for SIGINT.", ErrorCodes::CANNOT_SET_SIGNAL_HANDLER);
+            if (SIG_ERR == signal(SIGQUIT, SIG_IGN))
+                throwFromErrno("Cannot set signal handler for SIGQUIT.", ErrorCodes::CANNOT_SET_SIGNAL_HANDLER);
 
             ShellCommand::Config config(pager);
             config.pipe_stdin_only = true;
@@ -710,6 +719,30 @@ void ClientBase::initLogsOutputStream()
     }
 }
 
+void ClientBase::adjustSettings()
+{
+    Settings settings = global_context->getSettings();
+
+    /// NOTE: Do not forget to set changed=false to avoid sending it to the server (to avoid breakage read only profiles)
+
+    /// In case of multi-query we allow data after semicolon since it will be
+    /// parsed by the client and interpreted as new query
+    if (is_multiquery && !global_context->getSettingsRef().input_format_values_allow_data_after_semicolon.changed)
+    {
+        settings.input_format_values_allow_data_after_semicolon = true;
+        settings.input_format_values_allow_data_after_semicolon.changed = false;
+    }
+
+    /// If pager is specified then output_format_pretty_max_rows is ignored, this should be handled by pager.
+    if (!pager.empty() && !global_context->getSettingsRef().output_format_pretty_max_rows.changed)
+    {
+        settings.output_format_pretty_max_rows = std::numeric_limits<UInt64>::max();
+        settings.output_format_pretty_max_rows.changed = false;
+    }
+
+    global_context->setSettings(settings);
+}
+
 void ClientBase::initTtyBuffer(ProgressOption progress)
 {
     if (tty_buf)
@@ -1300,6 +1333,15 @@ void ClientBase::resetOutput()
     {
         pager_cmd->in.close();
         pager_cmd->wait();
+
+        if (SIG_ERR == signal(SIGPIPE, SIG_DFL))
+            throwFromErrno("Cannot set signal handler for SIIGPIEP.", ErrorCodes::CANNOT_SET_SIGNAL_HANDLER);
+        if (SIG_ERR == signal(SIGINT, SIG_DFL))
+            throwFromErrno("Cannot set signal handler for SIGINT.", ErrorCodes::CANNOT_SET_SIGNAL_HANDLER);
+        if (SIG_ERR == signal(SIGQUIT, SIG_DFL))
+            throwFromErrno("Cannot set signal handler for SIGQUIT.", ErrorCodes::CANNOT_SET_SIGNAL_HANDLER);
+
+        setupSignalHandler();
     }
     pager_cmd = nullptr;
 

src/Client/ClientBase.h

@@ -58,8 +58,6 @@ enum ProgressOption
 ProgressOption toProgressOption(std::string progress);
 std::istream& operator>> (std::istream & in, ProgressOption & progress);
 
-void interruptSignalHandler(int signum);
-
 class InternalTextLogs;
 class WriteBufferFromFileDescriptor;
 
@@ -184,6 +182,9 @@ protected:
     static bool isSyncInsertWithData(const ASTInsertQuery & insert_query, const ContextPtr & context);
     bool processMultiQueryFromFile(const String & file_name);
 
+    /// Adjust some settings after command line options and config had been processed.
+    void adjustSettings();
+
     void initTtyBuffer(ProgressOption progress);
 
     /// Should be one of the first, to be destroyed the last,
@@ -212,6 +213,8 @@ protected:
     bool stderr_is_a_tty = false; /// stderr is a terminal.
     uint64_t terminal_width = 0;
 
+    String pager;
+
     String format; /// Query results output format.
     bool select_into_file = false; /// If writing result INTO OUTFILE. It affects progress rendering.
     bool select_into_file_and_stdout = false; /// If writing result INTO OUTFILE AND STDOUT. It affects progress rendering.