Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug-fix: Don't use pointers to elements in an actively modified vector during query processing. #182

Merged
merged 5 commits into from
Dec 21, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
19 changes: 12 additions & 7 deletions components/core/src/Grep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -487,17 +487,17 @@ SubQueryMatchabilityResult generate_logtypes_and_vars_for_subquery(
return SubQueryMatchabilityResult::MayMatch;
}

bool Grep::process_raw_query(
std::optional<Query> Grep::process_raw_query(
Archive const& archive,
string const& search_string,
epochtime_t search_begin_ts,
epochtime_t search_end_ts,
bool ignore_case,
Query& query,
log_surgeon::lexers::ByteLexer& forward_lexer,
log_surgeon::lexers::ByteLexer& reverse_lexer,
bool use_heuristic
) {
Query query;
// Set properties which require no processing
query.set_search_begin_timestamp(search_begin_ts);
query.set_search_end_timestamp(search_end_ts);
Expand Down Expand Up @@ -558,11 +558,11 @@ bool Grep::process_raw_query(
// - (token1 as logtype) (token2 as var)
// - (token1 as var) (token2 as logtype)
// - (token1 as var) (token2 as var)
SubQuery sub_query;
vector<SubQuery> sub_queries;
string logtype;
bool type_of_one_token_changed = true;
while (type_of_one_token_changed) {
sub_query.clear();
SubQuery sub_query;

// Compute logtypes and variables for query
auto matchability = generate_logtypes_and_vars_for_subquery(
Expand All @@ -579,9 +579,9 @@ bool Grep::process_raw_query(

// Since other sub-queries will be superceded by this one, we can stop processing
// now
return true;
return query;
case SubQueryMatchabilityResult::MayMatch:
query.add_sub_query(sub_query);
sub_queries.push_back(std::move(sub_query));
break;
case SubQueryMatchabilityResult::WontMatch:
default:
Expand All @@ -599,7 +599,12 @@ bool Grep::process_raw_query(
}
}

return query.contains_sub_queries();
if (sub_queries.empty()) {
return std::nullopt;
}

query.set_sub_queries(std::move(sub_queries));
return query;
}

bool Grep::get_bounds_of_next_potential_var(
Expand Down
7 changes: 3 additions & 4 deletions components/core/src/Grep.hpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#ifndef GREP_HPP
#define GREP_HPP

#include <optional>
#include <string>

#include <log_surgeon/Lexer.hpp>
Expand Down Expand Up @@ -35,19 +36,17 @@ class Grep {
* @param search_begin_ts
* @param search_end_ts
* @param ignore_case
* @param query
* @param forward_lexer DFA for determining if input is in the schema
* @param reverse_lexer DFA for determining if reverse of input is in the schema
* @param use_heuristic
* @return true if query may match messages, false otherwise
* @return Query if it may match a message, std::nullopt otherwise
*/
static bool process_raw_query(
static std::optional<Query> process_raw_query(
wraymo marked this conversation as resolved.
Show resolved Hide resolved
streaming_archive::reader::Archive const& archive,
std::string const& search_string,
epochtime_t search_begin_ts,
epochtime_t search_end_ts,
bool ignore_case,
Query& query,
log_surgeon::lexers::ByteLexer& forward_lexer,
log_surgeon::lexers::ByteLexer& reverse_lexer,
bool use_heuristic
Expand Down
12 changes: 7 additions & 5 deletions components/core/src/Query.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,14 @@ void Query::set_search_string(string const& search_string) {
m_search_string_matches_all = (m_search_string.empty() || "*" == m_search_string);
}

void Query::add_sub_query(SubQuery const& sub_query) {
m_sub_queries.push_back(sub_query);
void Query::set_sub_queries(std::vector<SubQuery> sub_queries) {
m_sub_queries = std::move(sub_queries);

// Add to relevant sub-queries if necessary
if (sub_query.get_ids_of_matching_segments().count(m_prev_segment_id)) {
m_relevant_sub_queries.push_back(&m_sub_queries.back());
for (auto& sub_query : m_sub_queries) {
// Add to relevant sub-queries if necessary
if (sub_query.get_ids_of_matching_segments().count(m_prev_segment_id)) {
m_relevant_sub_queries.push_back(&sub_query);
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion components/core/src/Query.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ class Query {
void set_ignore_case(bool ignore_case) { m_ignore_case = ignore_case; }

void set_search_string(std::string const& search_string);
void add_sub_query(SubQuery const& sub_query);
void set_sub_queries(std::vector<SubQuery> sub_queries);
void clear_sub_queries();
/**
* Populates the set of relevant sub-queries with only those that match the given segment
Expand Down
27 changes: 13 additions & 14 deletions components/core/src/clg/clg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,22 +206,21 @@ static bool search(
std::set<segment_id_t> ids_of_segments_to_search;
bool is_superseding_query = false;
for (auto const& search_string : search_strings) {
Query query;
if (Grep::process_raw_query(
archive,
search_string,
search_begin_ts,
search_end_ts,
command_line_args.ignore_case(),
query,
forward_lexer,
reverse_lexer,
use_heuristic
))
{
auto query_processing_result = Grep::process_raw_query(
archive,
search_string,
search_begin_ts,
search_end_ts,
command_line_args.ignore_case(),
forward_lexer,
reverse_lexer,
use_heuristic
);
if (query_processing_result.has_value()) {
auto& query = query_processing_result.value();
no_queries_match = false;

if (query.contains_sub_queries() == false) {
if (false == query.contains_sub_queries()) {
// Search string supersedes all other possible search strings
is_superseding_query = true;
// Remove existing queries since they are superseded by this one
Expand Down
26 changes: 12 additions & 14 deletions components/core/src/clo/clo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,23 +260,21 @@ static bool search_archive(
auto search_begin_ts = command_line_args.get_search_begin_ts();
auto search_end_ts = command_line_args.get_search_end_ts();

Query query;
if (false
== Grep::process_raw_query(
archive_reader,
command_line_args.get_search_string(),
search_begin_ts,
search_end_ts,
command_line_args.ignore_case(),
query,
*forward_lexer,
*reverse_lexer,
use_heuristic
))
{
auto query_processing_result = Grep::process_raw_query(
archive_reader,
command_line_args.get_search_string(),
search_begin_ts,
search_end_ts,
command_line_args.ignore_case(),
*forward_lexer,
*reverse_lexer,
use_heuristic
);
if (false == query_processing_result.has_value()) {
return true;
}

auto& query = query_processing_result.value();
// Get all segments potentially containing query results
std::set<segment_id_t> ids_of_segments_to_search;
for (auto& sub_query : query.get_sub_queries()) {
Expand Down
Loading