From 5f5470c2cdefdf57e6de9bd49b7312f7bc2a3e50 Mon Sep 17 00:00:00 2001 From: Maksim Kita Date: Sun, 15 Aug 2021 13:32:56 +0300 Subject: [PATCH] Removed DenseHashMap, DenseHashSet --- src/Common/DenseHashMap.h | 29 ------ src/Common/DenseHashSet.h | 25 ----- src/Common/SparseHashMap.h | 1 - src/Core/NamesAndTypes.cpp | 12 +-- src/Storages/MergeTree/IMergeTreeReader.cpp | 9 +- src/Storages/MergeTree/IMergeTreeReader.h | 5 +- src/Storages/StorageInMemoryMetadata.cpp | 102 +++++++++++--------- 7 files changed, 72 insertions(+), 111 deletions(-) delete mode 100644 src/Common/DenseHashMap.h delete mode 100644 src/Common/DenseHashSet.h diff --git a/src/Common/DenseHashMap.h b/src/Common/DenseHashMap.h deleted file mode 100644 index 9ac21c82676..00000000000 --- a/src/Common/DenseHashMap.h +++ /dev/null @@ -1,29 +0,0 @@ -#pragma once -#include - -/// DenseHashMap is a wrapper for google::dense_hash_map. -/// Some hacks are needed to make it work in "Arcadia". -/// "Arcadia" is a proprietary monorepository in Yandex. -/// It uses slightly changed version of sparsehash with a different set of hash functions (which we don't need). -/// Those defines are needed to make it compile. -#if defined(ARCADIA_BUILD) -#define HASH_FUN_H -template -struct THash; -#endif - -#include - -#if !defined(ARCADIA_BUILD) - template , - class EqualKey = std::equal_to, - class Alloc = google::libc_allocator_with_realloc>> - using DenseHashMap = google::dense_hash_map; -#else - template , - class EqualKey = std::equal_to, - class Alloc = google::sparsehash::libc_allocator_with_realloc>> - using DenseHashMap = google::sparsehash::dense_hash_map; - - #undef THash -#endif diff --git a/src/Common/DenseHashSet.h b/src/Common/DenseHashSet.h deleted file mode 100644 index e8c06f36aa3..00000000000 --- a/src/Common/DenseHashSet.h +++ /dev/null @@ -1,25 +0,0 @@ -#pragma once - -/// DenseHashSet is a wrapper for google::dense_hash_set. -/// See comment in DenseHashMap.h -#if defined(ARCADIA_BUILD) -#define HASH_FUN_H -template -struct THash; -#endif - -#include - -#if !defined(ARCADIA_BUILD) - template , - class EqualKey = std::equal_to, - class Alloc = google::libc_allocator_with_realloc> - using DenseHashSet = google::dense_hash_set; -#else - template , - class EqualKey = std::equal_to, - class Alloc = google::sparsehash::libc_allocator_with_realloc> - using DenseHashSet = google::sparsehash::dense_hash_set; - - #undef THash -#endif diff --git a/src/Common/SparseHashMap.h b/src/Common/SparseHashMap.h index f01fc633d84..0f86cc13612 100644 --- a/src/Common/SparseHashMap.h +++ b/src/Common/SparseHashMap.h @@ -1,7 +1,6 @@ #pragma once /// SparseHashMap is a wrapper for google::sparse_hash_map. -/// See comment in DenseHashMap.h #if defined(ARCADIA_BUILD) #define HASH_FUN_H template diff --git a/src/Core/NamesAndTypes.cpp b/src/Core/NamesAndTypes.cpp index 54f83fc13fc..b47f5a6823b 100644 --- a/src/Core/NamesAndTypes.cpp +++ b/src/Core/NamesAndTypes.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -6,7 +7,6 @@ #include #include #include -#include namespace DB @@ -163,8 +163,7 @@ NamesAndTypesList NamesAndTypesList::filter(const Names & names) const NamesAndTypesList NamesAndTypesList::addTypes(const Names & names) const { /// NOTE: It's better to make a map in `IStorage` than to create it here every time again. - DenseHashMap types; - types.set_empty_key(StringRef()); + HashMapWithSavedHash types; for (const auto & column : *this) types[column.name] = &column.type; @@ -172,10 +171,11 @@ NamesAndTypesList NamesAndTypesList::addTypes(const Names & names) const NamesAndTypesList res; for (const String & name : names) { - auto it = types.find(name); + const auto * it = types.find(name); if (it == types.end()) - throw Exception("No column " + name, ErrorCodes::THERE_IS_NO_COLUMN); - res.emplace_back(name, *it->second); + throw Exception(ErrorCodes::THERE_IS_NO_COLUMN, "No column {}", name); + + res.emplace_back(name, *it->getMapped()); } return res; diff --git a/src/Storages/MergeTree/IMergeTreeReader.cpp b/src/Storages/MergeTree/IMergeTreeReader.cpp index 5378b84a5d0..d659259e1a9 100644 --- a/src/Storages/MergeTree/IMergeTreeReader.cpp +++ b/src/Storages/MergeTree/IMergeTreeReader.cpp @@ -48,7 +48,6 @@ IMergeTreeReader::IMergeTreeReader( part_columns = Nested::collect(part_columns); } - columns_from_part.set_empty_key(StringRef()); for (const auto & column_from_part : part_columns) columns_from_part[column_from_part.name] = &column_from_part.type; } @@ -213,7 +212,7 @@ NameAndTypePair IMergeTreeReader::getColumnFromPart(const NameAndTypePair & requ { auto name_in_storage = required_column.getNameInStorage(); - decltype(columns_from_part.begin()) it; + ColumnsFromPart::ConstLookupResult it; if (alter_conversions.isColumnRenamed(name_in_storage)) { String old_name = alter_conversions.getColumnOldName(name_in_storage); @@ -227,7 +226,7 @@ NameAndTypePair IMergeTreeReader::getColumnFromPart(const NameAndTypePair & requ if (it == columns_from_part.end()) return required_column; - const auto & type = *it->second; + const DataTypePtr & type = *it->getMapped(); if (required_column.isSubcolumn()) { auto subcolumn_name = required_column.getSubcolumnName(); @@ -236,10 +235,10 @@ NameAndTypePair IMergeTreeReader::getColumnFromPart(const NameAndTypePair & requ if (!subcolumn_type) return required_column; - return {String(it->first), subcolumn_name, type, subcolumn_type}; + return {String(it->getKey()), subcolumn_name, type, subcolumn_type}; } - return {String(it->first), type}; + return {String(it->getKey()), type}; } void IMergeTreeReader::performRequiredConversions(Columns & res_columns) diff --git a/src/Storages/MergeTree/IMergeTreeReader.h b/src/Storages/MergeTree/IMergeTreeReader.h index 8d80719efaf..696cc2f105b 100644 --- a/src/Storages/MergeTree/IMergeTreeReader.h +++ b/src/Storages/MergeTree/IMergeTreeReader.h @@ -1,7 +1,7 @@ #pragma once #include -#include +#include #include #include @@ -95,7 +95,8 @@ private: /// Actual data type of columns in part - DenseHashMap columns_from_part; + using ColumnsFromPart = HashMapWithSavedHash; + ColumnsFromPart columns_from_part; }; } diff --git a/src/Storages/StorageInMemoryMetadata.cpp b/src/Storages/StorageInMemoryMetadata.cpp index 91f69cdac7d..5183b925141 100644 --- a/src/Storages/StorageInMemoryMetadata.cpp +++ b/src/Storages/StorageInMemoryMetadata.cpp @@ -1,7 +1,7 @@ #include -#include -#include +#include +#include #include #include #include @@ -320,8 +320,7 @@ Block StorageInMemoryMetadata::getSampleBlockForColumns( { Block res; - DenseHashMap virtuals_map; - virtuals_map.set_empty_key(StringRef()); + HashMapWithSavedHash virtuals_map; /// Virtual columns must be appended after ordinary, because user can /// override them. @@ -335,9 +334,9 @@ Block StorageInMemoryMetadata::getSampleBlockForColumns( { res.insert({column->type->createColumn(), column->type, column->name}); } - else if (auto it = virtuals_map.find(name); it != virtuals_map.end()) + else if (auto * it = virtuals_map.find(name); it != virtuals_map.end()) { - const auto & type = *it->second; + const auto & type = *it->getMapped(); res.insert({type->createColumn(), type, name}); } else @@ -470,8 +469,8 @@ bool StorageInMemoryMetadata::hasSelectQuery() const namespace { - using NamesAndTypesMap = DenseHashMap; - using UniqueStrings = DenseHashSet; + using NamesAndTypesMap = HashMapWithSavedHash; + using UniqueStrings = HashSetWithSavedHash; String listOfColumns(const NamesAndTypesList & available_columns) { @@ -488,20 +487,12 @@ namespace NamesAndTypesMap getColumnsMap(const NamesAndTypesList & columns) { NamesAndTypesMap res; - res.set_empty_key(StringRef()); for (const auto & column : columns) res.insert({column.name, column.type.get()}); return res; } - - UniqueStrings initUniqueStrings() - { - UniqueStrings strings; - strings.set_empty_key(StringRef()); - return strings; - } } void StorageInMemoryMetadata::check(const Names & column_names, const NamesAndTypesList & virtuals, const StorageID & storage_id) const @@ -514,11 +505,12 @@ void StorageInMemoryMetadata::check(const Names & column_names, const NamesAndTy } const auto virtuals_map = getColumnsMap(virtuals); - auto unique_names = initUniqueStrings(); + UniqueStrings unique_names; for (const auto & name : column_names) { - bool has_column = getColumns().hasColumnOrSubcolumn(ColumnsDescription::AllPhysical, name) || virtuals_map.count(name); + bool has_column = getColumns().hasColumnOrSubcolumn(ColumnsDescription::AllPhysical, name) + || virtuals_map.find(name) != nullptr; if (!has_column) { @@ -540,23 +532,31 @@ void StorageInMemoryMetadata::check(const NamesAndTypesList & provided_columns) const NamesAndTypesList & available_columns = getColumns().getAllPhysical(); const auto columns_map = getColumnsMap(available_columns); - auto unique_names = initUniqueStrings(); + UniqueStrings unique_names; + for (const NameAndTypePair & column : provided_columns) { - auto it = columns_map.find(column.name); + const auto * it = columns_map.find(column.name); if (columns_map.end() == it) throw Exception( - "There is no column with name " + column.name + ". There are columns: " + listOfColumns(available_columns), - ErrorCodes::NO_SUCH_COLUMN_IN_TABLE); + ErrorCodes::NO_SUCH_COLUMN_IN_TABLE, + "There is no column with name {}. There are columns: {}", + column.name, + listOfColumns(available_columns)); - if (!column.type->equals(*it->second)) + if (!column.type->equals(*it->getMapped())) throw Exception( - "Type mismatch for column " + column.name + ". Column has type " + it->second->getName() + ", got type " - + column.type->getName(), - ErrorCodes::TYPE_MISMATCH); + ErrorCodes::TYPE_MISMATCH, + "Type mismatch for column {}. Column has type {}, got type {}", + column.name, + it->getMapped()->getName(), + column.type->getName()); if (unique_names.end() != unique_names.find(column.name)) - throw Exception("Column " + column.name + " queried more than once", ErrorCodes::COLUMN_QUERIED_MORE_THAN_ONCE); + throw Exception(ErrorCodes::COLUMN_QUERIED_MORE_THAN_ONCE, + "Column {} queried more than once", + column.name); + unique_names.insert(column.name); } } @@ -572,26 +572,38 @@ void StorageInMemoryMetadata::check(const NamesAndTypesList & provided_columns, "Empty list of columns queried. There are columns: " + listOfColumns(available_columns), ErrorCodes::EMPTY_LIST_OF_COLUMNS_QUERIED); - auto unique_names = initUniqueStrings(); + UniqueStrings unique_names; + for (const String & name : column_names) { - auto it = provided_columns_map.find(name); + const auto * it = provided_columns_map.find(name); if (provided_columns_map.end() == it) continue; - auto jt = available_columns_map.find(name); + const auto * jt = available_columns_map.find(name); if (available_columns_map.end() == jt) throw Exception( - "There is no column with name " + name + ". There are columns: " + listOfColumns(available_columns), - ErrorCodes::NO_SUCH_COLUMN_IN_TABLE); + ErrorCodes::NO_SUCH_COLUMN_IN_TABLE, + "There is no column with name {}. There are columns: {}", + name, + listOfColumns(available_columns)); - if (!it->second->equals(*jt->second)) + const auto & provided_column_type = *it->getMapped(); + const auto & available_column_type = *jt->getMapped(); + + if (!provided_column_type.equals(available_column_type)) throw Exception( - "Type mismatch for column " + name + ". Column has type " + jt->second->getName() + ", got type " + it->second->getName(), - ErrorCodes::TYPE_MISMATCH); + ErrorCodes::TYPE_MISMATCH, + "Type mismatch for column {}. Column has type {}, got type {}", + name, + provided_column_type.getName(), + available_column_type.getName()); if (unique_names.end() != unique_names.find(name)) - throw Exception("Column " + name + " queried more than once", ErrorCodes::COLUMN_QUERIED_MORE_THAN_ONCE); + throw Exception(ErrorCodes::COLUMN_QUERIED_MORE_THAN_ONCE, + "Column {} queried more than once", + name); + unique_names.insert(name); } } @@ -612,17 +624,21 @@ void StorageInMemoryMetadata::check(const Block & block, bool need_all) const names_in_block.insert(column.name); - auto it = columns_map.find(column.name); + const auto * it = columns_map.find(column.name); if (columns_map.end() == it) throw Exception( - "There is no column with name " + column.name + ". There are columns: " + listOfColumns(available_columns), - ErrorCodes::NO_SUCH_COLUMN_IN_TABLE); + ErrorCodes::NO_SUCH_COLUMN_IN_TABLE, + "There is no column with name {}. There are columns: {}", + column.name, + listOfColumns(available_columns)); - if (!column.type->equals(*it->second)) + if (!column.type->equals(*it->getMapped())) throw Exception( - "Type mismatch for column " + column.name + ". Column has type " + it->second->getName() + ", got type " - + column.type->getName(), - ErrorCodes::TYPE_MISMATCH); + ErrorCodes::TYPE_MISMATCH, + "Type mismatch for column {}. Column has type {}, got type {}", + column.name, + it->getMapped()->getName(), + column.type->getName()); } if (need_all && names_in_block.size() < columns_map.size())