Skip to content

Commit ccdcfc3

Browse files
authored
feat(search): Add DD option support for FT.DROPINDEX command (#5885)
Fixed: #5513
1 parent c430f5a commit ccdcfc3

File tree

4 files changed

+124
-11
lines changed

4 files changed

+124
-11
lines changed

src/server/search/doc_index.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -645,15 +645,16 @@ void ShardDocIndices::InitIndex(const OpArgs& op_args, std::string_view name,
645645
});
646646
}
647647

648-
bool ShardDocIndices::DropIndex(string_view name) {
648+
unique_ptr<ShardDocIndex> ShardDocIndices::DropIndex(string_view name) {
649649
auto it = indices_.find(name);
650650
if (it == indices_.end())
651-
return false;
651+
return nullptr;
652652

653653
DropIndexCache(*it->second);
654+
auto index = std::move(it->second);
654655
indices_.erase(it);
655656

656-
return true;
657+
return index;
657658
}
658659

659660
void ShardDocIndices::DropAllIndices() {

src/server/search/doc_index.h

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,11 @@ class ShardDocIndex {
224224
std::string_view Get(DocId id) const;
225225
size_t Size() const;
226226

227+
// Get const reference to the internal ids map
228+
const absl::flat_hash_map<std::string, DocId>& GetDocKeysMap() const {
229+
return ids_;
230+
}
231+
227232
private:
228233
absl::flat_hash_map<std::string, DocId> ids_;
229234
std::vector<std::string> keys_;
@@ -277,6 +282,11 @@ class ShardDocIndex {
277282
void RebuildForGroup(const OpArgs& op_args, const std::string_view& group_id,
278283
const std::vector<std::string_view>& terms);
279284

285+
// Public access to key index for direct operations (e.g., when dropping index with DD)
286+
const DocKeyIndex& key_index() const {
287+
return key_index_;
288+
}
289+
280290
private:
281291
// Clears internal data. Traverses all matching documents and assigns ids.
282292
void Rebuild(const OpArgs& op_args, PMR_NS::memory_resource* mr);
@@ -309,8 +319,8 @@ class ShardDocIndices {
309319
void InitIndex(const OpArgs& op_args, std::string_view name,
310320
std::shared_ptr<const DocIndex> index);
311321

312-
// Drop index, return true if it existed and was dropped
313-
bool DropIndex(std::string_view name);
322+
// Drop index, return the dropped index if it existed or nullptr otherwise
323+
std::unique_ptr<ShardDocIndex> DropIndex(std::string_view name);
314324

315325
// Drop all indices
316326
void DropAllIndices();

src/server/search/search_family.cc

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,7 +1120,7 @@ void SearchFamily::FtAlter(CmdArgList args, const CommandContext& cmd_cntx) {
11201120
// Rebuild index
11211121
// TODO: Introduce partial rebuild
11221122
auto upd_cb = [idx_name, index_info](Transaction* tx, EngineShard* es) {
1123-
es->search_indices()->DropIndex(idx_name);
1123+
(void)es->search_indices()->DropIndex(idx_name);
11241124
es->search_indices()->InitIndex(tx->GetOpArgs(es), idx_name, index_info);
11251125
return OpStatus::OK;
11261126
};
@@ -1131,14 +1131,40 @@ void SearchFamily::FtAlter(CmdArgList args, const CommandContext& cmd_cntx) {
11311131

11321132
void SearchFamily::FtDropIndex(CmdArgList args, const CommandContext& cmd_cntx) {
11331133
string_view idx_name = ArgS(args, 0);
1134-
// TODO: Handle optional DD param
1134+
1135+
// Parse optional DD (Delete Documents) parameter
1136+
bool delete_docs = args.size() > 1 && absl::EqualsIgnoreCase(args[1], "DD");
11351137

11361138
atomic_uint num_deleted{0};
1137-
cmd_cntx.tx->ScheduleSingleHop([&](Transaction* t, EngineShard* es) {
1138-
if (es->search_indices()->DropIndex(idx_name))
1139-
num_deleted.fetch_add(1);
1139+
1140+
auto cb = [&](Transaction* t, EngineShard* es) {
1141+
// Drop the index and get its pointer
1142+
auto index = es->search_indices()->DropIndex(idx_name);
1143+
if (!index)
1144+
return OpStatus::OK;
1145+
1146+
num_deleted.fetch_add(1);
1147+
1148+
// If DD is set, delete all documents that were in the index
1149+
if (delete_docs) {
1150+
// Get const reference to document keys map (index will be destroyed after this scope)
1151+
const auto& doc_keys = index->key_index().GetDocKeysMap();
1152+
1153+
auto op_args = t->GetOpArgs(es);
1154+
auto& db_slice = op_args.GetDbSlice();
1155+
1156+
for (const auto& [key, doc_id] : doc_keys) {
1157+
auto it = db_slice.FindMutable(op_args.db_cntx, key).it;
1158+
if (IsValid(it)) {
1159+
db_slice.Del(op_args.db_cntx, it);
1160+
}
1161+
}
1162+
}
1163+
11401164
return OpStatus::OK;
1141-
});
1165+
};
1166+
1167+
cmd_cntx.tx->Execute(cb, true);
11421168

11431169
DCHECK(num_deleted == 0u || num_deleted == shard_set->size());
11441170
if (num_deleted == 0u)

src/server/search/search_family_test.cc

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3364,4 +3364,80 @@ TEST_F(SearchFamilyTest, InvalidConfigOptions) {
33643364
EXPECT_THAT(resp, IsArray());
33653365
}
33663366

3367+
TEST_F(SearchFamilyTest, DropIndexWithDD) {
3368+
// Create an index on HASH documents
3369+
Run({"FT.CREATE", "idx", "ON", "HASH", "PREFIX", "1", "doc:", "SCHEMA", "name", "TEXT"});
3370+
3371+
// Add some documents
3372+
Run({"HSET", "doc:1", "name", "Alice"});
3373+
Run({"HSET", "doc:2", "name", "Bob"});
3374+
Run({"HSET", "doc:3", "name", "Charlie"});
3375+
3376+
// Verify documents exist
3377+
auto resp = Run({"EXISTS", "doc:1", "doc:2", "doc:3"});
3378+
EXPECT_THAT(resp, IntArg(3));
3379+
3380+
// Verify index works
3381+
resp = Run({"FT.SEARCH", "idx", "*"});
3382+
EXPECT_THAT(resp, AreDocIds("doc:1", "doc:2", "doc:3"));
3383+
3384+
// Drop index WITHOUT DD - documents should remain
3385+
Run({"FT.DROPINDEX", "idx"});
3386+
resp = Run({"EXISTS", "doc:1", "doc:2", "doc:3"});
3387+
EXPECT_THAT(resp, IntArg(3));
3388+
3389+
// Create index again
3390+
Run({"FT.CREATE", "idx", "ON", "HASH", "PREFIX", "1", "doc:", "SCHEMA", "name", "TEXT"});
3391+
3392+
// Verify index works again
3393+
resp = Run({"FT.SEARCH", "idx", "*"});
3394+
EXPECT_THAT(resp, AreDocIds("doc:1", "doc:2", "doc:3"));
3395+
3396+
// Drop index WITH DD - documents should be deleted
3397+
Run({"FT.DROPINDEX", "idx", "DD"});
3398+
resp = Run({"EXISTS", "doc:1", "doc:2", "doc:3"});
3399+
EXPECT_THAT(resp, IntArg(0));
3400+
}
3401+
3402+
TEST_F(SearchFamilyTest, DropIndexWithDDJson) {
3403+
// Create an index on JSON documents
3404+
Run({"FT.CREATE", "jidx", "ON", "JSON", "PREFIX", "1", "jdoc:", "SCHEMA", "$.name", "AS", "name",
3405+
"TEXT"});
3406+
3407+
// Add some JSON documents
3408+
Run({"JSON.SET", "jdoc:1", "$", R"({"name": "Alice"})"});
3409+
Run({"JSON.SET", "jdoc:2", "$", R"({"name": "Bob"})"});
3410+
Run({"JSON.SET", "jdoc:3", "$", R"({"name": "Charlie"})"});
3411+
3412+
// Verify documents exist
3413+
auto resp = Run({"EXISTS", "jdoc:1", "jdoc:2", "jdoc:3"});
3414+
EXPECT_THAT(resp, IntArg(3));
3415+
3416+
// Verify index works
3417+
resp = Run({"FT.SEARCH", "jidx", "*"});
3418+
EXPECT_THAT(resp, AreDocIds("jdoc:1", "jdoc:2", "jdoc:3"));
3419+
3420+
// Drop index WITH DD - documents should be deleted
3421+
Run({"FT.DROPINDEX", "jidx", "DD"});
3422+
resp = Run({"EXISTS", "jdoc:1", "jdoc:2", "jdoc:3"});
3423+
EXPECT_THAT(resp, IntArg(0));
3424+
}
3425+
3426+
TEST_F(SearchFamilyTest, DropIndexWithInvalidOption) {
3427+
// Create an index
3428+
Run({"FT.CREATE", "idx", "ON", "HASH", "PREFIX", "1", "doc:", "SCHEMA", "name", "TEXT"});
3429+
Run({"HSET", "doc:1", "name", "test"});
3430+
3431+
// Drop with unrecognized option (should be ignored, index dropped but documents remain)
3432+
auto resp = Run({"FT.DROPINDEX", "idx", "INVALID"});
3433+
EXPECT_THAT(resp, "OK");
3434+
3435+
// Document should still exist
3436+
resp = Run({"EXISTS", "doc:1"});
3437+
EXPECT_THAT(resp, IntArg(1));
3438+
3439+
// Clean up
3440+
Run({"DEL", "doc:1"});
3441+
}
3442+
33673443
} // namespace dfly

0 commit comments

Comments
 (0)