From 8d50a7c9df2352e4949aa456206bbd7b9ee49620 Mon Sep 17 00:00:00 2001 From: chuhao zeng Date: Wed, 20 Dec 2023 18:04:08 -0800 Subject: [PATCH] Fix ldbcmd cant use custom comparator (#12159) Summary: According to this [Q&A](https://github.com/facebook/rocksdb/wiki/RocksDB-FAQ#:~:text=Q%3A%20If%20I%20use%20non%2Ddefault%20comparators%20or%20merge%20operators%2C%20can%20I%20still%20use%20ldb%20tool%3F), user should be able to use LDB with passing a customized comparator into the option. In the process of opening DB in order to perform ldb commands, there is a exception saying comparator not match even if a option with customized comparator is provided. After initializing the column family to open DB, the `LDBCommand::OverrideBaseCFOptions` method does not update the comparator inside column family descriptor using the passed in options. This can cause a mismatch while doing version edit, and in function `ToggleUDT CompareComparator` it will failed and return a exception saying comparator not match. Propose fix by updating the column family descriptor's option using the user passed in option. Also a test case is provided to illustrate the steps. Pull Request resolved: https://github.com/facebook/rocksdb/pull/12159 Reviewed By: hx235 Differential Revision: D52267367 Pulled By: ajkr fbshipit-source-id: c240f93f440e02cb485893de058a46c6dbf9654b --- tools/ldb_cmd.cc | 4 ++++ tools/ldb_cmd_test.cc | 45 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/tools/ldb_cmd.cc b/tools/ldb_cmd.cc index 1e7feb712c..4b57c84f13 100644 --- a/tools/ldb_cmd.cc +++ b/tools/ldb_cmd.cc @@ -762,6 +762,10 @@ void LDBCommand::OverrideBaseCFOptions(ColumnFamilyOptions* cf_opts) { } } + if (options_.comparator != nullptr) { + cf_opts->comparator = options_.comparator; + } + cf_opts->force_consistency_checks = force_consistency_checks_; if (use_table_options) { cf_opts->table_factory.reset(NewBlockBasedTableFactory(table_options)); diff --git a/tools/ldb_cmd_test.cc b/tools/ldb_cmd_test.cc index 8e43972d4b..7175ea0d5a 100644 --- a/tools/ldb_cmd_test.cc +++ b/tools/ldb_cmd_test.cc @@ -14,6 +14,7 @@ #include "file/filename.h" #include "port/stack_trace.h" #include "rocksdb/advanced_options.h" +#include "rocksdb/comparator.h" #include "rocksdb/convenience.h" #include "rocksdb/db.h" #include "rocksdb/file_checksum.h" @@ -1207,6 +1208,50 @@ TEST_F(LdbCmdTest, RenameDbAndLoadOptions) { ASSERT_OK(DestroyDB(new_dbname, opts)); } +class MyComparator : public Comparator { + public: + int Compare(const rocksdb::Slice& a, const rocksdb::Slice& b) const override { + return a.compare(b); + } + void FindShortSuccessor(std::string* /*key*/) const override {} + void FindShortestSeparator(std::string* /*start*/, + const Slice& /*limit*/) const override {} + const char* Name() const override { return "my_comparator"; } +}; + +TEST_F(LdbCmdTest, CustomComparator) { + Env* env = TryLoadCustomOrDefaultEnv(); + MyComparator my_comparator; + Options opts; + opts.env = env; + opts.create_if_missing = true; + opts.create_missing_column_families = true; + opts.comparator = &my_comparator; + + std::string dbname = test::PerThreadDBPath(env, "ldb_cmd_test"); + DB* db = nullptr; + + std::vector cfds = {{kDefaultColumnFamilyName, opts}}; + std::vector handles; + ASSERT_OK(DestroyDB(dbname, opts)); + ASSERT_OK(DB::Open(opts, dbname, cfds, &handles, &db)); + ASSERT_OK(db->Put(WriteOptions(), "k1", "v1")); + + for (auto& h : handles) { + ASSERT_OK(db->DestroyColumnFamilyHandle(h)); + } + delete db; + + char arg1[] = "./ldb"; + std::string arg2 = "--db=" + dbname; + char arg3[] = "get"; + char arg4[] = "k1"; + char* argv[] = {arg1, const_cast(arg2.c_str()), arg3, arg4}; + + ASSERT_EQ(0, + LDBCommandRunner::RunCommand(4, argv, opts, LDBOptions(), nullptr)); +} + } // namespace ROCKSDB_NAMESPACE int main(int argc, char** argv) {