Skip to content

Commit

Permalink
Merge pull request ClickHouse#57111 from ClickHouse/backport/23.8/56484
Browse files Browse the repository at this point in the history
Backport ClickHouse#56484 to 23.8: Fix ON CLUSTER queries without database on initial node
  • Loading branch information
evillique authored Nov 24, 2023
2 parents 850befe + a426baf commit 5e012a0
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 5 deletions.
15 changes: 12 additions & 3 deletions src/Interpreters/InterpreterAlterQuery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ namespace ErrorCodes
extern const int TABLE_IS_READ_ONLY;
extern const int BAD_ARGUMENTS;
extern const int UNKNOWN_TABLE;
extern const int UNKNOWN_DATABASE;
}


Expand Down Expand Up @@ -74,9 +75,14 @@ BlockIO InterpreterAlterQuery::executeToTable(const ASTAlterQuery & alter)
if (!UserDefinedSQLFunctionFactory::instance().empty())
UserDefinedSQLFunctionVisitor::visit(query_ptr);

auto table_id = getContext()->resolveStorageID(alter, Context::ResolveOrdinary);
query_ptr->as<ASTAlterQuery &>().setDatabase(table_id.database_name);
StoragePtr table = DatabaseCatalog::instance().tryGetTable(table_id, getContext());
auto table_id = getContext()->tryResolveStorageID(alter, Context::ResolveOrdinary);
StoragePtr table;

if (table_id)
{
query_ptr->as<ASTAlterQuery &>().setDatabase(table_id.database_name);
table = DatabaseCatalog::instance().tryGetTable(table_id, getContext());
}

if (!alter.cluster.empty() && !maybeRemoveOnCluster(query_ptr, getContext()))
{
Expand All @@ -90,6 +96,9 @@ BlockIO InterpreterAlterQuery::executeToTable(const ASTAlterQuery & alter)

getContext()->checkAccess(getRequiredAccess());

if (!table_id)
throw Exception(ErrorCodes::UNKNOWN_DATABASE, "Database {} does not exist", backQuoteIfNeed(alter.getDatabase()));

DatabasePtr database = DatabaseCatalog::instance().getDatabase(table_id.database_name);
if (database->shouldReplicateQuery(getContext(), query_ptr))
{
Expand Down
7 changes: 5 additions & 2 deletions src/Interpreters/InterpreterCreateQuery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1215,9 +1215,9 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create)
DatabasePtr database;
bool need_add_to_database = !create.temporary;
if (need_add_to_database)
database = DatabaseCatalog::instance().getDatabase(database_name);
database = DatabaseCatalog::instance().tryGetDatabase(database_name);

if (need_add_to_database && database->shouldReplicateQuery(getContext(), query_ptr))
if (need_add_to_database && database && database->shouldReplicateQuery(getContext(), query_ptr))
{
chassert(!ddl_guard);
auto guard = DatabaseCatalog::instance().getDDLGuard(create.getDatabase(), create.getTable());
Expand All @@ -1232,6 +1232,9 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create)
return executeQueryOnCluster(create);
}

if (need_add_to_database && !database)
throw Exception(ErrorCodes::UNKNOWN_DATABASE, "Database {} does not exist", backQuoteIfNeed(database_name));

if (create.replace_table)
{
chassert(!ddl_guard);
Expand Down
Empty file.
12 changes: 12 additions & 0 deletions tests/integration/test_external_cluster/configs/clusters.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<clickhouse>
<remote_servers>
<external>
<shard>
<replica>
<host>data_node</host>
<port>9000</port>
</replica>
</shard>
</external>
</remote_servers>
</clickhouse>
69 changes: 69 additions & 0 deletions tests/integration/test_external_cluster/test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import re
import pytest
from helpers.cluster import ClickHouseCluster
from helpers.test_tools import assert_eq_with_retry

cluster = ClickHouseCluster(__file__)

control_node = cluster.add_instance(
"control_node",
main_configs=["configs/clusters.xml"],
with_zookeeper=True,
)

data_node = cluster.add_instance(
"data_node",
main_configs=["configs/clusters.xml"],
with_zookeeper=True,
macros={"shard": 1, "replica": 1},
)

uuid_regex = re.compile("[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}")


def assert_create_query(node, database_name, table_name, expected):
replace_uuid = lambda x: re.sub(uuid_regex, "uuid", x)
query = "SELECT create_table_query FROM system.tables WHERE database='{}' AND table='{}'".format(
database_name, table_name
)
assert_eq_with_retry(node, query, expected, get_result=replace_uuid)


@pytest.fixture(scope="module")
def started_cluster():
try:
cluster.start()
yield cluster
finally:
cluster.shutdown()


def test_ddl(started_cluster):
control_node.query("CREATE DATABASE test_db ON CLUSTER 'external' ENGINE=Atomic")
control_node.query(
"CREATE TABLE test_db.test_table ON CLUSTER 'external' (id Int64) Engine=MergeTree ORDER BY id"
)
control_node.query(
"ALTER TABLE test_db.test_table ON CLUSTER 'external' add column data String"
)

expected = "CREATE TABLE test_db.test_table (`id` Int64, `data` String) ENGINE = MergeTree ORDER BY id SETTINGS index_granularity = 8192"
assert_create_query(data_node, "test_db", "test_table", expected)

control_node.query("DROP TABLE test_db.test_table ON CLUSTER 'external'")
control_node.query("DROP DATABASE test_db ON CLUSTER 'external'")

expected = ""
assert_create_query(data_node, "test_db", "test_table", expected)


def test_ddl_replicated(started_cluster):
control_node.query(
"CREATE DATABASE test_db ON CLUSTER 'external' ENGINE=Replicated('/replicated')",
settings={"allow_experimental_database_replicated": 1},
)
# Exception is expected
assert "It's not initial query" in control_node.query_and_get_error(
"CREATE TABLE test_db.test_table ON CLUSTER 'external' (id Int64) Engine=MergeTree ORDER BY id"
)
control_node.query("DROP DATABASE test_db ON CLUSTER 'external'")

0 comments on commit 5e012a0

Please sign in to comment.