thevar1able/ClickHouse
1
0
Fork 0
mirror of https://github.com/ClickHouse/ClickHouse.git synced 2024-11-10 09:32:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix false-positive ciclyc dependency with XML dict

Browse Source
This commit is contained in:
Alexander Tokmakov 2021-12-06 16:35:34 +03:00
parent 90505a7341
commit 7797a72907
8 changed files with 51 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/Databases/DDLDependencyVisitor.cpp
View File

@ -11,7 +11,7 @@
namespace DB
{
TableNamesSet getDependenciesSetFromCreateQuery(ContextPtr global_context, const ASTPtr & ast)
TableNamesSet getDependenciesSetFromCreateQuery(ContextPtr global_context, const QualifiedTableName & table, const ASTPtr & ast)
{
assert(global_context == global_context->getGlobalContext());
TableLoadingDependenciesVisitor::Data data;
@ -20,6 +20,7 @@ TableNamesSet getDependenciesSetFromCreateQuery(ContextPtr global_context, const
data.global_context = global_context;
TableLoadingDependenciesVisitor visitor{data};
visitor.visit(ast);
data.dependencies.erase(table);
return data.dependencies;
}
@ -132,7 +133,10 @@ void DDLDependencyVisitor::extractTableNameFromArgument(const ASTFunction & func
}
if (qualified_name.database.empty())
{
/// It can be table/dictionary from default database or XML dictionary, but we cannot distinguish it here.
qualified_name.database = data.default_database;
}
data.dependencies.emplace(std::move(qualified_name));
}

2
src/Databases/DDLDependencyVisitor.h
View File

@ -12,7 +12,7 @@ class ASTStorage;
using TableNamesSet = std::unordered_set<QualifiedTableName>;
TableNamesSet getDependenciesSetFromCreateQuery(ContextPtr global_context, const ASTPtr & ast);
TableNamesSet getDependenciesSetFromCreateQuery(ContextPtr global_context, const QualifiedTableName & table, const ASTPtr & ast);
/// Visits ASTCreateQuery and extracts names of table (or dictionary) dependencies
/// from column default expressions (joinGet, dictGet, etc)

2
src/Databases/DatabaseMemory.cpp
View File

@ -121,7 +121,7 @@ void DatabaseMemory::alterTable(ContextPtr local_context, const StorageID & tabl
throw Exception(ErrorCodes::UNKNOWN_TABLE, "Cannot alter: There is no metadata of table {}", table_id.getNameForLogs());
applyMetadataChangesToCreateQuery(it->second, metadata);
TableNamesSet new_dependencies = getDependenciesSetFromCreateQuery(local_context->getGlobalContext(), it->second);
TableNamesSet new_dependencies = getDependenciesSetFromCreateQuery(local_context->getGlobalContext(), table_id.getQualifiedName(), it->second);
DatabaseCatalog::instance().updateLoadingDependencies(table_id, std::move(new_dependencies));
}

4
src/Databases/DatabaseOrdinary.cpp
View File

@ -181,8 +181,8 @@ void DatabaseOrdinary::loadTablesMetadata(ContextPtr local_context, ParsedTables
return;
}
TableNamesSet loading_dependencies = getDependenciesSetFromCreateQuery(getContext(), ast);
QualifiedTableName qualified_name{database_name, create_query->getTable()};
TableNamesSet loading_dependencies = getDependenciesSetFromCreateQuery(getContext(), qualified_name, ast);
std::lock_guard lock{metadata.mutex};
metadata.parsed_tables[qualified_name] = ParsedTableMetadata{full_path.string(), ast};
@ -297,7 +297,7 @@ void DatabaseOrdinary::alterTable(ContextPtr local_context, const StorageID & ta
out.close();
}
TableNamesSet new_dependencies = getDependenciesSetFromCreateQuery(local_context->getGlobalContext(), ast);
TableNamesSet new_dependencies = getDependenciesSetFromCreateQuery(local_context->getGlobalContext(), table_id.getQualifiedName(), ast);
DatabaseCatalog::instance().updateLoadingDependencies(table_id, std::move(new_dependencies));
commitAlterTable(table_id, table_metadata_tmp_path, table_metadata_path, statement, local_context);

8
src/Databases/TablesLoader.cpp
View File

@ -133,10 +133,14 @@ void TablesLoader::removeUnresolvableDependencies(bool remove_loaded)
/// Table exists and it's already loaded
if (DatabaseCatalog::instance().isTableExist(StorageID(dependency_name.database, dependency_name.table), global_context))
return remove_loaded;
/// It's XML dictionary. It was loaded before tables and DDL dictionaries.
/// It's XML dictionary.
if (dependency_name.database == metadata.default_database &&
global_context->getExternalDictionariesLoader().has(dependency_name.table))
return remove_loaded;
{
LOG_WARNING(log, "Tables {} depend on XML dictionary {}, but XML dictionaries are loaded independently."
"Consider converting it to DDL dictionary.", fmt::join(info.dependent_database_objects, ", "), dependency_name);
return true;
}
/// Some tables depends on table "dependency_name", but there is no such table in DatabaseCatalog and we don't have its metadata.
/// We will ignore it and try to load dependent tables without "dependency_name"

5
src/Interpreters/InterpreterCreateQuery.cpp
View File

@ -981,9 +981,10 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create)
return {};
/// If table has dependencies - add them to the graph
TableNamesSet loading_dependencies = getDependenciesSetFromCreateQuery(getContext()->getGlobalContext(), query_ptr);
QualifiedTableName qualified_name{database_name, create.getTable()};
TableNamesSet loading_dependencies = getDependenciesSetFromCreateQuery(getContext()->getGlobalContext(), qualified_name, query_ptr);
if (!loading_dependencies.empty())
DatabaseCatalog::instance().addLoadingDependencies(QualifiedTableName{database_name, create.getTable()}, std::move(loading_dependencies));
DatabaseCatalog::instance().addLoadingDependencies(std::move(qualified_name), std::move(loading_dependencies));
return fillTableIfNeeded(create);
}

25
tests/integration/test_dictionaries_dependency_xml/configs/dictionaries/node.xml Normal file
View File

@ -0,0 +1,25 @@
<clickhouse>
<dictionary>
<name>node</name>
<source>
<clickhouse>
<host>localhost</host>
<port>9000</port>
<user>default</user>
<password></password>
<db>system</db>
<query>select dummy, toString(dummy) from system.one</query>
</clickhouse>
</source>
<lifetime>0</lifetime>
<layout><flat /></layout>
<structure>
<id><name>key</name></id>
<attribute>
<name>name</name>
<type>String</type>
<null_value></null_value>
</attribute>
</structure>
</dictionary>
</clickhouse>

9
tests/integration/test_dictionaries_dependency_xml/test.py
View File

@ -3,7 +3,7 @@ from helpers.cluster import ClickHouseCluster
from helpers.test_tools import assert_eq_with_retry
DICTIONARY_FILES = ['configs/dictionaries/dep_x.xml', 'configs/dictionaries/dep_y.xml',
'configs/dictionaries/dep_z.xml']
'configs/dictionaries/dep_z.xml', 'configs/dictionaries/node.xml']
cluster = ClickHouseCluster(__file__)
instance = cluster.add_instance('instance', dictionaries=DICTIONARY_FILES, stay_alive=True)
@ -117,3 +117,10 @@ def test_dependent_tables(started_cluster):
query("drop table system.join")
query("drop database a")
query("drop database lazy")
def test_xml_dict_same_name(started_cluster):
instance.query("create table default.node ( key UInt64, name String ) Engine=Dictionary(node);")
instance.restart_clickhouse()
assert "node" in instance.query("show tables from default")
instance.query("drop table default.node")