fix a false positive case of parsing table factory from options file (#10094)

Summary:
During options file parsing, reset table factory before attempting to parse it
from string. This avoids mistakenly treating the default table factory as a
newly created one.

Signed-off-by: tabokie <xy.tao@outlook.com>

Pull Request resolved: https://github.com/facebook/rocksdb/pull/10094

Reviewed By: akankshamahajan15

Differential Revision: D36945378

Pulled By: ajkr

fbshipit-source-id: 94b2604e5e87682063b4b78f6370f3e8f101dc44
main
tabokie 2 years ago committed by Facebook GitHub Bot
parent d665afdbf3
commit 1d2950b8dd
  1. 3
      options/options_parser.cc
  2. 20
      options/options_test.cc

@ -453,12 +453,13 @@ Status RocksDBOptionsParser::EndSection(
section_arg); section_arg);
} }
// Ignore error as table factory deserialization is optional // Ignore error as table factory deserialization is optional
cf_opt->table_factory.reset();
s = TableFactory::CreateFromString( s = TableFactory::CreateFromString(
config_options, config_options,
section_title.substr( section_title.substr(
opt_section_titles[kOptionSectionTableOptions].size()), opt_section_titles[kOptionSectionTableOptions].size()),
&(cf_opt->table_factory)); &(cf_opt->table_factory));
if (s.ok()) { if (s.ok() && cf_opt->table_factory != nullptr) {
s = cf_opt->table_factory->ConfigureFromMap(config_options, opt_map); s = cf_opt->table_factory->ConfigureFromMap(config_options, opt_map);
// Translate any errors (NotFound, NotSupported, to InvalidArgument // Translate any errors (NotFound, NotSupported, to InvalidArgument
if (s.ok() || s.IsInvalidArgument()) { if (s.ok() || s.IsInvalidArgument()) {

@ -47,6 +47,22 @@ namespace ROCKSDB_NAMESPACE {
class OptionsTest : public testing::Test {}; class OptionsTest : public testing::Test {};
class UnregisteredTableFactory : public TableFactory {
public:
UnregisteredTableFactory() {}
const char* Name() const override { return "Unregistered"; }
using TableFactory::NewTableReader;
Status NewTableReader(const ReadOptions&, const TableReaderOptions&,
std::unique_ptr<RandomAccessFileReader>&&, uint64_t,
std::unique_ptr<TableReader>*, bool) const override {
return Status::NotSupported();
}
TableBuilder* NewTableBuilder(const TableBuilderOptions&,
WritableFileWriter*) const override {
return nullptr;
}
};
#ifndef ROCKSDB_LITE // GetOptionsFromMap is not supported in ROCKSDB_LITE #ifndef ROCKSDB_LITE // GetOptionsFromMap is not supported in ROCKSDB_LITE
TEST_F(OptionsTest, GetOptionsFromMapTest) { TEST_F(OptionsTest, GetOptionsFromMapTest) {
std::unordered_map<std::string, std::string> cf_options_map = { std::unordered_map<std::string, std::string> cf_options_map = {
@ -3662,6 +3678,10 @@ TEST_F(OptionsParserTest, DumpAndParse) {
cf_opt.table_factory.reset(test::RandomTableFactory(&rnd, c)); cf_opt.table_factory.reset(test::RandomTableFactory(&rnd, c));
} else if (c == 4) { } else if (c == 4) {
cf_opt.table_factory.reset(NewBlockBasedTableFactory(special_bbto)); cf_opt.table_factory.reset(NewBlockBasedTableFactory(special_bbto));
} else if (c == 5) {
// A table factory that doesn't support deserialization should be
// supported.
cf_opt.table_factory.reset(new UnregisteredTableFactory());
} }
base_cf_opts.emplace_back(cf_opt); base_cf_opts.emplace_back(cf_opt);
} }

Loading…
Cancel
Save