code review fixes & added exception in case lifetime parameter presence in config

src/Dictionaries/DirectDictionary.cpp

@@ -20,18 +20,15 @@ DirectDictionary::DirectDictionary(
     const std::string & name_,
     const DictionaryStructure & dict_struct_,
     DictionarySourcePtr source_ptr_,
-    const DictionaryLifetime dict_lifetime_,
     BlockPtr saved_block_)
     : database(database_)
     , name(name_)
     , full_name{database_.empty() ? name_ : (database_ + "." + name_)}
     , dict_struct(dict_struct_)
     , source_ptr{std::move(source_ptr_)}
-    , dict_lifetime(dict_lifetime_)
     , saved_block{std::move(saved_block_)}
 {
     createAttributes();
-    calculateBytesAllocated();
 }
 
 
@@ -57,7 +54,8 @@ static inline DirectDictionary::Key getAt(const DirectDictionary::Key & value, c
 
 DirectDictionary::Key DirectDictionary::getValueOrNullByKey(const Key & to_find) const
 {
-    auto stream = source_ptr->loadAll();
+    std::vector<Key> required_key = {to_find};
+    auto stream = source_ptr->loadIds(required_key);
     stream->readPrefix();
 
     bool is_found = false;
@@ -330,18 +328,6 @@ void DirectDictionary::createAttributes()
 }
 
 
-void DirectDictionary::calculateBytesAllocated()
-{
-    bytes_allocated += attributes.size() * sizeof(attributes.front());
-
-    for (const auto & attribute : attributes)
-    {
-        if (attribute.type == AttributeUnderlyingType::utString)
-            bytes_allocated += sizeof(Arena) + attribute.string_arena->size();
-    }
-}
-
-
 template <typename T>
 void DirectDictionary::createAttributeImpl(Attribute & attribute, const Field & null_value)
 {
@@ -427,8 +413,9 @@ void DirectDictionary::getItemsImpl(
         value_by_key[ids[row]] = get_default(row);
 
     std::vector<Key> to_load;
+    to_load.reserve(value_by_key.size());
     for (auto it = value_by_key.begin(); it != value_by_key.end(); ++it)
-        to_load.push_back(static_cast<Key>(it->getKey()));
+        to_load.emplace_back(static_cast<Key>(it->getKey()));
 
     auto stream = source_ptr->loadIds(to_load);
     stream->readPrefix();
@@ -480,8 +467,9 @@ void DirectDictionary::getItemsStringImpl(
         value_by_key[ids[row]] = get_default(row);
 
     std::vector<Key> to_load;
+    to_load.reserve(value_by_key.size());
     for (auto it = value_by_key.begin(); it != value_by_key.end(); ++it)
-        to_load.push_back(static_cast<Key>(it->getKey()));
+        to_load.emplace_back(static_cast<Key>(it->getKey()));
 
     auto stream = source_ptr->loadIds(to_load);
     stream->readPrefix();
@@ -530,8 +518,15 @@ void DirectDictionary::has(const Attribute &, const PaddedPODArray<Key> & ids, P
     const auto rows = ext::size(ids);
 
     HashMap<Key, UInt8> has_key;
+    for (const auto row : ext::range(0, rows))
+        has_key[ids[row]] = 0;
 
-    auto stream = source_ptr->loadAll();
+    std::vector<Key> to_load;
+    to_load.reserve(has_key.size());
+    for (auto it = has_key.begin(); it != has_key.end(); ++it)
+        to_load.emplace_back(static_cast<Key>(it->getKey()));
+
+    auto stream = source_ptr->loadIds(to_load);
     stream->readPrefix();
 
     while (const auto block = stream->read())
@@ -603,8 +598,12 @@ void registerDictionaryDirect(DictionaryFactory & factory)
 
         const String database = config.getString(config_prefix + ".database", "");
         const String name = config.getString(config_prefix + ".name");
-        const DictionaryLifetime dict_lifetime{config, config_prefix + ".lifetime"};
+
-        return std::make_unique<DirectDictionary>(database, name, dict_struct, std::move(source_ptr), dict_lifetime);
+        if (config.has(config_prefix + ".lifetime.min") || config.has(config_prefix + ".lifetime.max"))
+            throw Exception{"'lifetime' parameter is redundant for the dictionary' of layout 'direct'", ErrorCodes::BAD_ARGUMENTS};
+
+
+        return std::make_unique<DirectDictionary>(database, name, dict_struct, std::move(source_ptr));
     };
     factory.registerLayout("direct", create_layout, false);
 }

src/Dictionaries/DirectDictionary.h

@@ -27,7 +27,6 @@ public:
         const std::string & name_,
         const DictionaryStructure & dict_struct_,
         DictionarySourcePtr source_ptr_,
-        const DictionaryLifetime dict_lifetime_,
         BlockPtr saved_block_ = nullptr);
 
     const std::string & getDatabase() const override { return database; }
@@ -36,19 +35,19 @@ public:
 
     std::string getTypeName() const override { return "Direct"; }
 
-    size_t getBytesAllocated() const override { return bytes_allocated; }
+    size_t getBytesAllocated() const override { return 0; }
 
     size_t getQueryCount() const override { return query_count.load(std::memory_order_relaxed); }
 
     double getHitRate() const override { return 1.0; }
 
-    size_t getElementCount() const override { return element_count; }
+    size_t getElementCount() const override { return 0; }
 
-    double getLoadFactor() const override { return static_cast<double>(element_count) / bucket_count; }
+    double getLoadFactor() const override { return 0; }
 
     std::shared_ptr<const IExternalLoadable> clone() const override
     {
-        return std::make_shared<DirectDictionary>(database, name, dict_struct, source_ptr->clone(), dict_lifetime, saved_block);
+        return std::make_shared<DirectDictionary>(database, name, dict_struct, source_ptr->clone(), saved_block);
     }
 
     const IDictionarySource * getSource() const override { return source_ptr.get(); }
@@ -145,9 +144,6 @@ public:
     BlockInputStreamPtr getBlockInputStream(const Names & column_names, size_t max_block_size) const override;
 
 private:
-    template <typename Value>
-    using ContainerType = PaddedPODArray<Value>;
-
     struct Attribute final
     {
         AttributeUnderlyingType type;
@@ -224,9 +220,6 @@ private:
     std::vector<Attribute> attributes;
     const Attribute * hierarchical_attribute = nullptr;
 
-    size_t bytes_allocated = 0;
-    size_t element_count = 0;
-    size_t bucket_count = 0;
     mutable std::atomic<size_t> query_count{0};
 
     BlockPtr saved_block;

src/Dictionaries/getDictionaryConfigurationFromAST.cpp

@@ -56,16 +56,18 @@ void buildLifetimeConfiguration(
     const ASTDictionaryLifetime * lifetime)
 {
 
-    AutoPtr<Element> lifetime_element(doc->createElement("lifetime"));
+    if (lifetime) {
-    AutoPtr<Element> min_element(doc->createElement("min"));
+        AutoPtr<Element> lifetime_element(doc->createElement("lifetime"));
-    AutoPtr<Element> max_element(doc->createElement("max"));
+        AutoPtr<Element> min_element(doc->createElement("min"));
-    AutoPtr<Text> min_sec(doc->createTextNode(toString(lifetime->min_sec)));
+        AutoPtr<Element> max_element(doc->createElement("max"));
-    min_element->appendChild(min_sec);
+        AutoPtr<Text> min_sec(doc->createTextNode(toString(lifetime->min_sec)));
-    AutoPtr<Text> max_sec(doc->createTextNode(toString(lifetime->max_sec)));
+        min_element->appendChild(min_sec);
-    max_element->appendChild(max_sec);
+        AutoPtr<Text> max_sec(doc->createTextNode(toString(lifetime->max_sec)));
-    lifetime_element->appendChild(min_element);
+        max_element->appendChild(max_sec);
-    lifetime_element->appendChild(max_element);
+        lifetime_element->appendChild(min_element);
-    root->appendChild(lifetime_element);
+        lifetime_element->appendChild(max_element);
+        root->appendChild(lifetime_element);
+    }
 }
 
 /*
@@ -411,7 +413,8 @@ void checkAST(const ASTCreateQuery & query)
     if (query.dictionary->layout == nullptr)
         throw Exception("Cannot create dictionary with empty layout", ErrorCodes::INCORRECT_DICTIONARY_DEFINITION);
 
-    if (query.dictionary->lifetime == nullptr)
+    const auto is_direct_layout = !strcasecmp(query.dictionary->layout->layout_type.data(), "direct");
+    if (query.dictionary->lifetime == nullptr && !is_direct_layout)
         throw Exception("Cannot create dictionary with empty lifetime", ErrorCodes::INCORRECT_DICTIONARY_DEFINITION);
 
     if (query.dictionary->primary_key == nullptr)

src/Interpreters/IExternalLoadable.h

@@ -19,10 +19,11 @@ namespace DB
 /// Min and max lifetimes for a loadable object or it's entry
 struct ExternalLoadableLifetime
 {
-    UInt64 min_sec;
+    UInt64 min_sec = 0;
-    UInt64 max_sec;
+    UInt64 max_sec = 0;
 
     ExternalLoadableLifetime(const Poco::Util::AbstractConfiguration & config, const std::string & config_prefix);
+    ExternalLoadableLifetime() {}
 };
 
 /// Get delay before trying to load again after error.

tests/queries/0_stateless/01268_dictionary_direct_layout.sql

@@ -63,7 +63,6 @@ CREATE DICTIONARY ordinary_db.dict1
 )
 PRIMARY KEY key_column
 SOURCE(CLICKHOUSE(HOST 'localhost' PORT 9000 USER 'default' TABLE 'table_for_dict1' PASSWORD '' DB 'database_for_dict'))
-LIFETIME(MIN 1 MAX 600)
 LAYOUT(DIRECT()) SETTINGS(max_result_bytes=1);
 
 CREATE DICTIONARY ordinary_db.dict2
@@ -74,7 +73,6 @@ CREATE DICTIONARY ordinary_db.dict2
 )
 PRIMARY KEY region_id
 SOURCE(CLICKHOUSE(HOST 'localhost' PORT 9000 USER 'default' TABLE 'table_for_dict2' PASSWORD '' DB 'database_for_dict'))
-LIFETIME(MIN 1 MAX 600)
 LAYOUT(DIRECT());
 
 CREATE DICTIONARY ordinary_db.dict3
@@ -85,7 +83,6 @@ CREATE DICTIONARY ordinary_db.dict3
 )
 PRIMARY KEY region_id
 SOURCE(CLICKHOUSE(HOST 'localhost' PORT 9000 USER 'default' TABLE 'table_for_dict3' PASSWORD '' DB 'database_for_dict'))
-LIFETIME(MIN 1 MAX 600)
 LAYOUT(DIRECT());
 
 SELECT 'INITIALIZING DICTIONARY';