Merge pull request ClickHouse#57134 from vitlibar/check-dictionary-so…

…urce-type-on-creation Check dictionary source type on creation
azat · Nov 23, 2023 · 2eca762 · 2eca762
2 parents 6545775 + e0c9661
commit 2eca762
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 3 deletions.
diff --git a/src/Dictionaries/DictionarySourceFactory.cpp b/src/Dictionaries/DictionarySourceFactory.cpp
@@ -108,6 +108,18 @@ DictionarySourcePtr DictionarySourceFactory::create(
         source_type);
 }
 
+void DictionarySourceFactory::checkSourceAvailable(const std::string & source_type, const std::string & dictionary_name, const ContextPtr & /* context */) const
+{
+    const auto found = registered_sources.find(source_type);
+    if (found == registered_sources.end())
+    {
+        throw Exception(ErrorCodes::UNKNOWN_ELEMENT_IN_CONFIG,
+            "{}: unknown dictionary source type: {}",
+            dictionary_name,
+            source_type);
+    }
+}
+
 DictionarySourceFactory & DictionarySourceFactory::instance()
 {
     static DictionarySourceFactory instance;

diff --git a/src/Dictionaries/DictionarySourceFactory.h b/src/Dictionaries/DictionarySourceFactory.h
@@ -52,6 +52,9 @@ class DictionarySourceFactory : private boost::noncopyable
         const std::string & default_database,
         bool check_config) const;
 
+    /// Checks that a specified source exists and available for the current user.
+    void checkSourceAvailable(const std::string & source_type, const std::string & dictionary_name, const ContextPtr & context) const;
+
 private:
     using SourceRegistry = std::unordered_map<std::string, Creator>;
     SourceRegistry registered_sources;

diff --git a/src/Dictionaries/getDictionaryConfigurationFromAST.cpp b/src/Dictionaries/getDictionaryConfigurationFromAST.cpp
@@ -16,6 +16,7 @@
 #include <Parsers/ASTFunctionWithKeyValueArguments.h>
 #include <Parsers/ASTDictionaryAttributeDeclaration.h>
 #include <Dictionaries/DictionaryFactory.h>
+#include <Dictionaries/DictionarySourceFactory.h>
 #include <Functions/FunctionFactory.h>
 #include <Common/isLocalAddress.h>
 #include <Interpreters/Context.h>
@@ -518,8 +519,11 @@ void buildSourceConfiguration(
     AutoPtr<Element> root,
     const ASTFunctionWithKeyValueArguments * source,
     const ASTDictionarySettings * settings,
+    const String & dictionary_name,
     ContextPtr context)
 {
+    DictionarySourceFactory::instance().checkSourceAvailable(source->name, dictionary_name, context);
+
     AutoPtr<Element> outer_element(doc->createElement("source"));
     root->appendChild(outer_element);
     AutoPtr<Element> source_element(doc->createElement(source->name));
@@ -591,6 +595,10 @@ getDictionaryConfigurationFromAST(const ASTCreateQuery & query, ContextPtr conte
     checkAST(query);
     checkLifetime(query);
 
+    String dictionary_name = query.getTable();
+    String db_name = !database_.empty() ? database_ : query.getDatabase();
+    String full_dictionary_name = (!db_name.empty() ? (db_name + ".") : "") + dictionary_name;
+
     AutoPtr<Poco::XML::Document> xml_document(new Poco::XML::Document());
     AutoPtr<Poco::XML::Element> document_root(xml_document->createElement("dictionaries"));
     xml_document->appendChild(document_root);
@@ -600,12 +608,12 @@ getDictionaryConfigurationFromAST(const ASTCreateQuery & query, ContextPtr conte
 
     AutoPtr<Poco::XML::Element> name_element(xml_document->createElement("name"));
     current_dictionary->appendChild(name_element);
-    AutoPtr<Text> name(xml_document->createTextNode(query.getTable()));
+    AutoPtr<Text> name(xml_document->createTextNode(dictionary_name));
     name_element->appendChild(name);
 
     AutoPtr<Poco::XML::Element> database_element(xml_document->createElement("database"));
     current_dictionary->appendChild(database_element);
-    AutoPtr<Text> database(xml_document->createTextNode(!database_.empty() ? database_ : query.getDatabase()));
+    AutoPtr<Text> database(xml_document->createTextNode(db_name));
     database_element->appendChild(database);
 
     if (query.uuid != UUIDHelpers::Nil)
@@ -641,7 +649,7 @@ getDictionaryConfigurationFromAST(const ASTCreateQuery & query, ContextPtr conte
     buildPrimaryKeyConfiguration(xml_document, structure_element, complex, pk_attrs, query.dictionary_attributes_list);
 
     buildLayoutConfiguration(xml_document, current_dictionary, query.dictionary->dict_settings, dictionary_layout);
-    buildSourceConfiguration(xml_document, current_dictionary, query.dictionary->source, query.dictionary->dict_settings, context);
+    buildSourceConfiguration(xml_document, current_dictionary, query.dictionary->source, query.dictionary->dict_settings, full_dictionary_name, context);
     buildLifetimeConfiguration(xml_document, current_dictionary, query.dictionary->lifetime);
 
     if (query.dictionary->range)

diff --git a/tests/queries/0_stateless/02918_wrong_dictionary_source.reference b/tests/queries/0_stateless/02918_wrong_dictionary_source.reference
@@ -0,0 +1 @@
+0
diff --git a/tests/queries/0_stateless/02918_wrong_dictionary_source.sql b/tests/queries/0_stateless/02918_wrong_dictionary_source.sql
@@ -0,0 +1,11 @@
+DROP DICTIONARY IF EXISTS id_value_dictionary;
+DROP TABLE IF EXISTS source_table;
+
+CREATE TABLE source_table(id UInt64, value String) ENGINE = MergeTree ORDER BY tuple();
+
+-- There is no "CLICKHOUSEX" dictionary source, so the next query must fail even if `dictionaries_lazy_load` is enabled.
+CREATE DICTIONARY id_value_dictionary(id UInt64, value String) PRIMARY KEY id SOURCE(CLICKHOUSEX(TABLE 'source_table')) LIFETIME(MIN 0 MAX 1000) LAYOUT(FLAT()); -- { serverError UNKNOWN_ELEMENT_IN_CONFIG }
+
+SELECT count() FROM system.dictionaries WHERE name=='id_value_dictionary' AND database==currentDatabase();
+
+DROP TABLE source_table;