tursom/memgraph
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Apply suggestions from code review

Browse Source
This commit is contained in:
Ante Pušić 2024-02-27 16:25:34 +01:00
parent 7f5a77d2dd
commit c79655e397
13 changed files with 52 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
include/_mgp.hpp
View File

@ -331,7 +331,7 @@ inline bool graph_has_text_index(mgp_graph *graph, const char *index_name) {
}
inline mgp_map *graph_search_text_index(mgp_graph *graph, const char *index_name, const char *search_query,
int search_mode, mgp_memory *memory) {
text_search_mode search_mode, mgp_memory *memory) {
return MgInvoke<mgp_map *>(mgp_graph_search_text_index, graph, index_name, search_query, search_mode, memory);
}

10
include/mg_procedure.h
View File

@ -895,12 +895,20 @@ enum mgp_error mgp_graph_get_vertex_by_id(struct mgp_graph *g, struct mgp_vertex
/// The current implementation always returns without errors.
enum mgp_error mgp_graph_has_text_index(struct mgp_graph *graph, const char *index_name, int *result);
/// Available modes of searching text indices.
MGP_ENUM_CLASS text_search_mode{
SPECIFIED_PROPERTIES,
REGEX,
ALL_PROPERTIES,
};
/// Search the named text index for the given query. The result is a list of the vertices whose text properties match
/// the given query.
/// Return mgp_error::MGP_ERROR_UNABLE_TO_ALLOCATE if there’s an allocation error while constructing the results map.
/// Return mgp_error::MGP_ERROR_KEY_ALREADY_EXISTS if the same key is being created in the results map more than once.
enum mgp_error mgp_graph_search_text_index(struct mgp_graph *graph, const char *index_name, const char *search_query,
int search_mode, struct mgp_memory *memory, struct mgp_map **result);
enum text_search_mode search_mode, struct mgp_memory *memory,
struct mgp_map **result);
/// Search the named text index for the given query and aggregate over the search results.
/// Return mgp_error::MGP_ERROR_UNABLE_TO_ALLOCATE if there’s an allocation error while constructing the results map.

10
include/mgp.hpp
View File

@ -4349,12 +4349,6 @@ inline List ListAllLabelPropertyIndices(mgp_graph *memgraph_graph) {
return List(label_property_indices);
}
enum class TextSearchMode : uint8_t {
SPECIFIED_PROPERTIES = 0,
REGEX = 1,
ALL_PROPERTIES = 2,
};
namespace {
constexpr std::string_view kErrorMsgKey = "error_msg";
constexpr std::string_view kSearchResultsKey = "search_results";
@ -4362,9 +4356,9 @@ constexpr std::string_view kAggregationResultsKey = "aggregation_results";
} // namespace
inline List SearchTextIndex(mgp_graph *memgraph_graph, std::string_view index_name, std::string_view search_query,
TextSearchMode search_mode) {
text_search_mode search_mode) {
auto results_or_error = Map(mgp::MemHandlerCallback(graph_search_text_index, memgraph_graph, index_name.data(),
search_query.data(), static_cast<int>(search_mode)));
search_query.data(), search_mode));
if (!results_or_error.KeyExists(kErrorMsgKey) || !results_or_error.KeyExists(kSearchResultsKey)) {
throw TextSearchException{"Incomplete text index search results!"};
}

4
query_modules/CMakeLists.txt
View File

@ -6,6 +6,8 @@ project(memgraph_query_modules)
disallow_in_source_build()
find_package(fmt REQUIRED)
# Everything that is installed here, should be under the "query_modules" component.
set(CMAKE_INSTALL_DEFAULT_COMPONENT_NAME "query_modules")
string(TOLOWER ${CMAKE_BUILD_TYPE} lower_build_type)
@ -61,7 +63,7 @@ install(FILES schema.cpp DESTINATION lib/memgraph/query_modules/src)
add_library(text SHARED text_search_module.cpp)
target_include_directories(text PRIVATE ${CMAKE_SOURCE_DIR}/include)
target_compile_options(text PRIVATE -Wall)
target_link_libraries(text PRIVATE -static-libgcc -static-libstdc++)
target_link_libraries(text PRIVATE -static-libgcc -static-libstdc++ fmt::fmt)
# Strip C++ example in release build.
if (lower_build_type STREQUAL "release")
add_custom_command(TARGET text POST_BUILD

11
query_modules/text_search_module.cpp
View File

@ -26,7 +26,7 @@ constexpr std::string_view kParameterSearchQuery = "search_query";
constexpr std::string_view kParameterAggregationQuery = "aggregation_query";
constexpr std::string_view kReturnNode = "node";
constexpr std::string_view kReturnAggregation = "aggregation";
const std::string kSearchAllPrefix = "all:";
const std::string kSearchAllPrefix = "all";
void Search(mgp_list *args, mgp_graph *memgraph_graph, mgp_result *result, mgp_memory *memory);
void RegexSearch(mgp_list *args, mgp_graph *memgraph_graph, mgp_result *result, mgp_memory *memory);
@ -43,7 +43,7 @@ void TextSearch::Search(mgp_list *args, mgp_graph *memgraph_graph, mgp_result *r
const auto *index_name = arguments[0].ValueString().data();
const auto *search_query = arguments[1].ValueString().data();
for (const auto &node :
mgp::SearchTextIndex(memgraph_graph, index_name, search_query, mgp::TextSearchMode::SPECIFIED_PROPERTIES)) {
mgp::SearchTextIndex(memgraph_graph, index_name, search_query, text_search_mode::SPECIFIED_PROPERTIES)) {
auto record = record_factory.NewRecord();
record.Insert(TextSearch::kReturnNode.data(), node.ValueNode());
}
@ -60,8 +60,7 @@ void TextSearch::RegexSearch(mgp_list *args, mgp_graph *memgraph_graph, mgp_resu
try {
const auto *index_name = arguments[0].ValueString().data();
const auto *search_query = arguments[1].ValueString().data();
for (const auto &node :
mgp::SearchTextIndex(memgraph_graph, index_name, search_query, mgp::TextSearchMode::REGEX)) {
for (const auto &node : mgp::SearchTextIndex(memgraph_graph, index_name, search_query, text_search_mode::REGEX)) {
auto record = record_factory.NewRecord();
record.Insert(TextSearch::kReturnNode.data(), node.ValueNode());
}
@ -78,9 +77,9 @@ void TextSearch::SearchAllProperties(mgp_list *args, mgp_graph *memgraph_graph,
try {
const auto *index_name = arguments[0].ValueString().data();
const auto search_query = kSearchAllPrefix + std::string(arguments[1].ValueString());
const auto *search_query = fmt::format("{}:{}", kSearchAllPrefix, arguments[1].ValueString()).data();
for (const auto &node :
mgp::SearchTextIndex(memgraph_graph, index_name, search_query, mgp::TextSearchMode::ALL_PROPERTIES)) {
mgp::SearchTextIndex(memgraph_graph, index_name, search_query, text_search_mode::ALL_PROPERTIES)) {
auto record = record_factory.NewRecord();
record.Insert(TextSearch::kReturnNode.data(), node.ValueNode());
}

4
src/query/db_accessor.hpp
View File

@ -581,8 +581,8 @@ class DbAccessor final {
}
std::vector<storage::Gid> TextIndexSearch(const std::string &index_name, const std::string &search_query,
uint8_t search_mode) const {
return accessor_->TextIndexSearch(index_name, search_query, static_cast<storage::TextSearchMode>(search_mode));
text_search_mode search_mode) const {
return accessor_->TextIndexSearch(index_name, search_query, search_mode);
}
std::string TextIndexAggregate(const std::string &index_name, const std::string &search_query,

2
src/query/procedure/mg_procedure_impl.cpp
View File

@ -3451,7 +3451,7 @@ void WrapTextIndexAggregation(mgp_memory *memory, mgp_map **result, const std::s
}
mgp_error mgp_graph_search_text_index(mgp_graph *graph, const char *index_name, const char *search_query,
int search_mode, mgp_memory *memory, mgp_map **result) {
text_search_mode search_mode, mgp_memory *memory, mgp_map **result) {
return WrapExceptions([graph, memory, index_name, search_query, search_mode, result]() {
std::vector<memgraph::storage::Gid> search_results;
std::string error_msg;

1
src/storage/v2/CMakeLists.txt
View File

@ -43,4 +43,5 @@ add_library(mg-storage-v2 STATIC
inmemory/replication/recovery.cpp
)
target_include_directories(mg-storage-v2 PUBLIC ${CMAKE_SOURCE_DIR}/include)
target_link_libraries(mg-storage-v2 mg::replication Threads::Threads mg-utils mg-flags gflags absl::flat_hash_map mg-rpc mg-slk mg-events mg-memory mgcxx_text_search tantivy_text_search)

6
src/storage/v2/durability/durability.cpp
View File

@ -149,7 +149,7 @@ void RecoverConstraints(const RecoveredIndicesAndConstraints::ConstraintsMetadat
void RecoverIndicesAndStats(const RecoveredIndicesAndConstraints::IndicesMetadata &indices_metadata, Indices *indices,
utils::SkipList<Vertex> *vertices, NameIdMapper *name_id_mapper,
const std::optional<ParallelizedSchemaCreationInfo> &parallel_exec_info,
std::optional<std::filesystem::path> storage_dir) {
const std::optional<std::filesystem::path> &storage_dir) {
spdlog::info("Recreating indices from metadata.");
// Recover label indices.
@ -204,6 +204,10 @@ void RecoverIndicesAndStats(const RecoveredIndicesAndConstraints::IndicesMetadat
auto &mem_text_index = indices->text_index_;
for (const auto &[index_name, label] : indices_metadata.text_indices) {
try {
if (!storage_dir.has_value()) {
throw RecoveryFailure("There must exist a storage directory in order to recover text indices!");
}
mem_text_index.RecoverIndex(storage_dir.value(), index_name, label, vertices->access(), name_id_mapper);
} catch (...) {
throw RecoveryFailure("The text index must be created here!");

2
src/storage/v2/durability/durability.hpp
View File

@ -103,7 +103,7 @@ std::optional<std::vector<WalDurabilityInfo>> GetWalFiles(const std::filesystem:
void RecoverIndicesAndStats(const RecoveredIndicesAndConstraints::IndicesMetadata &indices_metadata, Indices *indices,
utils::SkipList<Vertex> *vertices, NameIdMapper *name_id_mapper,
const std::optional<ParallelizedSchemaCreationInfo> &parallel_exec_info = std::nullopt,
std::optional<std::filesystem::path> storage_dir = std::nullopt);
const std::optional<std::filesystem::path> &storage_dir = std::nullopt);
// Helper function used to recover all discovered constraints. The
// constraints must be recovered after the data recovery is done

21
src/storage/v2/indices/text_index.cpp
View File

@ -167,8 +167,9 @@ void TextIndex::CommitLoadedNodes(mgcxx::text_search::Context &index_context) {
}
}
void TextIndex::AddNode(Vertex *vertex_after_update, NameIdMapper *name_id_mapper,
std::optional<std::vector<mgcxx::text_search::Context *>> maybe_applicable_text_indices) {
void TextIndex::AddNode(
Vertex *vertex_after_update, NameIdMapper *name_id_mapper,
const std::optional<std::vector<mgcxx::text_search::Context *>> &maybe_applicable_text_indices) {
if (!flags::AreExperimentsEnabled(flags::Experiments::TEXT_SEARCH)) {
throw query::TextSearchDisabledException();
}
@ -201,8 +202,9 @@ void TextIndex::UpdateNode(Vertex *vertex_after_update, NameIdMapper *name_id_ma
AddNode(vertex_after_update, name_id_mapper, applicable_text_indices);
}
void TextIndex::RemoveNode(Vertex *vertex_after_update,
std::optional<std::vector<mgcxx::text_search::Context *>> applicable_text_indices) {
void TextIndex::RemoveNode(
Vertex *vertex_after_update,
const std::optional<std::vector<mgcxx::text_search::Context *>> &maybe_applicable_text_indices) {
if (!flags::AreExperimentsEnabled(flags::Experiments::TEXT_SEARCH)) {
throw query::TextSearchDisabledException();
}
@ -210,7 +212,8 @@ void TextIndex::RemoveNode(Vertex *vertex_after_update,
auto search_node_to_be_deleted =
mgcxx::text_search::SearchInput{.search_query = fmt::format("metadata.gid:{}", vertex_after_update->gid.AsInt())};
for (auto *index_context : applicable_text_indices.value_or(GetApplicableTextIndices(vertex_after_update->labels))) {
for (auto *index_context :
maybe_applicable_text_indices.value_or(GetApplicableTextIndices(vertex_after_update->labels))) {
try {
mgcxx::text_search::delete_document(*index_context, search_node_to_be_deleted, kDoSkipCommit);
} catch (const std::exception &e) {
@ -332,7 +335,7 @@ mgcxx::text_search::SearchOutput TextIndex::SearchAllProperties(const std::strin
}
std::vector<Gid> TextIndex::Search(const std::string &index_name, const std::string &search_query,
TextSearchMode search_mode) {
text_search_mode search_mode) {
if (!flags::AreExperimentsEnabled(flags::Experiments::TEXT_SEARCH)) {
throw query::TextSearchDisabledException();
}
@ -343,13 +346,13 @@ std::vector<Gid> TextIndex::Search(const std::string &index_name, const std::str
mgcxx::text_search::SearchOutput search_results;
switch (search_mode) {
case TextSearchMode::SPECIFIED_PROPERTIES:
case text_search_mode::SPECIFIED_PROPERTIES:
search_results = SearchGivenProperties(index_name, search_query);
break;
case TextSearchMode::REGEX:
case text_search_mode::REGEX:
search_results = RegexSearch(index_name, search_query);
break;
case TextSearchMode::ALL_PROPERTIES:
case text_search_mode::ALL_PROPERTIES:
search_results = SearchAllProperties(index_name, search_query);
break;
default:

19
src/storage/v2/indices/text_index.hpp
View File

@ -12,6 +12,7 @@
#pragma once
#include <json/json.hpp>
#include "mg_procedure.h"
#include "storage/v2/id_types.hpp"
#include "storage/v2/name_id_mapper.hpp"
#include "storage/v2/vertex.hpp"
@ -27,12 +28,6 @@ struct TextIndexData {
LabelId scope_;
};
enum class TextSearchMode : uint8_t {
SPECIFIED_PROPERTIES = 0,
REGEX = 1,
ALL_PROPERTIES = 2,
};
class TextIndex {
private:
static constexpr bool kDoSkipCommit = true;
@ -75,13 +70,15 @@ class TextIndex {
std::map<std::string, TextIndexData> index_;
std::map<LabelId, std::string> label_to_index_;
void AddNode(Vertex *vertex, NameIdMapper *name_id_mapper,
std::optional<std::vector<mgcxx::text_search::Context *>> applicable_text_indices = std::nullopt);
void AddNode(
Vertex *vertex, NameIdMapper *name_id_mapper,
const std::optional<std::vector<mgcxx::text_search::Context *>> &maybe_applicable_text_indices = std::nullopt);
void UpdateNode(Vertex *vertex, NameIdMapper *name_id_mapper, const std::vector<LabelId> &removed_labels = {});
void RemoveNode(Vertex *vertex,
std::optional<std::vector<mgcxx::text_search::Context *>> applicable_text_indices = std::nullopt);
void RemoveNode(
Vertex *vertex,
const std::optional<std::vector<mgcxx::text_search::Context *>> &maybe_applicable_text_indices = std::nullopt);
void CreateIndex(const std::filesystem::path &storage_dir, const std::string &index_name, LabelId label,
memgraph::query::DbAccessor *db);
@ -93,7 +90,7 @@ class TextIndex {
bool IndexExists(const std::string &index_name) const;
std::vector<Gid> Search(const std::string &index_name, const std::string &search_query, TextSearchMode search_mode);
std::vector<Gid> Search(const std::string &index_name, const std::string &search_query, text_search_mode search_mode);
std::string Aggregate(const std::string &index_name, const std::string &search_query,
const std::string &aggregation_query);

3
src/storage/v2/storage.hpp
View File

@ -20,6 +20,7 @@
#include "io/network/endpoint.hpp"
#include "kvstore/kvstore.hpp"
#include "mg_procedure.h"
#include "query/exceptions.hpp"
#include "replication/config.hpp"
#include "replication/replication_server.hpp"
@ -241,7 +242,7 @@ class Storage {
}
std::vector<Gid> TextIndexSearch(const std::string &index_name, const std::string &search_query,
TextSearchMode search_mode) const {
text_search_mode search_mode) const {
return storage_->indices_.text_index_.Search(index_name, search_query, search_mode);
}