Skip to content

Commit e780c12

Browse files
committed
Add tests, remove saving host in config
1 parent 03b4945 commit e780c12

File tree

6 files changed

+92
-45
lines changed

6 files changed

+92
-45
lines changed

programs/client/Client.cpp

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,12 @@
11
#include <stdlib.h>
22
#include <fcntl.h>
3-
#include <signal.h>
43
#include <map>
54
#include <iostream>
6-
#include <fstream>
75
#include <iomanip>
8-
#include <unordered_set>
9-
#include <algorithm>
106
#include <optional>
117
#include <base/scope_guard_safe.h>
128
#include <boost/program_options.hpp>
139
#include <boost/algorithm/string/replace.hpp>
14-
#include <Poco/String.h>
1510
#include <filesystem>
1611
#include <string>
1712
#include "Client.h"
@@ -24,6 +19,7 @@
2419
#include <Common/formatReadable.h>
2520
#include <Common/TerminalSize.h>
2621
#include <Common/Config/configReadClient.h>
22+
#include <Common/DNSResolver.h>
2723

2824
#include <Core/QueryProcessingStage.h>
2925
#include <Client/TestHint.h>
@@ -33,13 +29,13 @@
3329
#include <IO/ReadBufferFromString.h>
3430
#include <IO/ReadHelpers.h>
3531
#include <IO/WriteHelpers.h>
36-
#include <IO/Operators.h>
3732
#include <IO/WriteBufferFromOStream.h>
3833
#include <IO/UseSSL.h>
3934

4035
#include <Parsers/ASTCreateQuery.h>
4136
#include <Parsers/ASTDropQuery.h>
42-
#include <Parsers/ASTIdentifier.h>
37+
#include <Parsers/ASTSetQuery.h>
38+
#include <Parsers/ASTUseQuery.h>
4339
#include <Parsers/ASTInsertQuery.h>
4440
#include <Parsers/ASTSelectQuery.h>
4541

@@ -77,12 +73,6 @@ void Client::processError(const String & query) const
7773
fmt::print(stderr, "Received exception from server (version {}):\n{}\n",
7874
server_version,
7975
getExceptionMessage(*server_exception, print_stack_trace, true));
80-
bool print_stack_trace = config().getBool("stacktrace", false);
81-
fmt::print(
82-
stderr,
83-
"Received exception from server (version {}):\n{}\n",
84-
server_version,
85-
getExceptionMessage(*server_exception, print_stack_trace, true));
8676
if (is_interactive)
8777
{
8878
fmt::print(stderr, "\n");
@@ -492,17 +482,23 @@ catch (...)
492482

493483
void Client::connect()
494484
{
485+
for (size_t attempted_address_index = 0; attempted_address_index < hosts_ports.size(); ++attempted_address_index)
486+
{
487+
DB::DNSResolver::instance().resolveHost(hosts_ports[attempted_address_index].host);
488+
}
495489
bool is_secure = config().getBool("secure", false);
496-
connection_parameters = ConnectionParameters(config());
490+
String tcp_port_config_key = is_secure ? "tcp_port_secure" : "tcp_port";
491+
UInt16 default_port = config().getInt("port",
492+
config().getInt(tcp_port_config_key,
493+
is_secure ? DBMS_DEFAULT_SECURE_PORT : DBMS_DEFAULT_PORT));
494+
connection_parameters = ConnectionParameters(config(), hosts_ports[0].host,
495+
hosts_ports[0].port.value_or(default_port));
497496

498497
String server_name;
499498
UInt64 server_version_major = 0;
500499
UInt64 server_version_minor = 0;
501500
UInt64 server_version_patch = 0;
502501

503-
String tcp_port_config_key = is_secure ? "tcp_port_secure" : "tcp_port";
504-
UInt16 default_port = config().getInt(tcp_port_config_key, is_secure ? DBMS_DEFAULT_SECURE_PORT : DBMS_DEFAULT_PORT);
505-
506502
for (size_t attempted_address_index = 0; attempted_address_index < hosts_ports.size(); ++attempted_address_index)
507503
{
508504
connection_parameters.host = hosts_ports[attempted_address_index].host;
@@ -527,6 +523,8 @@ void Client::connect()
527523

528524
connection->getServerVersion(
529525
connection_parameters.timeouts, server_name, server_version_major, server_version_minor, server_version_patch, server_revision);
526+
config().setString("host", connection_parameters.host);
527+
config().setInt("port", connection_parameters.port);
530528
break;
531529
}
532530
catch (const Exception & e)
@@ -549,14 +547,16 @@ void Client::connect()
549547
if (attempted_address_index == hosts_ports.size() - 1)
550548
throw;
551549

552-
std::cerr << "Connection attempt to database at "
553-
<< connection_parameters.host << ":" << connection_parameters.port
554-
<< " resulted in failure"
555-
<< std::endl
556-
<< getExceptionMessage(e, false)
557-
<< std::endl
558-
<< "Attempting connection to the next provided address"
559-
<< std::endl;
550+
if (is_interactive) {
551+
std::cerr << "Connection attempt to database at "
552+
<< connection_parameters.host << ":" << connection_parameters.port
553+
<< " resulted in failure"
554+
<< std::endl
555+
<< getExceptionMessage(e, false)
556+
<< std::endl
557+
<< "Attempting connection to the next provided address"
558+
<< std::endl;
559+
}
560560
}
561561
}
562562
}
@@ -1003,9 +1003,10 @@ void Client::addOptions(OptionsDescription & options_description)
10031003
options_description.main_description->add_options()
10041004
("config,c", po::value<std::string>(), "config-file path (another shorthand)")
10051005
("host,h", po::value<std::vector<HostPort>>()->multitoken()->default_value({{"localhost"}}, "localhost"),
1006-
"list of server hosts with optionally assigned port to connect. Every argument looks like '<host>[:<port>] for example"
1007-
"'localhost:port'. If port isn't assigned, connection is made by port from '--port' param")
1008-
("port", po::value<int>()->default_value(9000), "server port")
1006+
"list of server hosts with optionally assigned port to connect. List elements are separated by a space."
1007+
"Every list element looks like '<host>[:<port>]'. If port isn't assigned, connection is made by port from '--port' param"
1008+
"Example of usage: '-h host1:1 host2, host3:3'")
1009+
("port", po::value<int>()->default_value(9000), "server port, which is default port for every host from '--host' param")
10091010
("secure,s", "Use TLS connection")
10101011
("user,u", po::value<std::string>()->default_value("default"), "user")
10111012
/** If "--password [value]" is used but the value is omitted, the bad argument exception will be thrown.
@@ -1112,13 +1113,8 @@ void Client::processOptions(const OptionsDescription & options_description,
11121113

11131114
if (options.count("config"))
11141115
config().setString("config-file", options["config"].as<std::string>());
1115-
if (options.count("host") && !options["host"].defaulted())
1116-
{
1116+
if (options.count("host"))
11171117
hosts_ports = options["host"].as<std::vector<HostPort>>();
1118-
config().setString("host", hosts_ports[0].host);
1119-
if (hosts_ports[0].port.has_value())
1120-
config().setInt("port", hosts_ports[0].port.value());
1121-
}
11221118
if (options.count("interleave-queries-file"))
11231119
interleave_queries_files = options["interleave-queries-file"].as<std::vector<std::string>>();
11241120
if (options.count("port") && !options["port"].defaulted())

src/Client/ClientBase.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ class ClientBase : public Poco::Util::Application, public IHints<2, ClientBase>
249249
if (delimiter_pos != String::npos)
250250
{
251251
hostPort.host = host_with_port.substr(0, delimiter_pos);
252-
hostPort.port = std::stoi(host_with_port.substr(delimiter_pos + 1, host_with_port.length()));
252+
hostPort.port = boost::lexical_cast<uint>(host_with_port.substr(delimiter_pos + 1, host_with_port.length()));
253253
}
254254
else
255255
hostPort.host = host_with_port;

src/Client/ConnectionParameters.cpp

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,13 @@ namespace ErrorCodes
2323
extern const int BAD_ARGUMENTS;
2424
}
2525

26-
ConnectionParameters::ConnectionParameters(const Poco::Util::AbstractConfiguration & config)
26+
ConnectionParameters::ConnectionParameters(const Poco::Util::AbstractConfiguration & config,
27+
std::string connection_host,
28+
int connection_port) : host(connection_host), port(connection_port)
2729
{
2830
bool is_secure = config.getBool("secure", false);
2931
security = is_secure ? Protocol::Secure::Enable : Protocol::Secure::Disable;
3032

31-
host = config.getString("host", "localhost");
32-
port = config.getInt(
33-
"port", config.getInt(is_secure ? "tcp_port_secure" : "tcp_port", is_secure ? DBMS_DEFAULT_SECURE_PORT : DBMS_DEFAULT_PORT));
34-
3533
default_database = config.getString("database", "");
3634

3735
/// changed the default value to "default" to fix the issue when the user in the prompt is blank
@@ -61,12 +59,21 @@ ConnectionParameters::ConnectionParameters(const Poco::Util::AbstractConfigurati
6159

6260
/// By default compression is disabled if address looks like localhost.
6361
compression = config.getBool("compression", !isLocalAddress(DNSResolver::instance().resolveHost(host)))
64-
? Protocol::Compression::Enable : Protocol::Compression::Disable;
62+
? Protocol::Compression::Enable : Protocol::Compression::Disable;
6563

6664
timeouts = ConnectionTimeouts(
67-
Poco::Timespan(config.getInt("connect_timeout", DBMS_DEFAULT_CONNECT_TIMEOUT_SEC), 0),
68-
Poco::Timespan(config.getInt("send_timeout", DBMS_DEFAULT_SEND_TIMEOUT_SEC), 0),
69-
Poco::Timespan(config.getInt("receive_timeout", DBMS_DEFAULT_RECEIVE_TIMEOUT_SEC), 0),
70-
Poco::Timespan(config.getInt("tcp_keep_alive_timeout", 0), 0));
65+
Poco::Timespan(config.getInt("connect_timeout", DBMS_DEFAULT_CONNECT_TIMEOUT_SEC), 0),
66+
Poco::Timespan(config.getInt("send_timeout", DBMS_DEFAULT_SEND_TIMEOUT_SEC), 0),
67+
Poco::Timespan(config.getInt("receive_timeout", DBMS_DEFAULT_RECEIVE_TIMEOUT_SEC), 0),
68+
Poco::Timespan(config.getInt("tcp_keep_alive_timeout", 0), 0));
69+
}
70+
71+
ConnectionParameters::ConnectionParameters(const Poco::Util::AbstractConfiguration & config)
72+
{
73+
bool is_secure = config.getBool("secure", false);
74+
std::string connection_host = config.getString("host", "localhost");
75+
int connection_port = config.getInt("port",
76+
config.getInt(is_secure ? "tcp_port_secure" : "tcp_port", is_secure ? DBMS_DEFAULT_SECURE_PORT : DBMS_DEFAULT_PORT));
77+
ConnectionParameters(config, connection_host, connection_port);
7178
}
7279
}

src/Client/ConnectionParameters.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ struct ConnectionParameters
2424

2525
ConnectionParameters() {}
2626
ConnectionParameters(const Poco::Util::AbstractConfiguration & config);
27+
ConnectionParameters(const Poco::Util::AbstractConfiguration & config, std::string host, int port);
2728
};
2829

2930
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
1
2+
1
3+
1
4+
1
5+
1
6+
1
7+
1
8+
1
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
#!/usr/bin/env bash
2+
3+
CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
4+
# shellcheck source=../shell_config.sh
5+
. "$CURDIR"/../shell_config.sh
6+
7+
# default values test
8+
${CLICKHOUSE_CLIENT} --query "SELECT 1"
9+
10+
# backward compatibility test
11+
${CLICKHOUSE_CLIENT} --host "${CLICKHOUSE_HOST}" --port "${CLICKHOUSE_PORT_TCP}" --query "SELECT 1";
12+
13+
not_resolvable_host="notlocalhost"
14+
exception_msg="Cannot resolve host (${not_resolvable_host}), error 0: ${not_resolvable_host}.
15+
Code: 198. DB::Exception: Not found address of host: ${not_resolvable_host}. (DNS_ERROR)
16+
"
17+
error="$(${CLICKHOUSE_CLIENT} --host "${CLICKHOUSE_HOST}" "${not_resolvable_host}" --query "SELECT 1" 2>&1 > /dev/null)";
18+
[ "${error}" == "${exception_msg}" ]; echo "$?"
19+
20+
not_number_port="abc"
21+
exception_msg="Bad arguments: the argument ('${CLICKHOUSE_HOST}:${not_number_port}') for option '--host' is invalid."
22+
error="$(${CLICKHOUSE_CLIENT} --host "${CLICKHOUSE_HOST}:${not_number_port}" --query "SELECT 1" 2>&1 > /dev/null)";
23+
[ "${error}" == "${exception_msg}" ]; echo "$?"
24+
25+
not_alive_host="10.100.0.0"
26+
${CLICKHOUSE_CLIENT} --host "${not_alive_host}" "${CLICKHOUSE_HOST}" --query "SELECT 1";
27+
28+
not_alive_port="1"
29+
exception_msg="Code: 210. DB::NetException: Connection refused (${CLICKHOUSE_HOST}:${not_alive_port}). (NETWORK_ERROR)
30+
"
31+
error="$(${CLICKHOUSE_CLIENT} --host "${CLICKHOUSE_HOST}" --port "${not_alive_port}" --query "SELECT 1" 2>&1 > /dev/null)"
32+
[ "${error}" == "${exception_msg}" ]; echo "$?"
33+
${CLICKHOUSE_CLIENT} --host "${CLICKHOUSE_HOST}:${not_alive_port}" "${CLICKHOUSE_HOST}" --query "SELECT 1";
34+
${CLICKHOUSE_CLIENT} --host "${CLICKHOUSE_HOST}:${CLICKHOUSE_PORT_TCP}" --port "${not_alive_port}" --query "SELECT 1";
35+

0 commit comments

Comments
 (0)