thevar1able/ClickHouse
1
0
Fork 0
mirror of https://github.com/ClickHouse/ClickHouse.git synced 2024-09-20 00:30:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix ambiguity when extracting auxiliary zk name

Browse Source
This commit is contained in:
Alexander Tokmakov 2021-10-28 19:19:41 +03:00
parent db18cb194c
commit c1310841d9
3 changed files with 39 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/Interpreters/Context.cpp
View File

@ -1971,6 +1971,9 @@ zkutil::ZooKeeperPtr Context::getAuxiliaryZooKeeper(const String & name) const
auto zookeeper = shared->auxiliary_zookeepers.find(name);
if (zookeeper == shared->auxiliary_zookeepers.end())
{
if (name.find(':') != std::string::npos || name.find('/') != std::string::npos)
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Invalid auxiliary ZooKeeper name {}: ':' and '/' are not allowed", name);
const auto & config = shared->auxiliary_zookeepers_config ? *shared->auxiliary_zookeepers_config : getConfigRef();
if (!config.has("auxiliary_zookeepers." + name))
throw Exception(

41
src/Storages/StorageReplicatedMergeTree.cpp
View File

@ -192,43 +192,54 @@ zkutil::ZooKeeperPtr StorageReplicatedMergeTree::getZooKeeper() const
return res;
}
static std::string normalizeZooKeeperPath(std::string zookeeper_path)
static std::string normalizeZooKeeperPath(std::string zookeeper_path, bool check_starts_with_slash, Poco::Logger * log = nullptr)
{
if (!zookeeper_path.empty() && zookeeper_path.back() == '/')
zookeeper_path.resize(zookeeper_path.size() - 1);
/// If zookeeper chroot prefix is used, path should start with '/', because chroot concatenates without it.
if (!zookeeper_path.empty() && zookeeper_path.front() != '/')
{
/// Do not allow this for new tables, print warning for tables created in old versions
if (check_starts_with_slash)
throw Exception(ErrorCodes::BAD_ARGUMENTS, "ZooKeeper path must starts with '/', got '{}'", zookeeper_path);
if (log)
LOG_WARNING(log, "ZooKeeper path ('{}') does not start with '/'. It will not be supported in future releases");
zookeeper_path = "/" + zookeeper_path;
}
return zookeeper_path;
}
static String extractZooKeeperName(const String & path)
{
static constexpr auto default_zookeeper_name = "default";
if (path.empty())
throw Exception("ZooKeeper path should not be empty", ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT);
auto pos = path.find(':');
if (pos != String::npos)
throw Exception("ZooKeeper path should not be empty", ErrorCodes::BAD_ARGUMENTS);
if (path[0] == '/')
return default_zookeeper_name;
auto pos = path.find(":/");
if (pos != String::npos && pos < path.find('/'))
{
auto zookeeper_name = path.substr(0, pos);
if (zookeeper_name.empty())
throw Exception("Zookeeper path should start with '/' or '<auxiliary_zookeeper_name>:/'", ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT);
throw Exception("Zookeeper path should start with '/' or '<auxiliary_zookeeper_name>:/'", ErrorCodes::BAD_ARGUMENTS);
return zookeeper_name;
}
static constexpr auto default_zookeeper_name = "default";
return default_zookeeper_name;
}
static String extractZooKeeperPath(const String & path)
static String extractZooKeeperPath(const String & path, bool check_starts_with_slash, Poco::Logger * log = nullptr)
{
if (path.empty())
throw Exception("ZooKeeper path should not be empty", ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT);
auto pos = path.find(':');
if (pos != String::npos)
throw Exception("ZooKeeper path should not be empty", ErrorCodes::BAD_ARGUMENTS);
if (path[0] == '/')
return path;
auto pos = path.find(":/");
if (pos != String::npos && pos < path.find('/'))
{
return normalizeZooKeeperPath(path.substr(pos + 1, String::npos));
return normalizeZooKeeperPath(path.substr(pos + 1, String::npos), check_starts_with_slash, log);
}
return normalizeZooKeeperPath(path);
return normalizeZooKeeperPath(path, check_starts_with_slash, log);
}
static MergeTreePartInfo makeDummyDropRangeForMovePartitionOrAttachPartitionFrom(const String & partition_id)
@ -275,7 +286,7 @@ StorageReplicatedMergeTree::StorageReplicatedMergeTree(
attach,
[this] (const std::string & name) { enqueuePartForCheck(name); })
, zookeeper_name(extractZooKeeperName(zookeeper_path_))
, zookeeper_path(extractZooKeeperPath(zookeeper_path_))
, zookeeper_path(extractZooKeeperPath(zookeeper_path_, /* check_starts_with_slash */ !attach, log))
, replica_name(replica_name_)
, replica_path(fs::path(zookeeper_path) / "replicas" / replica_name_)
, reader(*this)
@ -5425,7 +5436,7 @@ void StorageReplicatedMergeTree::fetchPartition(
info.table_id.uuid = UUIDHelpers::Nil;
auto expand_from = query_context->getMacros()->expand(from_, info);
String auxiliary_zookeeper_name = extractZooKeeperName(expand_from);
String from = extractZooKeeperPath(expand_from);
String from = extractZooKeeperPath(expand_from, /* check_starts_with_slash */ true);
if (from.empty())
throw Exception("ZooKeeper path should not be empty", ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT);
@ -6490,7 +6501,7 @@ void StorageReplicatedMergeTree::movePartitionToShard(
if (!move_part)
throw Exception("MOVE PARTITION TO SHARD is not supported, use MOVE PART instead", ErrorCodes::NOT_IMPLEMENTED);
if (normalizeZooKeeperPath(zookeeper_path) == normalizeZooKeeperPath(to))
if (normalizeZooKeeperPath(zookeeper_path, /* check_starts_with_slash */ true) == normalizeZooKeeperPath(to, /* check_starts_with_slash */ true))
throw Exception("Source and destination are the same", ErrorCodes::BAD_ARGUMENTS);
auto zookeeper = getZooKeeper();

10
tests/integration/test_replicated_merge_tree_with_auxiliary_zookeepers/test.py
View File

@ -101,3 +101,13 @@ def test_drop_replicated_merge_tree_with_auxiliary_zookeeper(started_cluster):
assert zk.exists('/clickhouse/tables/test/test_auxiliary_zookeeper')
drop_table([node1, node2], "test_auxiliary_zookeeper")
assert zk.exists('/clickhouse/tables/test/test_auxiliary_zookeeper') is None
def test_path_ambiguity(started_cluster):
drop_table([node1, node2], "test_path_ambiguity1")
drop_table([node1, node2], "test_path_ambiguity2")
node1.query("create table test_path_ambiguity1 (n int) engine=ReplicatedMergeTree('/test:bad:/path', '1') order by n")
assert "Invalid auxiliary ZooKeeper name" in node1.query_and_get_error("create table test_path_ambiguity2 (n int) engine=ReplicatedMergeTree('test:bad:/path', '1') order by n")
assert "ZooKeeper path must starts with '/'" in node1.query_and_get_error("create table test_path_ambiguity2 (n int) engine=ReplicatedMergeTree('test/bad:/path', '1') order by n")
node1.query("create table test_path_ambiguity2 (n int) engine=ReplicatedMergeTree('zookeeper2:/bad:/path', '1') order by n")
drop_table([node1, node2], "test_path_ambiguity1")
drop_table([node1, node2], "test_path_ambiguity2")