Skip to content

Commit 0931f96

Browse files
committed
fix(search): drop index DD option added
1 parent db7c11c commit 0931f96

File tree

4 files changed

+144
-3
lines changed

4 files changed

+144
-3
lines changed

src/server/search/doc_index.cc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,19 @@ size_t ShardDocIndex::DocKeyIndex::Size() const {
242242
return ids_.size();
243243
}
244244

245+
std::vector<std::string> ShardDocIndex::DocKeyIndex::GetAllKeys() const {
246+
std::vector<std::string> keys;
247+
keys.reserve(ids_.size());
248+
249+
for (const auto& [key, id] : ids_) {
250+
if (!key.empty()) {
251+
keys.push_back(key);
252+
}
253+
}
254+
255+
return keys;
256+
}
257+
245258
uint8_t DocIndex::GetObjCode() const {
246259
return type == JSON ? OBJ_JSON : OBJ_HASH;
247260
}
@@ -607,6 +620,10 @@ DocIndexInfo ShardDocIndex::GetInfo() const {
607620
return {*base_, key_index_.Size()};
608621
}
609622

623+
std::vector<std::string> ShardDocIndex::GetAllKeys() const {
624+
return key_index_.GetAllKeys();
625+
}
626+
610627
io::Result<StringVec, ErrorReply> ShardDocIndex::GetTagVals(string_view field) const {
611628
search::BaseIndex* base_index = indices_->GetIndex(field);
612629
if (base_index == nullptr) {

src/server/search/doc_index.h

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

227+
// Get all keys in the index
228+
std::vector<std::string> GetAllKeys() const;
229+
227230
private:
228231
absl::flat_hash_map<std::string, DocId> ids_;
229232
std::vector<std::string> keys_;
@@ -262,6 +265,9 @@ class ShardDocIndex {
262265

263266
DocIndexInfo GetInfo() const;
264267

268+
// Get all document keys in this index
269+
std::vector<std::string> GetAllKeys() const;
270+
265271
io::Result<StringVec, facade::ErrorReply> GetTagVals(std::string_view field) const;
266272

267273
// Get synonym manager for this shard

src/server/search/search_family.cc

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,18 +1131,60 @@ 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
11351134

1135+
// Parse optional DD (Delete Documents) parameter
1136+
bool delete_docs = false;
1137+
if (args.size() > 1) {
1138+
string_view option = ArgS(args, 1);
1139+
if (absl::EqualsIgnoreCase(option, "DD")) {
1140+
delete_docs = true;
1141+
} else {
1142+
return cmd_cntx.rb->SendError("Unknown argument");
1143+
}
1144+
}
1145+
1146+
// Collect document keys (if DD is set), drop index, and delete documents in single transaction
1147+
vector<vector<string>> shard_doc_keys(delete_docs ? shard_set->size() : 0);
11361148
atomic_uint num_deleted{0};
1137-
cmd_cntx.tx->ScheduleSingleHop([&](Transaction* t, EngineShard* es) {
1149+
1150+
auto cb = [&](Transaction* t, EngineShard* es) {
1151+
// Step 1: If DD is set, collect all document keys before dropping the index
1152+
if (delete_docs) {
1153+
auto* index = es->search_indices()->GetIndex(idx_name);
1154+
if (index) {
1155+
shard_doc_keys[es->shard_id()] = index->GetAllKeys();
1156+
}
1157+
}
1158+
1159+
// Step 2: Drop the index
11381160
if (es->search_indices()->DropIndex(idx_name))
11391161
num_deleted.fetch_add(1);
1162+
1163+
// Step 3: If DD is set, delete all documents that were in the index
1164+
if (delete_docs) {
1165+
const auto& keys = shard_doc_keys[es->shard_id()];
1166+
if (!keys.empty()) {
1167+
auto op_args = t->GetOpArgs(es);
1168+
auto& db_slice = op_args.GetDbSlice();
1169+
1170+
for (const auto& key : keys) {
1171+
auto it = db_slice.FindMutable(op_args.db_cntx, key).it;
1172+
if (IsValid(it)) {
1173+
db_slice.Del(op_args.db_cntx, it);
1174+
}
1175+
}
1176+
}
1177+
}
1178+
11401179
return OpStatus::OK;
1141-
});
1180+
};
1181+
1182+
cmd_cntx.tx->Execute(cb, true);
11421183

11431184
DCHECK(num_deleted == 0u || num_deleted == shard_set->size());
11441185
if (num_deleted == 0u)
11451186
return cmd_cntx.rb->SendError("-Unknown Index name");
1187+
11461188
return cmd_cntx.rb->SendOk();
11471189
}
11481190

src/server/search/search_family_test.cc

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3328,4 +3328,80 @@ TEST_F(SearchFamilyTest, InvalidConfigOptions) {
33283328
EXPECT_THAT(resp, IsArray());
33293329
}
33303330

3331+
TEST_F(SearchFamilyTest, DropIndexWithDD) {
3332+
// Create an index on HASH documents
3333+
Run({"FT.CREATE", "idx", "ON", "HASH", "PREFIX", "1", "doc:", "SCHEMA", "name", "TEXT"});
3334+
3335+
// Add some documents
3336+
Run({"HSET", "doc:1", "name", "Alice"});
3337+
Run({"HSET", "doc:2", "name", "Bob"});
3338+
Run({"HSET", "doc:3", "name", "Charlie"});
3339+
3340+
// Verify documents exist
3341+
auto resp = Run({"EXISTS", "doc:1", "doc:2", "doc:3"});
3342+
EXPECT_THAT(resp, IntArg(3));
3343+
3344+
// Verify index works
3345+
resp = Run({"FT.SEARCH", "idx", "*"});
3346+
EXPECT_THAT(resp, AreDocIds("doc:1", "doc:2", "doc:3"));
3347+
3348+
// Drop index WITHOUT DD - documents should remain
3349+
Run({"FT.DROPINDEX", "idx"});
3350+
resp = Run({"EXISTS", "doc:1", "doc:2", "doc:3"});
3351+
EXPECT_THAT(resp, IntArg(3));
3352+
3353+
// Create index again
3354+
Run({"FT.CREATE", "idx", "ON", "HASH", "PREFIX", "1", "doc:", "SCHEMA", "name", "TEXT"});
3355+
3356+
// Verify index works again
3357+
resp = Run({"FT.SEARCH", "idx", "*"});
3358+
EXPECT_THAT(resp, AreDocIds("doc:1", "doc:2", "doc:3"));
3359+
3360+
// Drop index WITH DD - documents should be deleted
3361+
Run({"FT.DROPINDEX", "idx", "DD"});
3362+
resp = Run({"EXISTS", "doc:1", "doc:2", "doc:3"});
3363+
EXPECT_THAT(resp, IntArg(0));
3364+
}
3365+
3366+
TEST_F(SearchFamilyTest, DropIndexWithDDJson) {
3367+
// Create an index on JSON documents
3368+
Run({"FT.CREATE", "jidx", "ON", "JSON", "PREFIX", "1", "jdoc:", "SCHEMA", "$.name", "AS", "name",
3369+
"TEXT"});
3370+
3371+
// Add some JSON documents
3372+
Run({"JSON.SET", "jdoc:1", "$", R"({"name": "Alice"})"});
3373+
Run({"JSON.SET", "jdoc:2", "$", R"({"name": "Bob"})"});
3374+
Run({"JSON.SET", "jdoc:3", "$", R"({"name": "Charlie"})"});
3375+
3376+
// Verify documents exist
3377+
auto resp = Run({"EXISTS", "jdoc:1", "jdoc:2", "jdoc:3"});
3378+
EXPECT_THAT(resp, IntArg(3));
3379+
3380+
// Verify index works
3381+
resp = Run({"FT.SEARCH", "jidx", "*"});
3382+
EXPECT_THAT(resp, AreDocIds("jdoc:1", "jdoc:2", "jdoc:3"));
3383+
3384+
// Drop index WITH DD - documents should be deleted
3385+
Run({"FT.DROPINDEX", "jidx", "DD"});
3386+
resp = Run({"EXISTS", "jdoc:1", "jdoc:2", "jdoc:3"});
3387+
EXPECT_THAT(resp, IntArg(0));
3388+
}
3389+
3390+
TEST_F(SearchFamilyTest, DropIndexWithInvalidOption) {
3391+
// Create an index
3392+
Run({"FT.CREATE", "idx", "ON", "HASH", "PREFIX", "1", "doc:", "SCHEMA", "name", "TEXT"});
3393+
3394+
// Try to drop with invalid option
3395+
auto resp = Run({"FT.DROPINDEX", "idx", "INVALID"});
3396+
EXPECT_THAT(resp, ErrArg("Unknown argument"));
3397+
3398+
// Index should still exist - verify it can be searched
3399+
Run({"HSET", "doc:1", "name", "test"});
3400+
resp = Run({"FT.SEARCH", "idx", "*"});
3401+
EXPECT_THAT(resp, AreDocIds("doc:1"));
3402+
3403+
// Clean up
3404+
Run({"FT.DROPINDEX", "idx", "DD"});
3405+
}
3406+
33313407
} // namespace dfly

0 commit comments

Comments
 (0)