Merge pull request #19361 from ClickHouse/aku/reconnect

Reconnect after client errors

programs/client/Client.cpp

@@ -680,6 +680,12 @@ private:
 
                     std::cerr << std::endl;
 
+                    client_exception = std::make_unique<Exception>(e);
+                }
+
+                if (client_exception)
+                {
+                    /// client_exception may have been set above or elsewhere.
                     /// Client-side exception during query execution can result in the loss of
                     /// sync in the connection protocol.
                     /// So we reconnect and allow to enter the next query.
@@ -914,12 +920,6 @@ private:
 
     void reportQueryError() const
     {
-        // If we probably have progress bar, we should add additional
-        // newline, otherwise exception may display concatenated with
-        // the progress bar.
-        if (need_render_progress)
-            std::cerr << '\n';
-
         if (server_exception)
         {
             std::string text = server_exception->displayText();
@@ -937,7 +937,7 @@ private:
         if (client_exception)
         {
             fmt::print(stderr,
-                "Error on processing query '{}':\n{}",
+                "Error on processing query '{}':\n{}\n",
                 full_query, client_exception->message());
         }
 

tests/clickhouse-test

@@ -649,7 +649,10 @@ def main(args):
     def is_test_from_dir(suite_dir, case):
         case_file = os.path.join(suite_dir, case)
         (name, ext) = os.path.splitext(case)
-        return os.path.isfile(case_file) and (ext == '.sql' or ext == '.sh' or ext == '.py')
+        # We could also test for executable files (os.access(case_file, os.X_OK),
+        # but it interferes with 01610_client_spawn_editor.editor, which is invoked
+        # as a query editor in the test, and must be marked as executable.
+        return os.path.isfile(case_file) and (ext == '.sql' or ext == '.sh' or ext == '.py' or ext == '.expect')
 
     def sute_key_func(item):
        if args.order == 'random':

tests/queries/0_stateless/00746_sql_fuzzy.sh

@@ -14,7 +14,7 @@ $CLICKHOUSE_CLIENT -q "select name from system.table_functions format TSV;" > "$
 # if you want long run use: env SQL_FUZZY_RUNS=100000 clickhouse-test sql_fuzzy
 
 for SQL_FUZZY_RUN in $(seq "${SQL_FUZZY_RUNS:=10}"); do
-    env SQL_FUZZY_RUN="$SQL_FUZZY_RUN" "$CURDIR"/00746_sql_fuzzy.pl | timeout 60 $CLICKHOUSE_CLIENT --format Null --max_execution_time 10 -n --ignore-error >/dev/null 2>&1
+    env SQL_FUZZY_RUN="$SQL_FUZZY_RUN" perl "$CURDIR"/00746_sql_fuzzy.pl | timeout 60 $CLICKHOUSE_CLIENT --format Null --max_execution_time 10 -n --ignore-error >/dev/null 2>&1
     if [[ $($CLICKHOUSE_CLIENT -q "SELECT 'Still alive'") != 'Still alive' ]]; then
         break
     fi

tests/queries/0_stateless/00921_datetime64_compatibility.sh

@@ -11,6 +11,6 @@ CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
 
 # ${CURDIR}/00921_datetime64_compatibility.python
 
-"${CURDIR}"/00921_datetime64_compatibility.python \
+python3 "${CURDIR}"/00921_datetime64_compatibility.python \
     | ${CLICKHOUSE_CLIENT} --ignore-error -T -nm --calculate_text_stack_trace 0 --log-level 'error' 2>&1 \
     | sed 's/Received exception .*//g; s/^\(Code: [0-9]\+\).*$/\1/g'

tests/queries/0_stateless/01293_client_interactive_vertical_multiline.expect

@@ -1,10 +1,18 @@
 #!/usr/bin/expect -f
 
 log_user 0
-set timeout 60
-spawn bash -c "$env(CLICKHOUSE_CLIENT_BINARY) $env(CLICKHOUSE_CLIENT_OPT)"
+set timeout 5
 match_max 100000
 
+# A default timeout action is to do nothing, change it to fail
+expect_after {
+    timeout {
+        exit 1
+    }
+}
+
+set basedir [file dirname $argv0]
+spawn bash -c "source $basedir/../shell_config.sh ; \$CLICKHOUSE_CLIENT_BINARY \$CLICKHOUSE_CLIENT_OPT"
 expect ":) "
 
 send -- "SELECT 1\r"
@@ -33,10 +41,7 @@ expect ":) "
 send -- ""
 expect eof
 
-set timeout 60
-spawn bash -c "$env(CLICKHOUSE_CLIENT_BINARY) $env(CLICKHOUSE_CLIENT_OPT) --multiline"
-match_max 100000
-
+spawn bash -c "source $basedir/../shell_config.sh ; \$CLICKHOUSE_CLIENT_BINARY \$CLICKHOUSE_CLIENT_OPT"
 expect ":) "
 
 send -- "SELECT 1;\r"

tests/queries/0_stateless/01293_client_interactive_vertical_multiline.sh

@@ -1,7 +0,0 @@
-#!/usr/bin/env bash
-
-CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
-# shellcheck source=../shell_config.sh
-. "$CURDIR"/../shell_config.sh
-
-${CURDIR}/01293_client_interactive_vertical_multiline.expect

tests/queries/0_stateless/01293_client_interactive_vertical_singleline.expect

@@ -2,9 +2,16 @@
 
 log_user 0
 set timeout 60
-spawn bash -c "$env(CLICKHOUSE_CLIENT_BINARY) $env(CLICKHOUSE_CLIENT_OPT)"
 match_max 100000
+# A default timeout action is to do nothing, change it to fail
+expect_after {
+    timeout {
+        exit 1
+    }
+}
 
+set basedir [file dirname $argv0]
+spawn bash -c "source $basedir/../shell_config.sh ; \$CLICKHOUSE_CLIENT_BINARY \$CLICKHOUSE_CLIENT_OPT"
 expect ":) "
 
 send -- "SELECT 1\r"

tests/queries/0_stateless/01293_client_interactive_vertical_singleline.sh

@@ -1,7 +0,0 @@
-#!/usr/bin/env bash
-
-CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
-# shellcheck source=../shell_config.sh
-. "$CURDIR"/../shell_config.sh
-
-${CURDIR}/01293_client_interactive_vertical_singleline.expect

tests/queries/0_stateless/01300_client_save_history_when_terminated.expect

@@ -3,8 +3,15 @@
 log_user 0
 set timeout 60
 match_max 100000
+# A default timeout action is to do nothing, change it to fail
+expect_after {
+    timeout {
+        exit 1
+    }
+}
+set basedir [file dirname $argv0]
 
-spawn bash -c "$env(CLICKHOUSE_CLIENT_BINARY) $env(CLICKHOUSE_CLIENT_OPT)"
+spawn bash -c "source $basedir/../shell_config.sh ; \$CLICKHOUSE_CLIENT_BINARY \$CLICKHOUSE_CLIENT_OPT"
 expect ":) "
 
 # Make a query
@@ -17,7 +24,7 @@ exec kill -9 [exp_pid]
 close
 
 # Run client one more time and press "up" to see the last recorded query
-spawn bash -c "$env(CLICKHOUSE_CLIENT_BINARY) $env(CLICKHOUSE_CLIENT_OPT)"
+spawn bash -c "source $basedir/../shell_config.sh ; \$CLICKHOUSE_CLIENT_BINARY \$CLICKHOUSE_CLIENT_OPT"
 expect ":) "
 send -- "\[A"
 expect "SELECT 'for the history'"

tests/queries/0_stateless/01300_client_save_history_when_terminated.sh

@@ -1,7 +0,0 @@
-#!/usr/bin/env bash
-
-CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
-# shellcheck source=../shell_config.sh
-. "$CURDIR"/../shell_config.sh
-
-${CURDIR}/01300_client_save_history_when_terminated.expect

tests/queries/0_stateless/01370_client_autocomplete_word_break_characters.expect

@@ -3,8 +3,15 @@
 log_user 0
 set timeout 1
 match_max 100000
+# A default timeout action is to do nothing, change it to fail
+expect_after {
+    timeout {
+        exit 1
+    }
+}
 
-spawn bash -c "$env(CLICKHOUSE_CLIENT_BINARY) $env(CLICKHOUSE_CLIENT_OPT)"
+set basedir [file dirname $argv0]
+spawn bash -c "source $basedir/../shell_config.sh ; \$CLICKHOUSE_CLIENT_BINARY \$CLICKHOUSE_CLIENT_OPT"
 expect ":) "
 
 # Make a query

tests/queries/0_stateless/01370_client_autocomplete_word_break_characters.sh

@@ -1,7 +0,0 @@
-#!/usr/bin/env bash
-
-CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
-# shellcheck source=../shell_config.sh
-. "$CURDIR"/../shell_config.sh
-
-${CURDIR}/01370_client_autocomplete_word_break_characters.expect

tests/queries/0_stateless/01520_client_print_query_id.expect

@@ -3,8 +3,15 @@
 log_user 0
 set timeout 5
 match_max 100000
+# A default timeout action is to do nothing, change it to fail
+expect_after {
+    timeout {
+        exit 1
+    }
+}
 
-spawn bash -c "$env(CLICKHOUSE_CLIENT_BINARY) $env(CLICKHOUSE_CLIENT_OPT)"
+set basedir [file dirname $argv0]
+spawn bash -c "source $basedir/../shell_config.sh ; \$CLICKHOUSE_CLIENT_BINARY \$CLICKHOUSE_CLIENT_OPT"
 expect ":) "
 
 # Make a query

tests/queries/0_stateless/01520_client_print_query_id.sh

@@ -1,7 +0,0 @@
-#!/usr/bin/env bash
-
-CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
-# shellcheck source=../shell_config.sh
-. "$CURDIR"/../shell_config.sh
-
-${CURDIR}/01520_client_print_query_id.expect

tests/queries/0_stateless/01526_client_start_and_exit.sh

@@ -10,7 +10,7 @@ ${CLICKHOUSE_CLIENT} -q "SELECT 'CREATE TABLE test_' || hex(randomPrintableASCII
 function stress()
 {
     while true; do
-        "${CURDIR}"/01526_client_start_and_exit.expect | grep -v -P 'ClickHouse client|Connecting|Connected|:\) Bye\.|new year|^\s*$|spawn bash|^0\s*$'
+        "${CURDIR}"/01526_client_start_and_exit.expect-not-a-test-case | grep -v -P 'ClickHouse client|Connecting|Connected|:\) Bye\.|new year|^\s*$|spawn bash|^0\s*$'
     done
 }
 

tests/queries/0_stateless/01564_test_hint_woes.reference

@@ -23,5 +23,9 @@ select nonexistent column; -- { serverError 47 }
 insert into values_01564 values (1); select 1;
 select 1;
 1
+-- a failing insert and then a normal insert (#https://github.com/ClickHouse/ClickHouse/issues/19353)
+CREATE TABLE t0 (c0 String, c1 Int32) ENGINE = Memory() ;
+INSERT INTO t0(c0, c1) VALUES ("1",1) ; -- { clientError 47 }
+INSERT INTO t0(c0, c1) VALUES ('1', 1) ;
 -- the return code must be zero after the final query has failed with expected error
 insert into values_01564 values (11); -- { serverError 469 }

tests/queries/0_stateless/01564_test_hint_woes.sql

@@ -42,5 +42,10 @@ OPTIMIZE TABLE values_01564 DEDUPLICATE BY; -- { clientError 62 }
 OPTIMIZE TABLE values_01564 DEDUPLICATE BY a EXCEPT a; -- { clientError 62 }
 select 'a' || distinct one || 'c' from system.one; -- { clientError 62 }
 
+-- a failing insert and then a normal insert (#https://github.com/ClickHouse/ClickHouse/issues/19353)
+CREATE TABLE t0 (c0 String, c1 Int32) ENGINE = Memory() ;
+INSERT INTO t0(c0, c1) VALUES ("1",1) ; -- { clientError 47 }
+INSERT INTO t0(c0, c1) VALUES ('1', 1) ;
+
 -- the return code must be zero after the final query has failed with expected error
 insert into values_01564 values (11); -- { serverError 469 }

tests/queries/0_stateless/01565_reconnect_after_client_error.expect

@@ -0,0 +1,39 @@
+#!/usr/bin/expect -f
+
+# This is a separate test, because we want to test the interactive mode.
+# https://github.com/ClickHouse/ClickHouse/issues/19353
+
+log_user 0
+set timeout 5
+match_max 100000
+
+# A default timeout action is to fail
+expect_after {
+    timeout {
+        exit 1
+    }
+}
+
+set basedir [file dirname $argv0]
+spawn bash -c "source $basedir/../shell_config.sh ; \$CLICKHOUSE_CLIENT_BINARY \$CLICKHOUSE_CLIENT_OPT -mn"
+expect "\n:) "
+
+send -- "DROP TABLE IF EXISTS t01565;\n"
+expect "\nOk."
+expect "\n:)"
+
+send -- "CREATE TABLE t01565 (c0 String, c1 Int32) ENGINE = Memory() ;\n"
+expect "\nOk."
+expect "\n:) "
+
+send -- "INSERT INTO t01565(c0, c1) VALUES (\"1\",1) ;\n"
+expect "\nConnected"
+expect "\n:) "
+
+send -- "INSERT INTO t01565(c0, c1) VALUES ('1', 1) ;\n"
+expect "\nOk."
+expect "\n:) "
+
+# Ctrl+D
+send -- "\4"
+expect eof