From 59633e36eded94fed7d7cae76983bcd3fe5a9c04 Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Tue, 12 May 2026 16:06:58 -0700 Subject: [PATCH] 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 | 36 +++++++++++++++++++ db/db_impl/db_impl.cc | 14 +++++++- .../bug_fixes/reject_empty_cf_name.md | 1 + 3 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 unreleased_history/bug_fixes/reject_empty_cf_name.md diff --git a/db/column_family_test.cc b/db/column_family_test.cc index 7cb505179c..e3aab78abf 100644 --- a/db/column_family_test.cc +++ b/db/column_family_test.cc @@ -734,6 +734,42 @@ TEST_P(ColumnFamilyTest, AddDrop) { std::vector({"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 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; diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index cb148bc698..401ecf75f9 100644 --- a/db/db_impl/db_impl.cc +++ b/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); diff --git a/unreleased_history/bug_fixes/reject_empty_cf_name.md b/unreleased_history/bug_fixes/reject_empty_cf_name.md new file mode 100644 index 0000000000..2513e289cd --- /dev/null +++ b/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.