Bug fix: Reject empty string as a column family name (#14732)

Summary:
Previously, calling `DB::CreateColumnFamily(opts, "", &handle)` returned `Status::OK()` with a usable handle, but the column family was not persisted in the manifest. Any data written to the empty-named CF was silently lost on DB reopen, and `ListColumnFamilies` would not show it.

The empty string is also reserved as a sentinel meaning "no/unknown column family" in various RocksDB APIs and serialization formats (e.g. `TablePropertiesCollectorFactory::Context::kUnknownColumnFamily` and table properties), so allowing it as a real CF name is ambiguous in addition to being broken.

This change rejects an empty CF name with `Status::InvalidArgument` at the top of `DBImpl::CreateColumnFamilyImpl`, which covers the single-CF `CreateColumnFamily` API as well as both `CreateColumnFamilies` overloads (by-names and by-descriptors).

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

Test Plan: * Added `ColumnFamilyTest.EmptyNameRejected` covering all three Create entry points; verifies `IsInvalidArgument()` and that no spurious handles are returned.

Reviewed By: hx235

Differential Revision: D104753911

Pulled By: pdillinger

fbshipit-source-id: e52d792830b965484a618f4e55981eee4eb6f515

db/column_family_test.cc

@@ -734,6 +734,42 @@ TEST_P(ColumnFamilyTest, AddDrop) {
               std::vector<std::string>({"default", "four", "three"}));
 }
 
+TEST_P(ColumnFamilyTest, EmptyNameRejected) {
+  // Creating a column family with an empty-string name should be rejected with
+  // InvalidArgument. Empty strings are reserved by various RocksDB APIs and
+  // serialization formats to mean "unknown/unspecified column family" (e.g.
+  // TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, table
+  // properties).
+  //
+  // Prior behavior was silently broken: CreateColumnFamily("") would return
+  // OK and a usable handle, but the CF was not persisted in the manifest --
+  // data written to it was lost on DB reopen.
+  Open();
+  ColumnFamilyHandle* handle = nullptr;
+  Status s =
+      db_->CreateColumnFamily(column_family_options_, std::string(), &handle);
+  ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();
+  ASSERT_EQ(handle, nullptr);
+
+  // Same via the bulk-by-names API. The first valid name should still get
+  // created; the failure should come on the empty one.
+  std::vector<ColumnFamilyHandle*> handles;
+  s = db_->CreateColumnFamilies(column_family_options_,
+                                {"ok_name", "", "another"}, &handles);
+  ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();
+  // Clean up the partially-created handle ("ok_name") before the failure.
+  ASSERT_EQ(handles.size(), 1U);
+  ASSERT_OK(db_->DropColumnFamily(handles[0]));
+  ASSERT_OK(db_->DestroyColumnFamilyHandle(handles[0]));
+  handles.clear();
+
+  // Same via the bulk-by-descriptor API.
+  s = db_->CreateColumnFamilies(
+      {ColumnFamilyDescriptor("", column_family_options_)}, &handles);
+  ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();
+  ASSERT_TRUE(handles.empty());
+}
+
 TEST_P(ColumnFamilyTest, BulkAddDrop) {
   constexpr int kNumCF = 1000;
   ColumnFamilyOptions cf_options;

db/db_impl/db_impl.cc

@@ -4428,9 +4428,21 @@ Status DBImpl::CreateColumnFamilyImpl(const ReadOptions& read_options,
                                       const std::string& column_family_name,
                                       ColumnFamilyHandle** handle) {
   options_mutex_.AssertHeld();
-  Status s;
   *handle = nullptr;
 
+  // The empty string is reserved as a "no/unknown column family name" sentinel
+  // in various APIs and serialization formats (e.g.
+  // TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, table
+  // properties), so reject it to avoid ambiguity. Note: prior to this check,
+  // CreateColumnFamily("") would silently "succeed" but the CF was not
+  // persisted in the manifest, causing data loss across DB reopens.
+  if (column_family_name.empty()) {
+    return Status::InvalidArgument(
+        "Column family name cannot be the empty string");
+  }
+
+  Status s;
+
   DBOptions db_options =
       BuildDBOptions(immutable_db_options_, mutable_db_options_);
   s = ColumnFamilyData::ValidateOptions(db_options, cf_options);

unreleased_history/bug_fixes/reject_empty_cf_name.md

@@ -0,0 +1 @@
+Reject the empty string as a column family name in `DB::CreateColumnFamily` / `DB::CreateColumnFamilies`. Previously such calls returned OK and a usable handle, but the column family was not persisted in the manifest, so any data written to it was silently lost on DB reopen.