Partially convert Rdb_cf_options class to use (#1461)

Upstream commit ID: facebook/mysql-5.6@68638be03ecd
PS-9395: Merge percona-202404 (https://jira.percona.com/browse/PS-9395)

Summary:
- Partially convert Rdb_cf_options methods to take std::string_view parameters.
  Specifically, do not convert the ones that are passed as keys to m_name_map,
  which is a std::unordered_map<string, string>. This can be converted in C++20,
  where lookups through a different key type are supported.
- Convert Rdb_cf_manager::is_cf_name_reverse and create_cf_flags_if_needed
  methods to take std::string_view arguments, update callers.
- Convert pointer arguments for the touched methods to const reference and
  reference ones, where possible, remove redundant nullptr-checking asserts.
  Remove redundant const from pass-by-value arguments, add [[nodiscard]].
- Update ER_CF_DIFFERENT to print an std::string_view parameter.

Pull Request resolved: facebook/mysql-5.6#1461

Differential Revision: D58594610

fbshipit-source-id: 90e90875051cad3221fb5c485c787ea09a18efb8

Author: laurynas-biveinis

share/messages_to_clients.txt

@@ -10136,7 +10136,7 @@ ER_SK_POPULATE_DURING_ALTER
   eng "MyRocks failed populating secondary key during alter."
 
 ER_CF_DIFFERENT
-  eng "Column family ('%s') flag (%d) is different from an existing flag (%d). Assign a new CF flag, or do not change existing CF flag."
+  eng "Column family ('%.*s') flag (%d) is different from an existing flag (%d). Assign a new CF flag, or do not change existing CF flag."
 
 ER_RDB_TTL_UNSUPPORTED
   eng "TTL support is currently disabled when table has a hidden PK."

storage/rocksdb/ha_rocksdb.cc

@@ -8532,7 +8532,7 @@ bool ha_rocksdb::create_cfs(
         return true;
       }
 
-      if (cf_manager.create_cf_flags_if_needed(local_dict_manager,
+      if (cf_manager.create_cf_flags_if_needed(*local_dict_manager,
                                                cf_handle->GetID(), cf_name,
                                                per_part_match_found)) {
         return true;
@@ -8545,7 +8545,7 @@ bool ha_rocksdb::create_cfs(
     auto &cf = cfs[i];
 
     cf.cf_handle = cf_handle;
-    cf.is_reverse_cf = Rdb_cf_manager::is_cf_name_reverse(cf_name.c_str());
+    cf.is_reverse_cf = Rdb_cf_manager::is_cf_name_reverse(cf_name);
     cf.is_per_partition_cf = per_part_match_found;
   }
 
@@ -16876,7 +16876,7 @@ static int rocksdb_validate_update_cf_options(
 
   // Basic sanity checking and parsing the options into a map. If this fails
   // then there's no point to proceed.
-  if (!Rdb_cf_options::parse_cf_options(str, &option_map, &output)) {
+  if (!Rdb_cf_options::parse_cf_options(str, option_map, &output)) {
     my_printf_error(ER_WRONG_VALUE_FOR_VAR, "%s", MYF(0), output.str().c_str());
     return HA_EXIT_FAILURE;
   }
@@ -16929,7 +16929,7 @@ static void rocksdb_set_update_cf_options(
   Rdb_cf_options::Name_to_config_t option_map;
 
   // This should never fail, because of rocksdb_validate_update_cf_options
-  if (!Rdb_cf_options::parse_cf_options(val, &option_map)) {
+  if (!Rdb_cf_options::parse_cf_options(val, option_map)) {
     return;
   }
 

storage/rocksdb/rdb_cf_manager.cc

@@ -39,10 +39,10 @@
 namespace myrocks {
 
 /* Check if ColumnFamily name says it's a reverse-ordered CF */
-bool Rdb_cf_manager::is_cf_name_reverse(const char *const name) {
-  /* nullptr means the default CF is used.. (TODO: can the default CF be
+bool Rdb_cf_manager::is_cf_name_reverse(std::string_view name) {
+  /* Empty string means the default CF is used. (TODO: can the default CF be
    * reverse?) */
-  return (name && !strncmp(name, "rev:", 4));
+  return name.compare(0, 4, "rev:") == 0;
 }
 
 bool Rdb_cf_manager::init(
@@ -456,33 +456,33 @@ int Rdb_cf_manager::drop_cf(Rdb_ddl_manager *const ddl_manager,
 }
 
 int Rdb_cf_manager::create_cf_flags_if_needed(
-    const Rdb_dict_manager *const dict_manager, const uint32 &cf_id,
-    const std::string &cf_name, const bool is_per_partition_cf) {
+    const Rdb_dict_manager &dict_manager, uint32_t cf_id,
+    std::string_view cf_name, bool is_per_partition_cf) {
   assert(!cf_name.empty());
   uchar flags =
-      (is_cf_name_reverse(cf_name.c_str()) ? Rdb_key_def::REVERSE_CF_FLAG : 0) |
+      (is_cf_name_reverse(cf_name) ? Rdb_key_def::REVERSE_CF_FLAG : 0) |
       (is_per_partition_cf ? Rdb_key_def::PER_PARTITION_CF_FLAG : 0);
 
   uint existing_cf_flags;
-  if (dict_manager->get_cf_flags(cf_id, &existing_cf_flags)) {
+  if (dict_manager.get_cf_flags(cf_id, &existing_cf_flags)) {
     // For the purposes of comparison we'll clear the partitioning bit. The
     // intent here is to make sure that both partitioned and non-partitioned
     // tables can refer to the same CF.
     existing_cf_flags &= ~Rdb_key_def::CF_FLAGS_TO_IGNORE;
     flags &= ~Rdb_key_def::CF_FLAGS_TO_IGNORE;
 
     if (existing_cf_flags != flags) {
-      my_error(ER_CF_DIFFERENT, MYF(0), cf_name.c_str(), flags,
-               existing_cf_flags);
+      my_error(ER_CF_DIFFERENT, MYF(0), static_cast<int>(cf_name.length()),
+               cf_name.data(), flags, existing_cf_flags);
 
       return HA_EXIT_FAILURE;
     }
   } else {
-    const std::unique_ptr<rocksdb::WriteBatch> wb = dict_manager->begin();
+    const auto wb = dict_manager.begin();
     rocksdb::WriteBatch *const batch = wb.get();
 
-    dict_manager->add_cf_flags(batch, cf_id, flags);
-    dict_manager->commit(batch);
+    dict_manager.add_cf_flags(batch, cf_id, flags);
+    dict_manager.commit(batch);
   }
 
   return HA_EXIT_SUCCESS;

storage/rocksdb/rdb_cf_manager.h

@@ -19,6 +19,7 @@
 /* C++ system header files */
 #include <map>
 #include <string>
+#include <string_view>
 #include <vector>
 
 /* RocksDB header files */
@@ -61,7 +62,7 @@ class Rdb_cf_manager : public Ensure_initialized {
   Rdb_cf_manager &operator=(const Rdb_cf_manager &) = delete;
   Rdb_cf_manager() = default;
 
-  static bool is_cf_name_reverse(const char *const name);
+  [[nodiscard]] static bool is_cf_name_reverse(std::string_view name);
 
   /*
     This is called right after the DB::Open() call. The parameters describe
@@ -108,9 +109,9 @@ class Rdb_cf_manager : public Ensure_initialized {
               Rdb_dict_manager *const dict_manager, const std::string &cf_name);
 
   /* Create cf flags if it does not exist */
-  int create_cf_flags_if_needed(const Rdb_dict_manager *const dict_manager,
-                                const uint32 &cf_id, const std::string &cf_name,
-                                const bool is_per_partition_cf = false);
+  [[nodiscard]] int create_cf_flags_if_needed(
+      const Rdb_dict_manager &dict_manager, uint32_t cf_id,
+      std::string_view cf_name, bool is_per_partition_cf = false);
 
   /* return true when success */
   bool get_cf_options(const std::string &cf_name,