From caa86bc59adf42c57354a88f0df38359cbb8059e Mon Sep 17 00:00:00 2001 From: Nikolai Kochetov Date: Mon, 21 May 2018 15:29:52 +0300 Subject: [PATCH] Fixed ColumnWithDictionary::serializeBinaryBulkWithMultipleStreams, added more comments. --- .../AggregateFunctionFactory.cpp | 2 +- dbms/src/Columns/ColumnUnique.h | 12 ++++++------ dbms/src/DataTypes/DataTypeWithDictionary.cpp | 15 +++++++++------ dbms/src/DataTypes/DataTypeWithDictionary.h | 2 +- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/dbms/src/AggregateFunctions/AggregateFunctionFactory.cpp b/dbms/src/AggregateFunctions/AggregateFunctionFactory.cpp index 0bfbc5e7a58..90109ff04c5 100644 --- a/dbms/src/AggregateFunctions/AggregateFunctionFactory.cpp +++ b/dbms/src/AggregateFunctions/AggregateFunctionFactory.cpp @@ -54,7 +54,7 @@ static DataTypes convertTypesWithDictionaryToNested(const DataTypes & types) res_types.push_back(type); } - return std::move(res_types); + return res_types; } AggregateFunctionPtr AggregateFunctionFactory::get( diff --git a/dbms/src/Columns/ColumnUnique.h b/dbms/src/Columns/ColumnUnique.h index 4c61bbe9d47..8bc4a2a06a8 100644 --- a/dbms/src/Columns/ColumnUnique.h +++ b/dbms/src/Columns/ColumnUnique.h @@ -145,7 +145,7 @@ private: void buildIndex(); ColumnType * getRawColumnPtr() { return static_cast(column_holder->assumeMutable().get()); } const ColumnType * getRawColumnPtr() const { return static_cast(column_holder.get()); } - IndexType insert(const StringRefWrapper & ref, IndexType value); + IndexType insertIntoMap(const StringRefWrapper & ref, IndexType value); }; @@ -213,7 +213,7 @@ void ColumnUnique::buildIndex() } template -IndexType ColumnUnique::insert(const StringRefWrapper & ref, IndexType value) +IndexType ColumnUnique::insertIntoMap(const StringRefWrapper & ref, IndexType value) { if (!index) buildIndex(); @@ -242,7 +242,7 @@ size_t ColumnUnique::uniqueInsert(const Field & x) return getDefaultValueIndex(); column->insert(x); - auto pos = insert(StringRefWrapper(column, prev_size), prev_size); + auto pos = insertIntoMap(StringRefWrapper(column, prev_size), prev_size); if (pos != prev_size) column->popBack(1); @@ -272,7 +272,7 @@ size_t ColumnUnique::uniqueInsertData(const char * pos, s if (!index->has(StringRefWrapper(StringRef(pos, length)))) { column->insertData(pos, length); - return static_cast(insert(StringRefWrapper(StringRef(pos, length)), size)); + return static_cast(insertIntoMap(StringRefWrapper(StringRef(pos, length)), size)); } return size; @@ -299,7 +299,7 @@ size_t ColumnUnique::uniqueInsertDataWithTerminatingZero( return getDefaultValueIndex(); } - auto position = insert(StringRefWrapper(column, prev_size), prev_size); + auto position = insertIntoMap(StringRefWrapper(column, prev_size), prev_size); if (position != prev_size) column->popBack(1); @@ -319,7 +319,7 @@ size_t ColumnUnique::uniqueDeserializeAndInsertFromArena( return getDefaultValueIndex(); } - auto index_pos = insert(StringRefWrapper(column, prev_size), prev_size); + auto index_pos = insertIntoMap(StringRefWrapper(column, prev_size), prev_size); if (index_pos != prev_size) column->popBack(1); diff --git a/dbms/src/DataTypes/DataTypeWithDictionary.cpp b/dbms/src/DataTypes/DataTypeWithDictionary.cpp index a9a81660fa5..199904a98c7 100644 --- a/dbms/src/DataTypes/DataTypeWithDictionary.cpp +++ b/dbms/src/DataTypes/DataTypeWithDictionary.cpp @@ -71,17 +71,19 @@ void DataTypeWithDictionary::serializeBinaryBulkWithMultipleStreams( const ColumnWithDictionary & column_with_dictionary = typeid_cast(column); MutableColumnPtr sub_index; - if (limit == 0) - limit = column.size(); + size_t max_limit = column.size() - offset; + limit = limit ? std::min(limit, max_limit) : max_limit; path.push_back(Substream::DictionaryKeys); if (auto stream = getter(path)) { const auto & indexes = column_with_dictionary.getIndexesPtr(); const auto & keys = column_with_dictionary.getUnique()->getNestedColumn(); - sub_index = (*indexes->cut(offset, limit - offset)).mutate(); + sub_index = (*indexes->cut(offset, limit)).mutate(); ColumnPtr unique_indexes = makeSubIndex(*sub_index); + /// unique_indexes->index(sub_index) == indexes[offset:offset + limit] auto used_keys = keys->index(unique_indexes, 0); + /// (used_keys, sub_index) is ColumnWithDictionary for range [offset:offset + limit] UInt64 used_keys_size = used_keys->size(); writeIntBinary(used_keys_size, *stream); @@ -94,7 +96,7 @@ void DataTypeWithDictionary::serializeBinaryBulkWithMultipleStreams( if (!sub_index) throw Exception("Dictionary keys wasn't serialized", ErrorCodes::LOGICAL_ERROR); - indexes_type->serializeBinaryBulk(*sub_index, *stream, offset, limit); + indexes_type->serializeBinaryBulk(*sub_index, *stream, 0, limit); } } @@ -127,7 +129,8 @@ void DataTypeWithDictionary::deserializeBinaryBulkWithMultipleStreams( auto index_col = indexes_type->createColumn(); indexes_type->deserializeBinaryBulk(*index_col, *stream, limit, 0); - column_with_dictionary.getIndexes()->insertRangeFrom(*indexes->index(std::move(index_col), 0), 0, limit); + auto index_size = index_col->size(); + column_with_dictionary.getIndexes()->insertRangeFrom(*indexes->index(std::move(index_col), 0), 0, index_size); } } @@ -226,7 +229,7 @@ MutableColumnPtr DataTypeWithDictionary::createColumn() const if (!column) throw Exception("Unexpected numeric type: " + type->getName(), ErrorCodes::LOGICAL_ERROR); - return std::move(column); + return column; } throw Exception("Unexpected dictionary type for DataTypeWithDictionary: " + type->getName(), diff --git a/dbms/src/DataTypes/DataTypeWithDictionary.h b/dbms/src/DataTypes/DataTypeWithDictionary.h index f8dc96e4669..a76ca22cd33 100644 --- a/dbms/src/DataTypes/DataTypeWithDictionary.h +++ b/dbms/src/DataTypes/DataTypeWithDictionary.h @@ -158,7 +158,7 @@ private: template MutableColumnPtr createColumnImpl() const; - friend class CreateColumnVector; + friend struct CreateColumnVector; }; }