From 7edd80c9b7880ac07531a6874f6d2e78b7a5be67 Mon Sep 17 00:00:00 2001
From: alesapin <alesapin@gmail.com>
Date: Thu, 24 Oct 2019 12:25:28 +0300
Subject: [PATCH] Add test for existing dictionary

---
 dbms/src/Databases/DatabasesCommon.cpp        | 12 +++++++---
 .../integration/test_dictionaries_ddl/test.py | 23 ++++++++++++++++++-
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/dbms/src/Databases/DatabasesCommon.cpp b/dbms/src/Databases/DatabasesCommon.cpp
index 1f1a1a7a004..2feda6fc2b3 100644
--- a/dbms/src/Databases/DatabasesCommon.cpp
+++ b/dbms/src/Databases/DatabasesCommon.cpp
@@ -186,14 +186,20 @@ void DatabaseWithOwnTablesBase::attachTable(const String & table_name, const Sto
 
 void DatabaseWithOwnTablesBase::attachDictionary(const String & dictionary_name, const Context & context, bool load)
 {
+    const auto & external_loader = context.getExternalDictionariesLoader();
+
+    String full_name = getDatabaseName() + "." + dictionary_name;
     {
         std::lock_guard lock(mutex);
-        if (!dictionaries.emplace(dictionary_name).second)
-            throw Exception("Dictionary " + name + "." + dictionary_name + " already exists.", ErrorCodes::DICTIONARY_ALREADY_EXISTS);
+        auto status = external_loader.getCurrentStatus(full_name);
+        if (status != ExternalLoader::Status::NOT_EXIST || !dictionaries.emplace(dictionary_name).second)
+            throw Exception(
+                      "Dictionary " + full_name + " already exists.",
+                      ErrorCodes::DICTIONARY_ALREADY_EXISTS);
     }
 
     if (load)
-        context.getExternalDictionariesLoader().reload(getDatabaseName() + "." + dictionary_name, true);
+        external_loader.reload(full_name, true);
 }
 
 void DatabaseWithOwnTablesBase::shutdown()
diff --git a/dbms/tests/integration/test_dictionaries_ddl/test.py b/dbms/tests/integration/test_dictionaries_ddl/test.py
index 6687bed215c..a949bee136a 100644
--- a/dbms/tests/integration/test_dictionaries_ddl/test.py
+++ b/dbms/tests/integration/test_dictionaries_ddl/test.py
@@ -12,6 +12,7 @@ SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__))
 cluster = ClickHouseCluster(__file__, base_configs_dir=os.path.join(SCRIPT_DIR, 'configs'))
 node1 = cluster.add_instance('node1', with_mysql=True, main_configs=['configs/dictionaries/simple_dictionary.xml'])
 node2 = cluster.add_instance('node2', with_mysql=True, main_configs=['configs/dictionaries/simple_dictionary.xml', 'configs/dictionaries/lazy_load.xml'])
+node3 = cluster.add_instance('node3', main_configs=['configs/dictionaries/dictionary_with_conflict_name.xml'])
 
 
 def create_mysql_conn(user, password, hostname, port):
@@ -33,7 +34,7 @@ def execute_mysql_query(connection, query):
 def started_cluster():
     try:
         cluster.start()
-        for clickhouse in [node1, node2]:
+        for clickhouse in [node1, node2, node3]:
             clickhouse.query("CREATE DATABASE test", user="admin")
             clickhouse.query("CREATE TABLE test.xml_dictionary_table (id UInt64, SomeValue1 UInt8, SomeValue2 String) ENGINE = MergeTree() ORDER BY id", user="admin")
             clickhouse.query("INSERT INTO test.xml_dictionary_table SELECT number, number % 23, hex(number) from numbers(1000)", user="admin")
@@ -161,3 +162,23 @@ def test_restricted_database(started_cluster):
         SOURCE(CLICKHOUSE(HOST 'localhost' PORT 9000 USER 'default' TABLE 'table_in_restricted_db' DB 'restricted_db'))
         LIFETIME(MIN 1 MAX 10)
         """)
+
+
+def test_conflicting_name(started_cluster):
+    assert node3.query("select dictGetUInt8('test.conflicting_dictionary', 'SomeValue1', toUInt64(17))") == '17\n'
+
+    with pytest.raises(QueryRuntimeException):
+        node3.query("""
+        CREATE DICTIONARY test.conflicting_dictionary(
+            id UInt64,
+            SomeValue1 UInt8,
+            SomeValue2 String
+        )
+        PRIMARY KEY id
+        LAYOUT(FLAT())
+        SOURCE(CLICKHOUSE(HOST 'localhost' PORT 9000 USER 'default' TABLE 'xml_dictionary_table' DB 'test'))
+        LIFETIME(MIN 1 MAX 10)
+        """)
+
+    # old version still works
+    node3.query("select dictGetUInt8('test.conflicting_dictionary', 'SomeValue1', toUInt64(17))") == '17\n'