diff --git a/src/Storages/MergeTree/MergeTreeDataMergerMutator.cpp b/src/Storages/MergeTree/MergeTreeDataMergerMutator.cpp index 6279d2d7d6f..baea7e72b21 100644 --- a/src/Storages/MergeTree/MergeTreeDataMergerMutator.cpp +++ b/src/Storages/MergeTree/MergeTreeDataMergerMutator.cpp @@ -1663,7 +1663,12 @@ NameToNameVector MergeTreeDataMergerMutator::collectFilesForRenames( { if (command.type == MutationCommand::Type::DROP_INDEX) { - if (source_part->checksums.has(INDEX_FILE_PREFIX + command.column_name + ".idx")) + if (source_part->checksums.has(INDEX_FILE_PREFIX + command.column_name + ".idx2")) + { + rename_vector.emplace_back(INDEX_FILE_PREFIX + command.column_name + ".idx2", ""); + rename_vector.emplace_back(INDEX_FILE_PREFIX + command.column_name + mrk_extension, ""); + } + else if (source_part->checksums.has(INDEX_FILE_PREFIX + command.column_name + ".idx")) { rename_vector.emplace_back(INDEX_FILE_PREFIX + command.column_name + ".idx", ""); rename_vector.emplace_back(INDEX_FILE_PREFIX + command.column_name + mrk_extension, ""); @@ -1749,6 +1754,7 @@ NameSet MergeTreeDataMergerMutator::collectFilesToSkip( for (const auto & index : indices_to_recalc) { files_to_skip.insert(index->getFileName() + ".idx"); + files_to_skip.insert(index->getFileName() + ".idx2"); files_to_skip.insert(index->getFileName() + mrk_extension); } for (const auto & projection : projections_to_recalc) @@ -1893,8 +1899,11 @@ std::set MergeTreeDataMergerMutator::getIndicesToRecalculate( { const auto & index = indices[i]; + bool has_index = + source_part->checksums.has(INDEX_FILE_PREFIX + index.name + ".idx") || + source_part->checksums.has(INDEX_FILE_PREFIX + index.name + ".idx2"); // If we ask to materialize and it already exists - if (!source_part->checksums.has(INDEX_FILE_PREFIX + index.name + ".idx") && materialized_indices.count(index.name)) + if (!has_index && materialized_indices.count(index.name)) { if (indices_to_recalc.insert(index_factory.get(index)).second) { diff --git a/src/Storages/MergeTree/MergeTreeDataPartWriterOnDisk.cpp b/src/Storages/MergeTree/MergeTreeDataPartWriterOnDisk.cpp index 9902add9847..4263640c1e0 100644 --- a/src/Storages/MergeTree/MergeTreeDataPartWriterOnDisk.cpp +++ b/src/Storages/MergeTree/MergeTreeDataPartWriterOnDisk.cpp @@ -9,11 +9,6 @@ namespace ErrorCodes extern const int LOGICAL_ERROR; } -namespace -{ - constexpr auto INDEX_FILE_EXTENSION = ".idx"; -} - void MergeTreeDataPartWriterOnDisk::Stream::finalize() { compressed.next(); @@ -165,7 +160,7 @@ void MergeTreeDataPartWriterOnDisk::initSkipIndices() std::make_unique( stream_name, data_part->volume->getDisk(), - part_path + stream_name, INDEX_FILE_EXTENSION, + part_path + stream_name, index_helper->getSerializedFileExtension(), part_path + stream_name, marks_file_extension, default_codec, settings.max_compress_block_size)); skip_indices_aggregators.push_back(index_helper->createIndexAggregator()); diff --git a/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp b/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp index 0b5351dcf01..f60acca12a7 100644 --- a/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp +++ b/src/Storages/MergeTree/MergeTreeDataSelectExecutor.cpp @@ -1457,9 +1457,10 @@ MarkRanges MergeTreeDataSelectExecutor::filterMarksUsingIndex( size_t & granules_dropped, Poco::Logger * log) { - if (!part->volume->getDisk()->exists(part->getFullRelativePath() + index_helper->getFileName() + ".idx")) + const std::string & path_prefix = part->getFullRelativePath() + index_helper->getFileName(); + if (!index_helper->getDeserializedFormat(part->volume->getDisk(), path_prefix)) { - LOG_DEBUG(log, "File for index {} does not exist. Skipping it.", backQuote(index_helper->index.name)); + LOG_DEBUG(log, "File for index {} does not exist ({}.*). Skipping it.", backQuote(index_helper->index.name), path_prefix); return ranges; } diff --git a/src/Storages/MergeTree/MergeTreeIndexFullText.cpp b/src/Storages/MergeTree/MergeTreeIndexFullText.cpp index 10136cd1069..1c71d77b334 100644 --- a/src/Storages/MergeTree/MergeTreeIndexFullText.cpp +++ b/src/Storages/MergeTree/MergeTreeIndexFullText.cpp @@ -101,14 +101,17 @@ MergeTreeIndexGranuleFullText::MergeTreeIndexGranuleFullText( void MergeTreeIndexGranuleFullText::serializeBinary(WriteBuffer & ostr) const { if (empty()) - throw Exception("Attempt to write empty fulltext index " + backQuote(index_name), ErrorCodes::LOGICAL_ERROR); + throw Exception(ErrorCodes::LOGICAL_ERROR, "Attempt to write empty fulltext index {}.", backQuote(index_name)); for (const auto & bloom_filter : bloom_filters) ostr.write(reinterpret_cast(bloom_filter.getFilter().data()), params.filter_size); } -void MergeTreeIndexGranuleFullText::deserializeBinary(ReadBuffer & istr) +void MergeTreeIndexGranuleFullText::deserializeBinary(ReadBuffer & istr, MergeTreeIndexVersion version) { + if (version != 1) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Unknown index version {}.", version); + for (auto & bloom_filter : bloom_filters) { istr.read(reinterpret_cast( diff --git a/src/Storages/MergeTree/MergeTreeIndexFullText.h b/src/Storages/MergeTree/MergeTreeIndexFullText.h index 1385621f97f..d34cbc61da2 100644 --- a/src/Storages/MergeTree/MergeTreeIndexFullText.h +++ b/src/Storages/MergeTree/MergeTreeIndexFullText.h @@ -45,7 +45,7 @@ struct MergeTreeIndexGranuleFullText final : public IMergeTreeIndexGranule ~MergeTreeIndexGranuleFullText() override = default; void serializeBinary(WriteBuffer & ostr) const override; - void deserializeBinary(ReadBuffer & istr) override; + void deserializeBinary(ReadBuffer & istr, MergeTreeIndexVersion version) override; bool empty() const override { return !has_elems; } diff --git a/src/Storages/MergeTree/MergeTreeIndexGranuleBloomFilter.cpp b/src/Storages/MergeTree/MergeTreeIndexGranuleBloomFilter.cpp index b513437fbe1..6a027b8cb8e 100644 --- a/src/Storages/MergeTree/MergeTreeIndexGranuleBloomFilter.cpp +++ b/src/Storages/MergeTree/MergeTreeIndexGranuleBloomFilter.cpp @@ -84,10 +84,12 @@ bool MergeTreeIndexGranuleBloomFilter::empty() const return !total_rows; } -void MergeTreeIndexGranuleBloomFilter::deserializeBinary(ReadBuffer & istr) +void MergeTreeIndexGranuleBloomFilter::deserializeBinary(ReadBuffer & istr, MergeTreeIndexVersion version) { if (!empty()) - throw Exception("Cannot read data to a non-empty bloom filter index.", ErrorCodes::LOGICAL_ERROR); + throw Exception(ErrorCodes::LOGICAL_ERROR, "Cannot read data to a non-empty bloom filter index."); + if (version != 1) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Unknown index version {}.", version); readVarUInt(total_rows, istr); for (auto & filter : bloom_filters) @@ -102,7 +104,7 @@ void MergeTreeIndexGranuleBloomFilter::deserializeBinary(ReadBuffer & istr) void MergeTreeIndexGranuleBloomFilter::serializeBinary(WriteBuffer & ostr) const { if (empty()) - throw Exception("Attempt to write empty bloom filter index.", ErrorCodes::LOGICAL_ERROR); + throw Exception(ErrorCodes::LOGICAL_ERROR, "Attempt to write empty bloom filter index."); static size_t atom_size = 8; writeVarUInt(total_rows, ostr); diff --git a/src/Storages/MergeTree/MergeTreeIndexGranuleBloomFilter.h b/src/Storages/MergeTree/MergeTreeIndexGranuleBloomFilter.h index cdd4b92f80c..82bd91138a7 100644 --- a/src/Storages/MergeTree/MergeTreeIndexGranuleBloomFilter.h +++ b/src/Storages/MergeTree/MergeTreeIndexGranuleBloomFilter.h @@ -16,8 +16,7 @@ public: bool empty() const override; void serializeBinary(WriteBuffer & ostr) const override; - - void deserializeBinary(ReadBuffer & istr) override; + void deserializeBinary(ReadBuffer & istr, MergeTreeIndexVersion version) override; const std::vector & getFilters() const { return bloom_filters; } diff --git a/src/Storages/MergeTree/MergeTreeIndexMinMax.cpp b/src/Storages/MergeTree/MergeTreeIndexMinMax.cpp index ebf553295be..3a83afbd280 100644 --- a/src/Storages/MergeTree/MergeTreeIndexMinMax.cpp +++ b/src/Storages/MergeTree/MergeTreeIndexMinMax.cpp @@ -40,28 +40,12 @@ void MergeTreeIndexGranuleMinMax::serializeBinary(WriteBuffer & ostr) const const DataTypePtr & type = index_sample_block.getByPosition(i).type; auto serialization = type->getDefaultSerialization(); - if (!type->isNullable()) - { - serialization->serializeBinary(hyperrectangle[i].left, ostr); - serialization->serializeBinary(hyperrectangle[i].right, ostr); - } - else - { - /// NOTE: that this serialization differs from - /// IMergeTreeDataPart::MinMaxIndex::store() due to preserve - /// backward compatibility. - bool is_null = hyperrectangle[i].left.isNull() || hyperrectangle[i].right.isNull(); // one is enough - writeBinary(is_null, ostr); - if (!is_null) - { - serialization->serializeBinary(hyperrectangle[i].left, ostr); - serialization->serializeBinary(hyperrectangle[i].right, ostr); - } - } + serialization->serializeBinary(hyperrectangle[i].left, ostr); + serialization->serializeBinary(hyperrectangle[i].right, ostr); } } -void MergeTreeIndexGranuleMinMax::deserializeBinary(ReadBuffer & istr) +void MergeTreeIndexGranuleMinMax::deserializeBinary(ReadBuffer & istr, MergeTreeIndexVersion version) { hyperrectangle.clear(); Field min_val; @@ -72,29 +56,53 @@ void MergeTreeIndexGranuleMinMax::deserializeBinary(ReadBuffer & istr) const DataTypePtr & type = index_sample_block.getByPosition(i).type; auto serialization = type->getDefaultSerialization(); - if (!type->isNullable()) + switch (version) { - serialization->deserializeBinary(min_val, istr); - serialization->deserializeBinary(max_val, istr); - } - else - { - /// NOTE: that this serialization differs from - /// IMergeTreeDataPart::MinMaxIndex::load() due to preserve - /// backward compatibility. - bool is_null; - readBinary(is_null, istr); - if (!is_null) - { + case 1: + if (!type->isNullable()) + { + serialization->deserializeBinary(min_val, istr); + serialization->deserializeBinary(max_val, istr); + } + else + { + /// NOTE: that this serialization differs from + /// IMergeTreeDataPart::MinMaxIndex::load() to preserve + /// backward compatibility. + /// + /// But this is deprecated format, so this is OK. + + bool is_null; + readBinary(is_null, istr); + if (!is_null) + { + serialization->deserializeBinary(min_val, istr); + serialization->deserializeBinary(max_val, istr); + } + else + { + min_val = Null(); + max_val = Null(); + } + } + break; + + /// New format with proper Nullable support for values that includes Null values + case 2: serialization->deserializeBinary(min_val, istr); serialization->deserializeBinary(max_val, istr); - } - else - { - min_val = Null(); - max_val = Null(); - } + + // NULL_LAST + if (min_val.isNull()) + min_val = PositiveInfinity(); + if (max_val.isNull()) + max_val = PositiveInfinity(); + + break; + default: + throw Exception(ErrorCodes::LOGICAL_ERROR, "Unknown index version {}.", version); } + hyperrectangle.emplace_back(min_val, true, max_val, true); } } @@ -203,6 +211,15 @@ bool MergeTreeIndexMinMax::mayBenefitFromIndexForIn(const ASTPtr & node) const return false; } +MergeTreeIndexFormat MergeTreeIndexMinMax::getDeserializedFormat(const DiskPtr disk, const std::string & relative_path_prefix) const +{ + if (disk->exists(relative_path_prefix + ".idx2")) + return {2, ".idx2"}; + else if (disk->exists(relative_path_prefix + ".idx")) + return {1, ".idx"}; + return {0 /* unknown */, ""}; +} + MergeTreeIndexPtr minmaxIndexCreator( const IndexDescription & index) { diff --git a/src/Storages/MergeTree/MergeTreeIndexMinMax.h b/src/Storages/MergeTree/MergeTreeIndexMinMax.h index 97b9b874484..0e05e25fb36 100644 --- a/src/Storages/MergeTree/MergeTreeIndexMinMax.h +++ b/src/Storages/MergeTree/MergeTreeIndexMinMax.h @@ -21,7 +21,7 @@ struct MergeTreeIndexGranuleMinMax final : public IMergeTreeIndexGranule ~MergeTreeIndexGranuleMinMax() override = default; void serializeBinary(WriteBuffer & ostr) const override; - void deserializeBinary(ReadBuffer & istr) override; + void deserializeBinary(ReadBuffer & istr, MergeTreeIndexVersion version) override; bool empty() const override { return hyperrectangle.empty(); } @@ -81,6 +81,9 @@ public: const SelectQueryInfo & query, ContextPtr context) const override; bool mayBenefitFromIndexForIn(const ASTPtr & node) const override; + + const char* getSerializedFileExtension() const override { return ".idx2"; } + MergeTreeIndexFormat getDeserializedFormat(const DiskPtr disk, const std::string & path_prefix) const override; }; } diff --git a/src/Storages/MergeTree/MergeTreeIndexReader.cpp b/src/Storages/MergeTree/MergeTreeIndexReader.cpp index eaba247009b..0a0f2511914 100644 --- a/src/Storages/MergeTree/MergeTreeIndexReader.cpp +++ b/src/Storages/MergeTree/MergeTreeIndexReader.cpp @@ -1,5 +1,29 @@ #include +namespace +{ + +using namespace DB; + +std::unique_ptr makeIndexReader( + const std::string & extension, + MergeTreeIndexPtr index, + MergeTreeData::DataPartPtr part, + size_t marks_count, + const MarkRanges & all_mark_ranges, + MergeTreeReaderSettings settings) +{ + return std::make_unique( + part->volume->getDisk(), + part->getFullRelativePath() + index->getFileName(), extension, marks_count, + all_mark_ranges, + std::move(settings), nullptr, nullptr, + part->getFileSizeOrZero(index->getFileName() + extension), + &part->index_granularity_info, + ReadBufferFromFileBase::ProfileCallback{}, CLOCK_MONOTONIC_COARSE); +} + +} namespace DB { @@ -7,27 +31,28 @@ namespace DB MergeTreeIndexReader::MergeTreeIndexReader( MergeTreeIndexPtr index_, MergeTreeData::DataPartPtr part_, size_t marks_count_, const MarkRanges & all_mark_ranges_, MergeTreeReaderSettings settings) - : index(index_), stream( - part_->volume->getDisk(), - part_->getFullRelativePath() + index->getFileName(), ".idx", marks_count_, - all_mark_ranges_, - std::move(settings), nullptr, nullptr, - part_->getFileSizeOrZero(index->getFileName() + ".idx"), - &part_->index_granularity_info, - ReadBufferFromFileBase::ProfileCallback{}, CLOCK_MONOTONIC_COARSE) + : index(index_) { - stream.seekToStart(); + const std::string & path_prefix = part_->getFullRelativePath() + index->getFileName(); + auto index_format = index->getDeserializedFormat(part_->volume->getDisk(), path_prefix); + + stream = makeIndexReader(index_format.extension, index_, part_, marks_count_, all_mark_ranges_, std::move(settings)); + version = index_format.version; + + stream->seekToStart(); } +MergeTreeIndexReader::~MergeTreeIndexReader() = default; + void MergeTreeIndexReader::seek(size_t mark) { - stream.seekToMark(mark); + stream->seekToMark(mark); } MergeTreeIndexGranulePtr MergeTreeIndexReader::read() { auto granule = index->createIndexGranule(); - granule->deserializeBinary(*stream.data_buffer); + granule->deserializeBinary(*stream->data_buffer, version); return granule; } diff --git a/src/Storages/MergeTree/MergeTreeIndexReader.h b/src/Storages/MergeTree/MergeTreeIndexReader.h index 68d681458be..4facd43c175 100644 --- a/src/Storages/MergeTree/MergeTreeIndexReader.h +++ b/src/Storages/MergeTree/MergeTreeIndexReader.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -16,6 +17,7 @@ public: size_t marks_count_, const MarkRanges & all_mark_ranges_, MergeTreeReaderSettings settings); + ~MergeTreeIndexReader(); void seek(size_t mark); @@ -23,7 +25,8 @@ public: private: MergeTreeIndexPtr index; - MergeTreeReaderStream stream; + std::unique_ptr stream; + uint8_t version = 0; }; } diff --git a/src/Storages/MergeTree/MergeTreeIndexSet.cpp b/src/Storages/MergeTree/MergeTreeIndexSet.cpp index 6cee80983d6..024b87c9a3e 100644 --- a/src/Storages/MergeTree/MergeTreeIndexSet.cpp +++ b/src/Storages/MergeTree/MergeTreeIndexSet.cpp @@ -48,8 +48,7 @@ MergeTreeIndexGranuleSet::MergeTreeIndexGranuleSet( void MergeTreeIndexGranuleSet::serializeBinary(WriteBuffer & ostr) const { if (empty()) - throw Exception( - "Attempt to write empty set index " + backQuote(index_name), ErrorCodes::LOGICAL_ERROR); + throw Exception(ErrorCodes::LOGICAL_ERROR, "Attempt to write empty set index {}.", backQuote(index_name)); const auto & size_type = DataTypePtr(std::make_shared()); auto size_serialization = size_type->getDefaultSerialization(); @@ -80,8 +79,11 @@ void MergeTreeIndexGranuleSet::serializeBinary(WriteBuffer & ostr) const } } -void MergeTreeIndexGranuleSet::deserializeBinary(ReadBuffer & istr) +void MergeTreeIndexGranuleSet::deserializeBinary(ReadBuffer & istr, MergeTreeIndexVersion version) { + if (version != 1) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Unknown index version {}.", version); + block.clear(); Field field_rows; diff --git a/src/Storages/MergeTree/MergeTreeIndexSet.h b/src/Storages/MergeTree/MergeTreeIndexSet.h index 28afe4f714d..23b336d274b 100644 --- a/src/Storages/MergeTree/MergeTreeIndexSet.h +++ b/src/Storages/MergeTree/MergeTreeIndexSet.h @@ -28,7 +28,7 @@ struct MergeTreeIndexGranuleSet final : public IMergeTreeIndexGranule MutableColumns && columns_); void serializeBinary(WriteBuffer & ostr) const override; - void deserializeBinary(ReadBuffer & istr) override; + void deserializeBinary(ReadBuffer & istr, MergeTreeIndexVersion version) override; size_t size() const { return block.rows(); } bool empty() const override { return !size(); } diff --git a/src/Storages/MergeTree/MergeTreeIndices.h b/src/Storages/MergeTree/MergeTreeIndices.h index 674daeb480d..557af891b74 100644 --- a/src/Storages/MergeTree/MergeTreeIndices.h +++ b/src/Storages/MergeTree/MergeTreeIndices.h @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -17,13 +18,37 @@ constexpr auto INDEX_FILE_PREFIX = "skp_idx_"; namespace DB { +using MergeTreeIndexVersion = uint8_t; +struct MergeTreeIndexFormat +{ + MergeTreeIndexVersion version; + const char* extension; + + operator bool() const { return version != 0; } +}; + /// Stores some info about a single block of data. struct IMergeTreeIndexGranule { virtual ~IMergeTreeIndexGranule() = default; + /// Serialize always last version. virtual void serializeBinary(WriteBuffer & ostr) const = 0; - virtual void deserializeBinary(ReadBuffer & istr) = 0; + + /// Version of the index to deserialize: + /// + /// - 2 -- minmax index for proper Nullable support, + /// - 1 -- everything else. + /// + /// Implementation is responsible for version check, + /// and throw LOGICAL_ERROR in case of unsupported version. + /// + /// See also: + /// - IMergeTreeIndex::getSerializedFileExtension() + /// - IMergeTreeIndex::getDeserializedFormat() + /// - MergeTreeDataMergerMutator::collectFilesToSkip() + /// - MergeTreeDataMergerMutator::collectFilesForRenames() + virtual void deserializeBinary(ReadBuffer & istr, MergeTreeIndexVersion version) = 0; virtual bool empty() const = 0; }; @@ -73,9 +98,26 @@ struct IMergeTreeIndex virtual ~IMergeTreeIndex() = default; - /// gets filename without extension + /// Returns filename without extension. String getFileName() const { return INDEX_FILE_PREFIX + index.name; } + /// Returns extension for serialization. + /// Reimplement if you want new index format. + /// + /// NOTE: In case getSerializedFileExtension() is reimplemented, + /// getDeserializedFormat() should be reimplemented too, + /// and check all previous extensions too + /// (to avoid breaking backward compatibility). + virtual const char* getSerializedFileExtension() const { return ".idx"; } + + /// Returns extension for deserialization. + /// + /// Return pair. + virtual MergeTreeIndexFormat getDeserializedFormat(const DiskPtr, const std::string & /* relative_path_prefix */) const + { + return {1, ".idx"}; + } + /// Checks whether the column is in data skipping index. virtual bool mayBenefitFromIndexForIn(const ASTPtr & node) const = 0; diff --git a/tests/queries/0_stateless/01410_nullable_key_and_index.sql b/tests/queries/0_stateless/01410_nullable_key_and_index.sql index 24ddb226c16..ba473b5c29a 100644 --- a/tests/queries/0_stateless/01410_nullable_key_and_index.sql +++ b/tests/queries/0_stateless/01410_nullable_key_and_index.sql @@ -49,15 +49,11 @@ SET force_primary_key = 0; SELECT * FROM nullable_minmax_index ORDER BY k; SET max_rows_to_read = 6; SELECT * FROM nullable_minmax_index WHERE v IS NULL; --- NOTE: granuals with Null values cannot be filtred in data skipping indexes, --- due to backward compatibility -SET max_rows_to_read = 0; +SET max_rows_to_read = 8; SELECT * FROM nullable_minmax_index WHERE v IS NOT NULL; SET max_rows_to_read = 6; SELECT * FROM nullable_minmax_index WHERE v > 2; --- NOTE: granuals with Null values cannot be filtred in data skipping indexes, --- due to backward compatibility -SET max_rows_to_read = 0; +SET max_rows_to_read = 4; SELECT * FROM nullable_minmax_index WHERE v <= 2; DROP TABLE nullable_key;