Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/server/search/doc_index.cc
Original file line number Diff line number Diff line change
Expand Up @@ -645,15 +645,16 @@ void ShardDocIndices::InitIndex(const OpArgs& op_args, std::string_view name,
});
}

bool ShardDocIndices::DropIndex(string_view name) {
unique_ptr<ShardDocIndex> ShardDocIndices::DropIndex(string_view name) {
auto it = indices_.find(name);
if (it == indices_.end())
return false;
return nullptr;

DropIndexCache(*it->second);
auto index = std::move(it->second);
indices_.erase(it);

return true;
return index;
}

void ShardDocIndices::DropAllIndices() {
Expand Down
14 changes: 12 additions & 2 deletions src/server/search/doc_index.h
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,11 @@ class ShardDocIndex {
std::string_view Get(DocId id) const;
size_t Size() const;

// Get const reference to the internal ids map
const absl::flat_hash_map<std::string, DocId>& GetDocKeysMap() const {
return ids_;
}

private:
absl::flat_hash_map<std::string, DocId> ids_;
std::vector<std::string> keys_;
Expand Down Expand Up @@ -277,6 +282,11 @@ class ShardDocIndex {
void RebuildForGroup(const OpArgs& op_args, const std::string_view& group_id,
const std::vector<std::string_view>& terms);

// Public access to key index for direct operations (e.g., when dropping index with DD)
const DocKeyIndex& key_index() const {
return key_index_;
}

private:
// Clears internal data. Traverses all matching documents and assigns ids.
void Rebuild(const OpArgs& op_args, PMR_NS::memory_resource* mr);
Expand Down Expand Up @@ -309,8 +319,8 @@ class ShardDocIndices {
void InitIndex(const OpArgs& op_args, std::string_view name,
std::shared_ptr<const DocIndex> index);

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

// Drop all indices
void DropAllIndices();
Expand Down
47 changes: 41 additions & 6 deletions src/server/search/search_family.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,7 @@ void SearchFamily::FtAlter(CmdArgList args, const CommandContext& cmd_cntx) {
// Rebuild index
// TODO: Introduce partial rebuild
auto upd_cb = [idx_name, index_info](Transaction* tx, EngineShard* es) {
es->search_indices()->DropIndex(idx_name);
(void)es->search_indices()->DropIndex(idx_name);
es->search_indices()->InitIndex(tx->GetOpArgs(es), idx_name, index_info);
return OpStatus::OK;
};
Expand All @@ -1131,14 +1131,49 @@ void SearchFamily::FtAlter(CmdArgList args, const CommandContext& cmd_cntx) {

void SearchFamily::FtDropIndex(CmdArgList args, const CommandContext& cmd_cntx) {
string_view idx_name = ArgS(args, 0);
// TODO: Handle optional DD param

// Parse optional DD (Delete Documents) parameter
bool delete_docs = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bool delete_docs = args.size() > 1 && absl::EqualsIgnoreCase(args[1], "DD")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

if (args.size() > 1) {
string_view option = ArgS(args, 1);
// Only check for DD option, ignore other arguments for compatibility
if (absl::EqualsIgnoreCase(option, "DD")) {
delete_docs = true;
}
}

atomic_uint num_deleted{0};
cmd_cntx.tx->ScheduleSingleHop([&](Transaction* t, EngineShard* es) {
if (es->search_indices()->DropIndex(idx_name))
num_deleted.fetch_add(1);

auto cb = [&](Transaction* t, EngineShard* es) {
// Drop the index and get its pointer
auto index = es->search_indices()->DropIndex(idx_name);
if (!index)
return OpStatus::OK;

num_deleted.fetch_add(1);

// If DD is set, delete all documents that were in the index
if (delete_docs) {
// Get const reference to document keys map (index will be destroyed after this scope)
const auto& doc_keys = index->key_index().GetDocKeysMap();

if (!doc_keys.empty()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for the if, you can just iterate over it right away

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

auto op_args = t->GetOpArgs(es);
auto& db_slice = op_args.GetDbSlice();

for (const auto& [key, doc_id] : doc_keys) {
auto it = db_slice.FindMutable(op_args.db_cntx, key).it;
if (IsValid(it)) {
db_slice.Del(op_args.db_cntx, it);
}
}
}
}

return OpStatus::OK;
});
};

cmd_cntx.tx->Execute(cb, true);

DCHECK(num_deleted == 0u || num_deleted == shard_set->size());
if (num_deleted == 0u)
Expand Down
76 changes: 76 additions & 0 deletions src/server/search/search_family_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3328,4 +3328,80 @@ TEST_F(SearchFamilyTest, InvalidConfigOptions) {
EXPECT_THAT(resp, IsArray());
}

TEST_F(SearchFamilyTest, DropIndexWithDD) {
// Create an index on HASH documents
Run({"FT.CREATE", "idx", "ON", "HASH", "PREFIX", "1", "doc:", "SCHEMA", "name", "TEXT"});

// Add some documents
Run({"HSET", "doc:1", "name", "Alice"});
Run({"HSET", "doc:2", "name", "Bob"});
Run({"HSET", "doc:3", "name", "Charlie"});

// Verify documents exist
auto resp = Run({"EXISTS", "doc:1", "doc:2", "doc:3"});
EXPECT_THAT(resp, IntArg(3));

// Verify index works
resp = Run({"FT.SEARCH", "idx", "*"});
EXPECT_THAT(resp, AreDocIds("doc:1", "doc:2", "doc:3"));

// Drop index WITHOUT DD - documents should remain
Run({"FT.DROPINDEX", "idx"});
resp = Run({"EXISTS", "doc:1", "doc:2", "doc:3"});
EXPECT_THAT(resp, IntArg(3));

// Create index again
Run({"FT.CREATE", "idx", "ON", "HASH", "PREFIX", "1", "doc:", "SCHEMA", "name", "TEXT"});

// Verify index works again
resp = Run({"FT.SEARCH", "idx", "*"});
EXPECT_THAT(resp, AreDocIds("doc:1", "doc:2", "doc:3"));

// Drop index WITH DD - documents should be deleted
Run({"FT.DROPINDEX", "idx", "DD"});
resp = Run({"EXISTS", "doc:1", "doc:2", "doc:3"});
EXPECT_THAT(resp, IntArg(0));
}

TEST_F(SearchFamilyTest, DropIndexWithDDJson) {
// Create an index on JSON documents
Run({"FT.CREATE", "jidx", "ON", "JSON", "PREFIX", "1", "jdoc:", "SCHEMA", "$.name", "AS", "name",
"TEXT"});

// Add some JSON documents
Run({"JSON.SET", "jdoc:1", "$", R"({"name": "Alice"})"});
Run({"JSON.SET", "jdoc:2", "$", R"({"name": "Bob"})"});
Run({"JSON.SET", "jdoc:3", "$", R"({"name": "Charlie"})"});

// Verify documents exist
auto resp = Run({"EXISTS", "jdoc:1", "jdoc:2", "jdoc:3"});
EXPECT_THAT(resp, IntArg(3));

// Verify index works
resp = Run({"FT.SEARCH", "jidx", "*"});
EXPECT_THAT(resp, AreDocIds("jdoc:1", "jdoc:2", "jdoc:3"));

// Drop index WITH DD - documents should be deleted
Run({"FT.DROPINDEX", "jidx", "DD"});
resp = Run({"EXISTS", "jdoc:1", "jdoc:2", "jdoc:3"});
EXPECT_THAT(resp, IntArg(0));
}

TEST_F(SearchFamilyTest, DropIndexWithInvalidOption) {
// Create an index
Run({"FT.CREATE", "idx", "ON", "HASH", "PREFIX", "1", "doc:", "SCHEMA", "name", "TEXT"});
Run({"HSET", "doc:1", "name", "test"});

// Drop with unrecognized option (should be ignored, index dropped but documents remain)
auto resp = Run({"FT.DROPINDEX", "idx", "INVALID"});
EXPECT_THAT(resp, "OK");

// Document should still exist
resp = Run({"EXISTS", "doc:1"});
EXPECT_THAT(resp, IntArg(1));

// Clean up
Run({"DEL", "doc:1"});
}

} // namespace dfly
Loading