From 6c73a4669353e57ad4b2b16cc6a9a8eb9d27a819 Mon Sep 17 00:00:00 2001 From: Zhongyi Xie Date: Tue, 22 May 2018 13:51:21 -0700 Subject: [PATCH] Fix a backward compatibility problem with table_properties being nullptr Summary: Currently when ldb built from master tries to open a DB from version 2.2, there will be a segfault because table_properties didn't exist back then. Closes https://github.com/facebook/rocksdb/pull/3890 Differential Revision: D8100914 Pulled By: miasantreble fbshipit-source-id: b255e8aedc54695432be2e704839c857dabdd65a --- table/block_based_table_reader.cc | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/table/block_based_table_reader.cc b/table/block_based_table_reader.cc index e719ee31d..c761ec082 100644 --- a/table/block_based_table_reader.cc +++ b/table/block_based_table_reader.cc @@ -176,16 +176,20 @@ Cache::Handle* GetEntryFromCache(Cache* block_cache, const Slice& key, // For hash based index, return true if prefix_extractor and // prefix_extractor_block mismatch, false otherwise. This flag will be used // as total_order_seek via NewIndexIterator -bool PrefixExtractorChanged(std::string prefix_extractor_block, - const SliceTransform* prefix_extractor) { +bool PrefixExtractorChanged( + std::shared_ptr table_properties, + const SliceTransform* prefix_extractor) { // BlockBasedTableOptions::kHashSearch requires prefix_extractor to be set. // Turn off hash index in prefix_extractor is not set; if prefix_extractor // is set but prefix_extractor_block is not set, also disable hash index - if (prefix_extractor == nullptr || prefix_extractor_block.empty()) { + if (prefix_extractor == nullptr || table_properties == nullptr || + table_properties->prefix_extractor_name.empty()) { return true; } + // prefix_extractor and prefix_extractor_block are both non-empty - if (prefix_extractor_block.compare(prefix_extractor->Name()) != 0) { + if (table_properties->prefix_extractor_name.compare( + prefix_extractor->Name()) != 0) { return true; } else { return false; @@ -887,8 +891,8 @@ Status BlockBasedTable::Open(const ImmutableCFOptions& ioptions, bool prefix_extractor_changed = false; // check prefix_extractor match only if hash based index is used if (rep->index_type == BlockBasedTableOptions::kHashSearch) { - prefix_extractor_changed = PrefixExtractorChanged( - rep->table_properties->prefix_extractor_name, prefix_extractor); + prefix_extractor_changed = + PrefixExtractorChanged(rep->table_properties, prefix_extractor); } unique_ptr iter(new_table->NewIndexIterator( ReadOptions(), prefix_extractor_changed, nullptr, &index_entry)); @@ -2018,8 +2022,8 @@ void BlockBasedTableIterator::FindKeyBackward() { InternalIterator* BlockBasedTable::NewIterator( const ReadOptions& read_options, const SliceTransform* prefix_extractor, Arena* arena, bool skip_filters) { - bool prefix_extractor_changed = PrefixExtractorChanged( - rep_->table_properties->prefix_extractor_name, prefix_extractor); + bool prefix_extractor_changed = + PrefixExtractorChanged(rep_->table_properties, prefix_extractor); if (arena == nullptr) { return new BlockBasedTableIterator( this, read_options, rep_->internal_comparator, @@ -2124,8 +2128,8 @@ Status BlockBasedTable::Get(const ReadOptions& read_options, const Slice& key, // BlockPrefixIndex. Only do this check when index_type is kHashSearch. bool prefix_extractor_changed = false; if (rep_->index_type == BlockBasedTableOptions::kHashSearch) { - prefix_extractor_changed = PrefixExtractorChanged( - rep_->table_properties->prefix_extractor_name, prefix_extractor); + prefix_extractor_changed = + PrefixExtractorChanged(rep_->table_properties, prefix_extractor); } auto iiter = NewIndexIterator(read_options, prefix_extractor_changed, &iiter_on_stack, /* index_entry */ nullptr,