From fa8a9c57a9a144f72d396e3e5e29cdf92558474c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ante=20Pu=C5=A1i=C4=87?= Date: Wed, 28 Feb 2024 15:08:50 +0100 Subject: [PATCH] Fix text search/aggregation result handling --- include/mg_procedure.h | 10 +++- include/mgp.hpp | 39 +++++++------ src/query/procedure/mg_procedure_impl.cpp | 69 ++++++++++++----------- 3 files changed, 67 insertions(+), 51 deletions(-) diff --git a/include/mg_procedure.h b/include/mg_procedure.h index faaf34f16..117dc66ab 100644 --- a/include/mg_procedure.h +++ b/include/mg_procedure.h @@ -902,15 +902,19 @@ MGP_ENUM_CLASS text_search_mode{ 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. +/// Search the named text index for the given query. The result is a map with the "search_results" and "error_msg" keys. +/// The "search_results" key contains the vertices whose text-indexed properties match the given query. +/// In case of a Tantivy error, the "search_results" key is absent, and "error_msg" contains the error message. /// 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, 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. +/// Aggregate over the results of a search over the named text index. The result is a map with the "aggregation_results" +/// and "error_msg" keys. +/// The "aggregation_results" key contains the vertices whose text-indexed properties match the given query. +/// In case of a Tantivy error, the "aggregation_results" key is absent, and "error_msg" contains the error message. /// 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_aggregate_over_text_index(struct mgp_graph *graph, const char *index_name, diff --git a/include/mgp.hpp b/include/mgp.hpp index 37bb5db27..f35231062 100644 --- a/include/mgp.hpp +++ b/include/mgp.hpp @@ -4359,19 +4359,22 @@ inline List SearchTextIndex(mgp_graph *memgraph_graph, std::string_view index_na text_search_mode search_mode) { auto results_or_error = Map(mgp::MemHandlerCallback(graph_search_text_index, memgraph_graph, index_name.data(), search_query.data(), search_mode)); - if (!results_or_error.KeyExists(kErrorMsgKey) || !results_or_error.KeyExists(kSearchResultsKey)) { + if (results_or_error.KeyExists(kErrorMsgKey)) { + if (!results_or_error.At(kErrorMsgKey).IsString()) { + throw TextSearchException{"The error message is not a string!"}; + } + throw TextSearchException(results_or_error.At(kErrorMsgKey).ValueString().data()); + } + + if (!results_or_error.KeyExists(kSearchResultsKey)) { throw TextSearchException{"Incomplete text index search results!"}; } - if (!results_or_error.At(kErrorMsgKey).IsString() || !results_or_error.At(kSearchResultsKey).IsList()) { + + if (!results_or_error.At(kSearchResultsKey).IsList()) { throw TextSearchException{"Text index search results have wrong type!"}; } - auto maybe_error = results_or_error[kErrorMsgKey].ValueString(); - if (!maybe_error.empty()) { - throw TextSearchException(maybe_error.data()); - } - - return results_or_error[kSearchResultsKey].ValueList(); + return results_or_error.At(kSearchResultsKey).ValueList(); } inline std::string_view AggregateOverTextIndex(mgp_graph *memgraph_graph, std::string_view index_name, @@ -4379,19 +4382,23 @@ inline std::string_view AggregateOverTextIndex(mgp_graph *memgraph_graph, std::s auto results_or_error = Map(mgp::MemHandlerCallback(graph_aggregate_over_text_index, memgraph_graph, index_name.data(), search_query.data(), aggregation_query.data())); - if (!results_or_error.KeyExists(kErrorMsgKey) || !results_or_error.KeyExists(kAggregationResultsKey)) { + + if (results_or_error.KeyExists(kErrorMsgKey)) { + if (!results_or_error.At(kErrorMsgKey).IsString()) { + throw TextSearchException{"The error message is not a string!"}; + } + throw TextSearchException(results_or_error.At(kErrorMsgKey).ValueString().data()); + } + + if (!results_or_error.KeyExists(kAggregationResultsKey)) { throw TextSearchException{"Incomplete text index aggregation results!"}; } - if (!results_or_error.At(kErrorMsgKey).IsString() || !results_or_error.At(kAggregationResultsKey).IsString()) { + + if (!results_or_error.At(kAggregationResultsKey).IsString()) { throw TextSearchException{"Text index aggregation results have wrong type!"}; } - auto maybe_error = results_or_error[kErrorMsgKey].ValueString(); - if (!maybe_error.empty()) { - throw TextSearchException(maybe_error.data()); - } - - return results_or_error[kAggregationResultsKey].ValueString(); + return results_or_error.At(kAggregationResultsKey).ValueString(); } inline bool CreateExistenceConstraint(mgp_graph *memgraph_graph, const std::string_view label, diff --git a/src/query/procedure/mg_procedure_impl.cpp b/src/query/procedure/mg_procedure_impl.cpp index db9f92fd4..201157cdd 100644 --- a/src/query/procedure/mg_procedure_impl.cpp +++ b/src/query/procedure/mg_procedure_impl.cpp @@ -3377,11 +3377,20 @@ mgp_vertex *GetVertexByGid(mgp_graph *graph, memgraph::storage::Gid id, mgp_memo } void WrapTextSearch(mgp_graph *graph, mgp_memory *memory, mgp_map **result, - const std::vector &vertex_ids = {}, const std::string &error_msg = "") { + const std::vector &vertex_ids = {}, + const std::optional &error_msg = std::nullopt) { if (const auto err = mgp_map_make_empty(memory, result); err != mgp_error::MGP_ERROR_NO_ERROR) { throw std::logic_error("Retrieving text search results failed during creation of a mgp_map"); } + mgp_value *error_value; + if (error_msg.has_value()) { + if (const auto err = mgp_value_make_string(error_msg.value().data(), memory, &error_value); + err != mgp_error::MGP_ERROR_NO_ERROR) { + throw std::logic_error("Retrieving text search results failed during creation of a string mgp_value"); + } + } + mgp_list *search_results{}; if (const auto err = mgp_list_make_empty(vertex_ids.size(), memory, &search_results); err != mgp_error::MGP_ERROR_NO_ERROR) { @@ -3403,49 +3412,45 @@ void WrapTextSearch(mgp_graph *graph, mgp_memory *memory, mgp_map **result, mgp_value *search_results_value; if (const auto err = mgp_value_make_list(search_results, &search_results_value); err != mgp_error::MGP_ERROR_NO_ERROR) { - throw std::logic_error("Retrieving text search results failed during creation of a map mgp_value"); + throw std::logic_error("Retrieving text search results failed during creation of a list mgp_value"); } - mgp_value *error_msg_value; - if (const auto err = mgp_value_make_string(error_msg.data(), memory, &error_msg_value); - err != mgp_error::MGP_ERROR_NO_ERROR) { - throw std::logic_error("Retrieving text search results failed during creation of a map mgp_value"); + if (error_msg.has_value()) { + if (const auto err = mgp_map_insert(*result, "error_msg", error_value); err != mgp_error::MGP_ERROR_NO_ERROR) { + throw std::logic_error("Retrieving text index search error failed during insertion into mgp_map"); + } + return; } if (const auto err = mgp_map_insert(*result, "search_results", search_results_value); err != mgp_error::MGP_ERROR_NO_ERROR) { - throw std::logic_error("Retrieving text search results failed during insertion into mgp_map"); - } - - if (const auto err = mgp_map_insert(*result, "error_msg", error_msg_value); err != mgp_error::MGP_ERROR_NO_ERROR) { - throw std::logic_error("Retrieving text search results failed during insertion into mgp_map"); + throw std::logic_error("Retrieving text index search results failed during insertion into mgp_map"); } } void WrapTextIndexAggregation(mgp_memory *memory, mgp_map **result, const std::string &aggregation_result, - const std::string &error_msg = "") { + const std::optional &error_msg = std::nullopt) { if (const auto err = mgp_map_make_empty(memory, result); err != mgp_error::MGP_ERROR_NO_ERROR) { - throw std::logic_error("Retrieving text index aggregation results failed during creation of a mgp_map"); + throw std::logic_error("Retrieving text search results failed during creation of a mgp_map"); } - mgp_value *aggregation_results_value; - if (const auto err = mgp_value_make_string(aggregation_result.data(), memory, &aggregation_results_value); + mgp_value *aggregation_result_or_error_value; + if (const auto err = mgp_value_make_string(error_msg.value_or(aggregation_result).data(), memory, + &aggregation_result_or_error_value); err != mgp_error::MGP_ERROR_NO_ERROR) { - throw std::logic_error("Retrieving text index aggregation results failed during creation of a string mgp_value"); + throw std::logic_error("Retrieving text search results failed during creation of a string mgp_value"); } - mgp_value *error_msg_value; - if (const auto err = mgp_value_make_string(error_msg.data(), memory, &error_msg_value); + if (error_msg.has_value()) { + if (const auto err = mgp_map_insert(*result, "error_msg", aggregation_result_or_error_value); + err != mgp_error::MGP_ERROR_NO_ERROR) { + throw std::logic_error("Retrieving text index aggregation error failed during insertion into mgp_map"); + } + return; + } + + if (const auto err = mgp_map_insert(*result, "aggregation_results", aggregation_result_or_error_value); err != mgp_error::MGP_ERROR_NO_ERROR) { - throw std::logic_error("Retrieving text index aggregation results failed during creation of a map mgp_value"); - } - - if (const auto err = mgp_map_insert(*result, "aggregation_results", aggregation_results_value); - err != mgp_error::MGP_ERROR_NO_ERROR) { - throw std::logic_error("Retrieving text index aggregation results failed during insertion into mgp_map"); - } - - if (const auto err = mgp_map_insert(*result, "error_msg", error_msg_value); err != mgp_error::MGP_ERROR_NO_ERROR) { throw std::logic_error("Retrieving text index aggregation results failed during insertion into mgp_map"); } } @@ -3453,14 +3458,14 @@ 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, text_search_mode search_mode, mgp_memory *memory, mgp_map **result) { return WrapExceptions([graph, memory, index_name, search_query, search_mode, result]() { - std::vector search_results; - std::string error_msg; + std::vector found_vertices_ids; + std::optional error_msg = std::nullopt; try { - search_results = graph->getImpl()->TextIndexSearch(index_name, search_query, search_mode); + found_vertices_ids = graph->getImpl()->TextIndexSearch(index_name, search_query, search_mode); } catch (memgraph::query::QueryException &e) { error_msg = e.what(); } - WrapTextSearch(graph, memory, result, search_results, error_msg); + WrapTextSearch(graph, memory, result, found_vertices_ids, error_msg); }); } @@ -3468,7 +3473,7 @@ mgp_error mgp_graph_aggregate_over_text_index(mgp_graph *graph, const char *inde const char *aggregation_query, mgp_memory *memory, mgp_map **result) { return WrapExceptions([graph, memory, index_name, search_query, aggregation_query, result]() { std::string search_results; - std::string error_msg; + std::optional error_msg = std::nullopt; try { search_results = graph->getImpl()->TextIndexAggregate(index_name, search_query, aggregation_query); } catch (memgraph::query::QueryException &e) {