Storage dictionary #763
Storage dictionary #763
Conversation
dbms/src/Dictionaries/IDictionary.h
Outdated
| @@ -53,6 +55,8 @@ struct IDictionaryBase | |||
|
|
|||
| virtual bool isInjective(const std::string & attribute_name) const = 0; | |||
|
|
|||
| virtual BlockInputStreamPtr getBlockInputStream(const Names & column_names) const = 0; | |||
| { | ||
| const auto keys_size = key_columns.size(); | ||
| size_t sum_keys_size{}; | ||
| for (const auto i : ext::range(0, keys_size)) | ||
|
|
||
| Arena arena; |
There was a problem hiding this comment.
Temporary Arena for every row is very expensive.
Better to implement overload IColumn::serializeValueIntoArena for ArenaWithFreeLists.
(or call it IColumn::serializeValueIntoArenaWithFreeLists and provide a wrapper here.)
| @@ -208,6 +210,10 @@ class CacheDictionary final : public IDictionary | |||
| const std::vector<Key> & requested_ids, PresentIdHandler && on_cell_updated, | |||
| AbsentIdHandler && on_id_not_found) const; | |||
|
|
|||
| PaddedPODArray<Key> getCachedIds() const; | |||
There was a problem hiding this comment.
This method should not be public.
BTW, you don't have the same method in ComplexKeyCacheDictionary
and this is inconsistent. Better to remove this method from here.
| } | ||
|
|
||
| using BlockInputStreamType = DictionaryBlockInputStream<ComplexKeyCacheDictionary, UInt64>; | ||
| return std::move(std::make_unique<BlockInputStreamType>(*this, keys, column_names)); |
There was a problem hiding this comment.
Why not std::make_shared instead of std::move(std::make_unique<BlockInputStreamType>(?
| std::vector<StringRef> ComplexKeyHashedDictionary::getKeys(const Attribute & attribute) const | ||
| { | ||
| const ContainerType<T> & attr = *std::get<ContainerPtrType<T>>(attribute.maps); | ||
| std::vector<StringRef> keys; |
There was a problem hiding this comment.
We should reserve space for vector.
In other places too.
There was a problem hiding this comment.
added reserve call
| BlockInputStreamPtr ComplexKeyHashedDictionary::getBlockInputStream(const Names & column_names) const | ||
| { | ||
| using BlockInputStreamType = DictionaryBlockInputStream<ComplexKeyHashedDictionary, UInt64>; | ||
| return std::move(std::make_unique<BlockInputStreamType>(*this, getKeys(), column_names)); |
|
|
||
| /* | ||
| * BlockInputStream implementation for external dictionaries | ||
| * read() returns single block consisting of the in-memory contents of the dictionaries |
There was a problem hiding this comment.
Single block will be too large for some dictionaries. It's better to generate blocks of max_block_size.
| if (attribute) { | ||
| name = attribute->name; | ||
| } | ||
| if (column_names.find(name) != column_names.end()) { |
| columns.emplace_back(column, attribute.type, attribute.name); | ||
| } | ||
| } | ||
| block = Block(columns); |
There was a problem hiding this comment.
Now std::move will be appropriate.
dbms/src/Storages/StorageFactory.cpp
Outdated
| @@ -262,7 +249,7 @@ StoragePtr StorageFactory::get( | |||
| } | |||
| else if (name == "View") | |||
| { | |||
| return StorageView::create( | |||
| return StorageView::create( | |||
There was a problem hiding this comment.
You should setup your IDE to trim trailing whitespaces.
dbms/src/Storages/StorageFactory.cpp
Outdated
| @@ -225,15 +225,6 @@ static void setGraphitePatternsFromConfig(const Context & context, | |||
| } | |||
|
|
|||
|
|
|||
| /// Some types are only for intermediate values of expressions and cannot be used in tables. | |||
There was a problem hiding this comment.
Bad merge. You have deleted my changes.
| } | ||
| }; | ||
|
|
||
| } |
There was a problem hiding this comment.
Better to setup your IDE to add newlines at end of file. This does not matter a lot (for C++11), but will be more consistent.
…ion in ComplexKeyCacheDictionary; style fixes
KochetovNicolai
force-pushed
the
storage-dictionary
branch
from
1e25259
to
38e9e83
Compare
Added:
IBlockInputStreamrepresenting in-memry stored dictionary dataChanged:
ComplexKeyHashDictionaryandComplexKeyCacheDictionarykey serialization format