Upgrade Antrl to 4.10.1 and remove antlr_lock (#441)

This commit is contained in:
Marko Budiselić 2022-07-26 08:31:38 +02:00 committed by GitHub
parent eb0b3141d5
commit 74d3663821
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 325 additions and 350 deletions

1
libs/.gitignore vendored
View File

@ -5,3 +5,4 @@
!CMakeLists.txt !CMakeLists.txt
!__main.cpp !__main.cpp
!pulsar.patch !pulsar.patch
!antlr4.10.1.patch

View File

@ -106,6 +106,7 @@ import_external_library(antlr4 STATIC
-DWITH_LIBCXX=OFF # because of debian bug -DWITH_LIBCXX=OFF # because of debian bug
-DCMAKE_SKIP_INSTALL_ALL_DEPENDENCY=true -DCMAKE_SKIP_INSTALL_ALL_DEPENDENCY=true
-DCMAKE_CXX_STANDARD=20 -DCMAKE_CXX_STANDARD=20
-DANTLR_BUILD_CPP_TESTS=OFF
BUILD_COMMAND $(MAKE) antlr4_static BUILD_COMMAND $(MAKE) antlr4_static
INSTALL_COMMAND $(MAKE) install) INSTALL_COMMAND $(MAKE) install)

13
libs/antlr4.10.1.patch Normal file
View File

@ -0,0 +1,13 @@
diff --git a/runtime/Cpp/runtime/CMakeLists.txt b/runtime/Cpp/runtime/CMakeLists.txt
index baf46cac9..2e7756de8 100644
--- a/runtime/Cpp/runtime/CMakeLists.txt
+++ b/runtime/Cpp/runtime/CMakeLists.txt
@@ -134,7 +134,7 @@ set_target_properties(antlr4_static
ARCHIVE_OUTPUT_DIRECTORY ${LIB_OUTPUT_DIR}
COMPILE_FLAGS "${disabled_compile_warnings} ${extra_static_compile_flags}")
-install(TARGETS antlr4_shared
+install(TARGETS antlr4_shared OPTIONAL
EXPORT antlr4-targets
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}

View File

@ -1,43 +0,0 @@
diff --git a/runtime/Cpp/runtime/CMakeLists.txt b/runtime/Cpp/runtime/CMakeLists.txt
index a8503bb..11362cf 100644
--- a/runtime/Cpp/runtime/CMakeLists.txt
+++ b/runtime/Cpp/runtime/CMakeLists.txt
@@ -5,8 +5,8 @@ set(THIRDPARTY_DIR ${CMAKE_BINARY_DIR}/runtime/thirdparty)
set(UTFCPP_DIR ${THIRDPARTY_DIR}/utfcpp)
ExternalProject_Add(
utfcpp
- GIT_REPOSITORY "git://github.com/nemtrif/utfcpp"
- GIT_TAG "v3.1.1"
+ GIT_REPOSITORY "https://github.com/nemtrif/utfcpp"
+ GIT_TAG "v3.2.1"
SOURCE_DIR ${UTFCPP_DIR}
UPDATE_DISCONNECTED 1
CMAKE_ARGS -DCMAKE_INSTALL_PREFIX=${UTFCPP_DIR}/install -Dgtest_force_shared_crt=ON
@@ -118,7 +118,7 @@ set_target_properties(antlr4_static
ARCHIVE_OUTPUT_DIRECTORY ${LIB_OUTPUT_DIR}
COMPILE_FLAGS "${disabled_compile_warnings} ${extra_static_compile_flags}")
-install(TARGETS antlr4_shared
+install(TARGETS antlr4_shared OPTIONAL
DESTINATION lib
EXPORT antlr4-targets)
install(TARGETS antlr4_static
diff --git a/runtime/Cpp/runtime/src/support/Any.h b/runtime/Cpp/runtime/src/support/Any.h
index 468db98..65a473b 100644
--- a/runtime/Cpp/runtime/src/support/Any.h
+++ b/runtime/Cpp/runtime/src/support/Any.h
@@ -122,12 +122,12 @@ private:
}
private:
- template<int N = 0, typename std::enable_if<N == N && std::is_nothrow_copy_constructible<T>::value, int>::type = 0>
+ template<int N = 0, typename std::enable_if<N == N && std::is_copy_constructible<T>::value, int>::type = 0>
Base* clone() const {
return new Derived<T>(value);
}
- template<int N = 0, typename std::enable_if<N == N && !std::is_nothrow_copy_constructible<T>::value, int>::type = 0>
+ template<int N = 0, typename std::enable_if<N == N && !std::is_copy_constructible<T>::value, int>::type = 0>
Base* clone() const {
return nullptr;
}

View File

@ -105,7 +105,7 @@ repo_clone_try_double () {
# Download from primary_urls might fail because the cache is not installed. # Download from primary_urls might fail because the cache is not installed.
declare -A primary_urls=( declare -A primary_urls=(
["antlr4-code"]="http://$local_cache_host/git/antlr4.git" ["antlr4-code"]="http://$local_cache_host/git/antlr4.git"
["antlr4-generator"]="http://$local_cache_host/file/antlr-4.9.2-complete.jar" ["antlr4-generator"]="http://$local_cache_host/file/antlr-4.10.1-complete.jar"
["cppitertools"]="http://$local_cache_host/git/cppitertools.git" ["cppitertools"]="http://$local_cache_host/git/cppitertools.git"
["rapidcheck"]="http://$local_cache_host/git/rapidcheck.git" ["rapidcheck"]="http://$local_cache_host/git/rapidcheck.git"
["gbenchmark"]="http://$local_cache_host/git/benchmark.git" ["gbenchmark"]="http://$local_cache_host/git/benchmark.git"
@ -130,7 +130,7 @@ declare -A primary_urls=(
# should fail. # should fail.
declare -A secondary_urls=( declare -A secondary_urls=(
["antlr4-code"]="https://github.com/antlr/antlr4.git" ["antlr4-code"]="https://github.com/antlr/antlr4.git"
["antlr4-generator"]="http://www.antlr.org/download/antlr-4.9.2-complete.jar" ["antlr4-generator"]="https://www.antlr.org/download/antlr-4.10.1-complete.jar"
["cppitertools"]="https://github.com/ryanhaining/cppitertools.git" ["cppitertools"]="https://github.com/ryanhaining/cppitertools.git"
["rapidcheck"]="https://github.com/emil-e/rapidcheck.git" ["rapidcheck"]="https://github.com/emil-e/rapidcheck.git"
["gbenchmark"]="https://github.com/google/benchmark.git" ["gbenchmark"]="https://github.com/google/benchmark.git"
@ -152,10 +152,10 @@ declare -A secondary_urls=(
# antlr # antlr
file_get_try_double "${primary_urls[antlr4-generator]}" "${secondary_urls[antlr4-generator]}" file_get_try_double "${primary_urls[antlr4-generator]}" "${secondary_urls[antlr4-generator]}"
antlr4_tag="4.9.2" # v4.9.2 antlr4_tag="4.10.1" # v4.10.1
repo_clone_try_double "${primary_urls[antlr4-code]}" "${secondary_urls[antlr4-code]}" "antlr4" "$antlr4_tag" true repo_clone_try_double "${primary_urls[antlr4-code]}" "${secondary_urls[antlr4-code]}" "antlr4" "$antlr4_tag" true
pushd antlr4 pushd antlr4
git apply ../antlr4.patch git apply ../antlr4.10.1.patch
popd popd
# cppitertools v2.0 2019-12-23 # cppitertools v2.0 2019-12-23

View File

@ -1252,9 +1252,8 @@ int main(int argc, char **argv) {
// the triggers // the triggers
auto storage_accessor = interpreter_context.db->Access(); auto storage_accessor = interpreter_context.db->Access();
auto dba = memgraph::query::DbAccessor{&storage_accessor}; auto dba = memgraph::query::DbAccessor{&storage_accessor};
interpreter_context.trigger_store.RestoreTriggers(&interpreter_context.ast_cache, &dba, interpreter_context.trigger_store.RestoreTriggers(
&interpreter_context.antlr_lock, interpreter_context.config.query, &interpreter_context.ast_cache, &dba, interpreter_context.config.query, interpreter_context.auth_checker);
interpreter_context.auth_checker);
} }
// As the Stream transformations are using modules, they have to be restored after the query modules are loaded. // As the Stream transformations are using modules, they have to be restored after the query modules are loaded.

View File

@ -82,7 +82,7 @@ add_custom_command(
OUTPUT ${antlr_opencypher_generated_src} ${antlr_opencypher_generated_include} OUTPUT ${antlr_opencypher_generated_src} ${antlr_opencypher_generated_include}
COMMAND ${CMAKE_COMMAND} -E make_directory ${opencypher_generated} COMMAND ${CMAKE_COMMAND} -E make_directory ${opencypher_generated}
COMMAND COMMAND
java -jar ${CMAKE_SOURCE_DIR}/libs/antlr-4.9.2-complete.jar java -jar ${CMAKE_SOURCE_DIR}/libs/antlr-4.10.1-complete.jar
-Dlanguage=Cpp -visitor -package antlropencypher -Dlanguage=Cpp -visitor -package antlropencypher
-o ${opencypher_generated} -o ${opencypher_generated}
${opencypher_lexer_grammar} ${opencypher_parser_grammar} ${opencypher_lexer_grammar} ${opencypher_parser_grammar}

View File

@ -21,8 +21,7 @@ namespace memgraph::query {
CachedPlan::CachedPlan(std::unique_ptr<LogicalPlan> plan) : plan_(std::move(plan)) {} CachedPlan::CachedPlan(std::unique_ptr<LogicalPlan> plan) : plan_(std::move(plan)) {}
ParsedQuery ParseQuery(const std::string &query_string, const std::map<std::string, storage::PropertyValue> &params, ParsedQuery ParseQuery(const std::string &query_string, const std::map<std::string, storage::PropertyValue> &params,
utils::SkipList<QueryCacheEntry> *cache, utils::SpinLock *antlr_lock, utils::SkipList<QueryCacheEntry> *cache, const InterpreterConfig::Query &query_config) {
const InterpreterConfig::Query &query_config) {
// Strip the query for caching purposes. The process of stripping a query // Strip the query for caching purposes. The process of stripping a query
// "normalizes" it by replacing any literals with new parameters. This // "normalizes" it by replacing any literals with new parameters. This
// results in just the *structure* of the query being taken into account for // results in just the *structure* of the query being taken into account for
@ -63,20 +62,16 @@ ParsedQuery ParseQuery(const std::string &query_string, const std::map<std::stri
}; };
if (it == accessor.end()) { if (it == accessor.end()) {
{ try {
std::unique_lock<utils::SpinLock> guard(*antlr_lock); parser = std::make_unique<frontend::opencypher::Parser>(stripped_query.query());
} catch (const SyntaxException &e) {
// There is a syntax exception in the stripped query. Re-run the parser
// on the original query to get an appropriate error messsage.
parser = std::make_unique<frontend::opencypher::Parser>(query_string);
try { // If an exception was not thrown here, the stripper messed something
parser = std::make_unique<frontend::opencypher::Parser>(stripped_query.query()); // up.
} catch (const SyntaxException &e) { LOG_FATAL("The stripped query can't be parsed, but the original can.");
// There is a syntax exception in the stripped query. Re-run the parser
// on the original query to get an appropriate error messsage.
parser = std::make_unique<frontend::opencypher::Parser>(query_string);
// If an exception was not thrown here, the stripper messed something
// up.
LOG_FATAL("The stripped query can't be parsed, but the original can.");
}
} }
// Convert the ANTLR4 parse tree into an AST. // Convert the ANTLR4 parse tree into an AST.

View File

@ -111,8 +111,7 @@ struct ParsedQuery {
}; };
ParsedQuery ParseQuery(const std::string &query_string, const std::map<std::string, storage::PropertyValue> &params, ParsedQuery ParseQuery(const std::string &query_string, const std::map<std::string, storage::PropertyValue> &params,
utils::SkipList<QueryCacheEntry> *cache, utils::SpinLock *antlr_lock, utils::SkipList<QueryCacheEntry> *cache, const InterpreterConfig::Query &query_config);
const InterpreterConfig::Query &query_config);
class SingleNodeLogicalPlan final : public LogicalPlan { class SingleNodeLogicalPlan final : public LogicalPlan {
public: public:

File diff suppressed because it is too large Load Diff

View File

@ -115,7 +115,7 @@ class CypherMainVisitor : public antlropencypher::MemgraphCypherBaseVisitor {
auto operators = ExtractOperators(all_children, allowed_operators); auto operators = ExtractOperators(all_children, allowed_operators);
for (auto *expression : _expressions) { for (auto *expression : _expressions) {
expressions.push_back(expression->accept(this)); expressions.push_back(std::any_cast<Expression *>(expression->accept(this)));
} }
Expression *first_operand = expressions[0]; Expression *first_operand = expressions[0];
@ -131,7 +131,7 @@ class CypherMainVisitor : public antlropencypher::MemgraphCypherBaseVisitor {
DMG_ASSERT(_expression, "can't happen"); DMG_ASSERT(_expression, "can't happen");
auto operators = ExtractOperators(all_children, allowed_operators); auto operators = ExtractOperators(all_children, allowed_operators);
Expression *expression = _expression->accept(this); Expression *expression = std::any_cast<Expression *>(_expression->accept(this));
for (int i = (int)operators.size() - 1; i >= 0; --i) { for (int i = (int)operators.size() - 1; i >= 0; --i) {
expression = CreateUnaryOperatorByToken(operators[i], expression); expression = CreateUnaryOperatorByToken(operators[i], expression);
} }

View File

@ -1181,7 +1181,7 @@ PreparedQuery PrepareExplainQuery(ParsedQuery parsed_query, std::map<std::string
// full query string) when given just the inner query to execute. // full query string) when given just the inner query to execute.
ParsedQuery parsed_inner_query = ParsedQuery parsed_inner_query =
ParseQuery(parsed_query.query_string.substr(kExplainQueryStart.size()), parsed_query.user_parameters, ParseQuery(parsed_query.query_string.substr(kExplainQueryStart.size()), parsed_query.user_parameters,
&interpreter_context->ast_cache, &interpreter_context->antlr_lock, interpreter_context->config.query); &interpreter_context->ast_cache, interpreter_context->config.query);
auto *cypher_query = utils::Downcast<CypherQuery>(parsed_inner_query.query); auto *cypher_query = utils::Downcast<CypherQuery>(parsed_inner_query.query);
MG_ASSERT(cypher_query, "Cypher grammar should not allow other queries in EXPLAIN"); MG_ASSERT(cypher_query, "Cypher grammar should not allow other queries in EXPLAIN");
@ -1248,7 +1248,7 @@ PreparedQuery PrepareProfileQuery(ParsedQuery parsed_query, bool in_explicit_tra
// full query string) when given just the inner query to execute. // full query string) when given just the inner query to execute.
ParsedQuery parsed_inner_query = ParsedQuery parsed_inner_query =
ParseQuery(parsed_query.query_string.substr(kProfileQueryStart.size()), parsed_query.user_parameters, ParseQuery(parsed_query.query_string.substr(kProfileQueryStart.size()), parsed_query.user_parameters,
&interpreter_context->ast_cache, &interpreter_context->antlr_lock, interpreter_context->config.query); &interpreter_context->ast_cache, interpreter_context->config.query);
auto *cypher_query = utils::Downcast<CypherQuery>(parsed_inner_query.query); auto *cypher_query = utils::Downcast<CypherQuery>(parsed_inner_query.query);
MG_ASSERT(cypher_query, "Cypher grammar should not allow other queries in PROFILE"); MG_ASSERT(cypher_query, "Cypher grammar should not allow other queries in PROFILE");
@ -1566,8 +1566,7 @@ Callback CreateTrigger(TriggerQuery *trigger_query,
interpreter_context->trigger_store.AddTrigger( interpreter_context->trigger_store.AddTrigger(
std::move(trigger_name), trigger_statement, user_parameters, ToTriggerEventType(event_type), std::move(trigger_name), trigger_statement, user_parameters, ToTriggerEventType(event_type),
before_commit ? TriggerPhase::BEFORE_COMMIT : TriggerPhase::AFTER_COMMIT, &interpreter_context->ast_cache, before_commit ? TriggerPhase::BEFORE_COMMIT : TriggerPhase::AFTER_COMMIT, &interpreter_context->ast_cache,
dba, &interpreter_context->antlr_lock, interpreter_context->config.query, std::move(owner), dba, interpreter_context->config.query, std::move(owner), interpreter_context->auth_checker);
interpreter_context->auth_checker);
return {}; return {};
}}; }};
} }
@ -2123,8 +2122,8 @@ Interpreter::PrepareResult Interpreter::Prepare(const std::string &query_string,
query_execution->summary["cost_estimate"] = 0.0; query_execution->summary["cost_estimate"] = 0.0;
utils::Timer parsing_timer; utils::Timer parsing_timer;
ParsedQuery parsed_query = ParseQuery(query_string, params, &interpreter_context_->ast_cache, ParsedQuery parsed_query =
&interpreter_context_->antlr_lock, interpreter_context_->config.query); ParseQuery(query_string, params, &interpreter_context_->ast_cache, interpreter_context_->config.query);
query_execution->summary["parsing_time"] = parsing_timer.Elapsed().count(); query_execution->summary["parsing_time"] = parsing_timer.Elapsed().count();
// Some queries require an active transaction in order to be prepared. // Some queries require an active transaction in order to be prepared.

View File

@ -173,13 +173,6 @@ struct InterpreterContext {
storage::Storage *db; storage::Storage *db;
// ANTLR has singleton instance that is shared between threads. It is
// protected by locks inside of ANTLR. Unfortunately, they are not protected
// in a very good way. Once we have ANTLR version without race conditions we
// can remove this lock. This will probably never happen since ANTLR
// developers introduce more bugs in each version. Fortunately, we have
// cache so this lock probably won't impact performance much...
utils::SpinLock antlr_lock;
std::optional<double> tsc_frequency{utils::GetTSCFrequency()}; std::optional<double> tsc_frequency{utils::GetTSCFrequency()};
std::atomic<bool> is_shutting_down{false}; std::atomic<bool> is_shutting_down{false};

View File

@ -153,10 +153,10 @@ std::vector<std::pair<Identifier, TriggerIdentifierTag>> GetPredefinedIdentifier
Trigger::Trigger(std::string name, const std::string &query, Trigger::Trigger(std::string name, const std::string &query,
const std::map<std::string, storage::PropertyValue> &user_parameters, const std::map<std::string, storage::PropertyValue> &user_parameters,
const TriggerEventType event_type, utils::SkipList<QueryCacheEntry> *query_cache, const TriggerEventType event_type, utils::SkipList<QueryCacheEntry> *query_cache,
DbAccessor *db_accessor, utils::SpinLock *antlr_lock, const InterpreterConfig::Query &query_config, DbAccessor *db_accessor, const InterpreterConfig::Query &query_config,
std::optional<std::string> owner, const query::AuthChecker *auth_checker) std::optional<std::string> owner, const query::AuthChecker *auth_checker)
: name_{std::move(name)}, : name_{std::move(name)},
parsed_statements_{ParseQuery(query, user_parameters, query_cache, antlr_lock, query_config)}, parsed_statements_{ParseQuery(query, user_parameters, query_cache, query_config)},
event_type_{event_type}, event_type_{event_type},
owner_{std::move(owner)} { owner_{std::move(owner)} {
// We check immediately if the query is valid by trying to create a plan. // We check immediately if the query is valid by trying to create a plan.
@ -257,7 +257,7 @@ inline constexpr uint64_t kVersion{2};
TriggerStore::TriggerStore(std::filesystem::path directory) : storage_{std::move(directory)} {} TriggerStore::TriggerStore(std::filesystem::path directory) : storage_{std::move(directory)} {}
void TriggerStore::RestoreTriggers(utils::SkipList<QueryCacheEntry> *query_cache, DbAccessor *db_accessor, void TriggerStore::RestoreTriggers(utils::SkipList<QueryCacheEntry> *query_cache, DbAccessor *db_accessor,
utils::SpinLock *antlr_lock, const InterpreterConfig::Query &query_config, const InterpreterConfig::Query &query_config,
const query::AuthChecker *auth_checker) { const query::AuthChecker *auth_checker) {
MG_ASSERT(before_commit_triggers_.size() == 0 && after_commit_triggers_.size() == 0, MG_ASSERT(before_commit_triggers_.size() == 0 && after_commit_triggers_.size() == 0,
"Cannot restore trigger when some triggers already exist!"); "Cannot restore trigger when some triggers already exist!");
@ -317,8 +317,8 @@ void TriggerStore::RestoreTriggers(utils::SkipList<QueryCacheEntry> *query_cache
std::optional<Trigger> trigger; std::optional<Trigger> trigger;
try { try {
trigger.emplace(trigger_name, statement, user_parameters, event_type, query_cache, db_accessor, antlr_lock, trigger.emplace(trigger_name, statement, user_parameters, event_type, query_cache, db_accessor, query_config,
query_config, std::move(owner), auth_checker); std::move(owner), auth_checker);
} catch (const utils::BasicException &e) { } catch (const utils::BasicException &e) {
spdlog::warn("Failed to create trigger '{}' because: {}", trigger_name, e.what()); spdlog::warn("Failed to create trigger '{}' because: {}", trigger_name, e.what());
continue; continue;
@ -336,8 +336,8 @@ void TriggerStore::AddTrigger(std::string name, const std::string &query,
const std::map<std::string, storage::PropertyValue> &user_parameters, const std::map<std::string, storage::PropertyValue> &user_parameters,
TriggerEventType event_type, TriggerPhase phase, TriggerEventType event_type, TriggerPhase phase,
utils::SkipList<QueryCacheEntry> *query_cache, DbAccessor *db_accessor, utils::SkipList<QueryCacheEntry> *query_cache, DbAccessor *db_accessor,
utils::SpinLock *antlr_lock, const InterpreterConfig::Query &query_config, const InterpreterConfig::Query &query_config, std::optional<std::string> owner,
std::optional<std::string> owner, const query::AuthChecker *auth_checker) { const query::AuthChecker *auth_checker) {
std::unique_lock store_guard{store_lock_}; std::unique_lock store_guard{store_lock_};
if (storage_.Get(name)) { if (storage_.Get(name)) {
throw utils::BasicException("Trigger with the same name already exists."); throw utils::BasicException("Trigger with the same name already exists.");
@ -345,8 +345,8 @@ void TriggerStore::AddTrigger(std::string name, const std::string &query,
std::optional<Trigger> trigger; std::optional<Trigger> trigger;
try { try {
trigger.emplace(std::move(name), query, user_parameters, event_type, query_cache, db_accessor, antlr_lock, trigger.emplace(std::move(name), query, user_parameters, event_type, query_cache, db_accessor, query_config,
query_config, std::move(owner), auth_checker); std::move(owner), auth_checker);
} catch (const utils::BasicException &e) { } catch (const utils::BasicException &e) {
const auto identifiers = GetPredefinedIdentifiers(event_type); const auto identifiers = GetPredefinedIdentifiers(event_type);
std::stringstream identifier_names_stream; std::stringstream identifier_names_stream;

View File

@ -34,7 +34,7 @@ namespace memgraph::query {
struct Trigger { struct Trigger {
explicit Trigger(std::string name, const std::string &query, explicit Trigger(std::string name, const std::string &query,
const std::map<std::string, storage::PropertyValue> &user_parameters, TriggerEventType event_type, const std::map<std::string, storage::PropertyValue> &user_parameters, TriggerEventType event_type,
utils::SkipList<QueryCacheEntry> *query_cache, DbAccessor *db_accessor, utils::SpinLock *antlr_lock, utils::SkipList<QueryCacheEntry> *query_cache, DbAccessor *db_accessor,
const InterpreterConfig::Query &query_config, std::optional<std::string> owner, const InterpreterConfig::Query &query_config, std::optional<std::string> owner,
const query::AuthChecker *auth_checker); const query::AuthChecker *auth_checker);
@ -81,14 +81,13 @@ struct TriggerStore {
explicit TriggerStore(std::filesystem::path directory); explicit TriggerStore(std::filesystem::path directory);
void RestoreTriggers(utils::SkipList<QueryCacheEntry> *query_cache, DbAccessor *db_accessor, void RestoreTriggers(utils::SkipList<QueryCacheEntry> *query_cache, DbAccessor *db_accessor,
utils::SpinLock *antlr_lock, const InterpreterConfig::Query &query_config, const InterpreterConfig::Query &query_config, const query::AuthChecker *auth_checker);
const query::AuthChecker *auth_checker);
void AddTrigger(std::string name, const std::string &query, void AddTrigger(std::string name, const std::string &query,
const std::map<std::string, storage::PropertyValue> &user_parameters, TriggerEventType event_type, const std::map<std::string, storage::PropertyValue> &user_parameters, TriggerEventType event_type,
TriggerPhase phase, utils::SkipList<QueryCacheEntry> *query_cache, DbAccessor *db_accessor, TriggerPhase phase, utils::SkipList<QueryCacheEntry> *query_cache, DbAccessor *db_accessor,
utils::SpinLock *antlr_lock, const InterpreterConfig::Query &query_config, const InterpreterConfig::Query &query_config, std::optional<std::string> owner,
std::optional<std::string> owner, const query::AuthChecker *auth_checker); const query::AuthChecker *auth_checker);
void DropTrigger(const std::string &name); void DropTrigger(const std::string &name);

View File

@ -20,6 +20,15 @@
#include "utils/signals.hpp" #include "utils/signals.hpp"
#include "utils/stacktrace.hpp" #include "utils/stacktrace.hpp"
// This test was introduced because Antlr Cpp runtime doesn't work well in a
// highly concurrent environment. Interpreter `interpret.hpp` contains
// `antlr_lock` used to avoid crashes.
// v4.6 and before -> Crashes.
// v4.8 -> Does NOT crash but sometimes this tests does NOT finish.
// Looks like a deadlock. -> The lock is still REQUIRED.
// v4.9 -> Seems to be working.
// v4.10 -> Seems to be working as well. -> antlr_lock removed
using namespace std::chrono_literals; using namespace std::chrono_literals;
TEST(Antlr, Sigsegv) { TEST(Antlr, Sigsegv) {

View File

@ -891,7 +891,6 @@ class TriggerStoreTest : public ::testing::Test {
std::optional<memgraph::query::DbAccessor> dba; std::optional<memgraph::query::DbAccessor> dba;
memgraph::utils::SkipList<memgraph::query::QueryCacheEntry> ast_cache; memgraph::utils::SkipList<memgraph::query::QueryCacheEntry> ast_cache;
memgraph::utils::SpinLock antlr_lock;
memgraph::query::AllowEverythingAuthChecker auth_checker; memgraph::query::AllowEverythingAuthChecker auth_checker;
private: private:
@ -909,7 +908,7 @@ TEST_F(TriggerStoreTest, Restore) {
const auto reset_store = [&] { const auto reset_store = [&] {
store.emplace(testing_directory); store.emplace(testing_directory);
store->RestoreTriggers(&ast_cache, &*dba, &antlr_lock, memgraph::query::InterpreterConfig::Query{}, &auth_checker); store->RestoreTriggers(&ast_cache, &*dba, memgraph::query::InterpreterConfig::Query{}, &auth_checker);
}; };
reset_store(); reset_store();
@ -930,12 +929,12 @@ TEST_F(TriggerStoreTest, Restore) {
store->AddTrigger( store->AddTrigger(
trigger_name_before, trigger_statement, trigger_name_before, trigger_statement,
std::map<std::string, memgraph::storage::PropertyValue>{{"parameter", memgraph::storage::PropertyValue{1}}}, std::map<std::string, memgraph::storage::PropertyValue>{{"parameter", memgraph::storage::PropertyValue{1}}},
event_type, memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba, &antlr_lock, event_type, memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba,
memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker); memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker);
store->AddTrigger( store->AddTrigger(
trigger_name_after, trigger_statement, trigger_name_after, trigger_statement,
std::map<std::string, memgraph::storage::PropertyValue>{{"parameter", memgraph::storage::PropertyValue{"value"}}}, std::map<std::string, memgraph::storage::PropertyValue>{{"parameter", memgraph::storage::PropertyValue{"value"}}},
event_type, memgraph::query::TriggerPhase::AFTER_COMMIT, &ast_cache, &*dba, &antlr_lock, event_type, memgraph::query::TriggerPhase::AFTER_COMMIT, &ast_cache, &*dba,
memgraph::query::InterpreterConfig::Query{}, {owner}, &auth_checker); memgraph::query::InterpreterConfig::Query{}, {owner}, &auth_checker);
const auto check_triggers = [&] { const auto check_triggers = [&] {
@ -986,16 +985,16 @@ TEST_F(TriggerStoreTest, AddTrigger) {
// Invalid query in statements // Invalid query in statements
ASSERT_THROW(store.AddTrigger("trigger", "RETUR 1", {}, memgraph::query::TriggerEventType::VERTEX_CREATE, ASSERT_THROW(store.AddTrigger("trigger", "RETUR 1", {}, memgraph::query::TriggerEventType::VERTEX_CREATE,
memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba, &antlr_lock, memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba,
memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker), memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker),
memgraph::utils::BasicException); memgraph::utils::BasicException);
ASSERT_THROW(store.AddTrigger("trigger", "RETURN createdEdges", {}, memgraph::query::TriggerEventType::VERTEX_CREATE, ASSERT_THROW(store.AddTrigger("trigger", "RETURN createdEdges", {}, memgraph::query::TriggerEventType::VERTEX_CREATE,
memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba, &antlr_lock, memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba,
memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker), memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker),
memgraph::utils::BasicException); memgraph::utils::BasicException);
ASSERT_THROW(store.AddTrigger("trigger", "RETURN $parameter", {}, memgraph::query::TriggerEventType::VERTEX_CREATE, ASSERT_THROW(store.AddTrigger("trigger", "RETURN $parameter", {}, memgraph::query::TriggerEventType::VERTEX_CREATE,
memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba, &antlr_lock, memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba,
memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker), memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker),
memgraph::utils::BasicException); memgraph::utils::BasicException);
@ -1003,15 +1002,15 @@ TEST_F(TriggerStoreTest, AddTrigger) {
"trigger", "RETURN $parameter", "trigger", "RETURN $parameter",
std::map<std::string, memgraph::storage::PropertyValue>{{"parameter", memgraph::storage::PropertyValue{1}}}, std::map<std::string, memgraph::storage::PropertyValue>{{"parameter", memgraph::storage::PropertyValue{1}}},
memgraph::query::TriggerEventType::VERTEX_CREATE, memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba, memgraph::query::TriggerEventType::VERTEX_CREATE, memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba,
&antlr_lock, memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker)); memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker));
// Inserting with the same name // Inserting with the same name
ASSERT_THROW(store.AddTrigger("trigger", "RETURN 1", {}, memgraph::query::TriggerEventType::VERTEX_CREATE, ASSERT_THROW(store.AddTrigger("trigger", "RETURN 1", {}, memgraph::query::TriggerEventType::VERTEX_CREATE,
memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba, &antlr_lock, memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba,
memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker), memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker),
memgraph::utils::BasicException); memgraph::utils::BasicException);
ASSERT_THROW(store.AddTrigger("trigger", "RETURN 1", {}, memgraph::query::TriggerEventType::VERTEX_CREATE, ASSERT_THROW(store.AddTrigger("trigger", "RETURN 1", {}, memgraph::query::TriggerEventType::VERTEX_CREATE,
memgraph::query::TriggerPhase::AFTER_COMMIT, &ast_cache, &*dba, &antlr_lock, memgraph::query::TriggerPhase::AFTER_COMMIT, &ast_cache, &*dba,
memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker), memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker),
memgraph::utils::BasicException); memgraph::utils::BasicException);
@ -1027,7 +1026,7 @@ TEST_F(TriggerStoreTest, DropTrigger) {
const auto *trigger_name = "trigger"; const auto *trigger_name = "trigger";
store.AddTrigger(trigger_name, "RETURN 1", {}, memgraph::query::TriggerEventType::VERTEX_CREATE, store.AddTrigger(trigger_name, "RETURN 1", {}, memgraph::query::TriggerEventType::VERTEX_CREATE,
memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba, &antlr_lock, memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba,
memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker); memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker);
ASSERT_THROW(store.DropTrigger("Unknown"), memgraph::utils::BasicException); ASSERT_THROW(store.DropTrigger("Unknown"), memgraph::utils::BasicException);
@ -1040,7 +1039,7 @@ TEST_F(TriggerStoreTest, TriggerInfo) {
std::vector<memgraph::query::TriggerStore::TriggerInfo> expected_info; std::vector<memgraph::query::TriggerStore::TriggerInfo> expected_info;
store.AddTrigger("trigger", "RETURN 1", {}, memgraph::query::TriggerEventType::VERTEX_CREATE, store.AddTrigger("trigger", "RETURN 1", {}, memgraph::query::TriggerEventType::VERTEX_CREATE,
memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba, &antlr_lock, memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba,
memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker); memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker);
expected_info.push_back({"trigger", "RETURN 1", memgraph::query::TriggerEventType::VERTEX_CREATE, expected_info.push_back({"trigger", "RETURN 1", memgraph::query::TriggerEventType::VERTEX_CREATE,
memgraph::query::TriggerPhase::BEFORE_COMMIT}); memgraph::query::TriggerPhase::BEFORE_COMMIT});
@ -1060,7 +1059,7 @@ TEST_F(TriggerStoreTest, TriggerInfo) {
check_trigger_info(); check_trigger_info();
store.AddTrigger("edge_update_trigger", "RETURN 1", {}, memgraph::query::TriggerEventType::EDGE_UPDATE, store.AddTrigger("edge_update_trigger", "RETURN 1", {}, memgraph::query::TriggerEventType::EDGE_UPDATE,
memgraph::query::TriggerPhase::AFTER_COMMIT, &ast_cache, &*dba, &antlr_lock, memgraph::query::TriggerPhase::AFTER_COMMIT, &ast_cache, &*dba,
memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker); memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker);
expected_info.push_back({"edge_update_trigger", "RETURN 1", memgraph::query::TriggerEventType::EDGE_UPDATE, expected_info.push_back({"edge_update_trigger", "RETURN 1", memgraph::query::TriggerEventType::EDGE_UPDATE,
memgraph::query::TriggerPhase::AFTER_COMMIT}); memgraph::query::TriggerPhase::AFTER_COMMIT});
@ -1174,7 +1173,7 @@ TEST_F(TriggerStoreTest, AnyTriggerAllKeywords) {
for (const auto keyword : keywords) { for (const auto keyword : keywords) {
SCOPED_TRACE(keyword); SCOPED_TRACE(keyword);
EXPECT_NO_THROW(store.AddTrigger(trigger_name, fmt::format("RETURN {}", keyword), {}, event_type, EXPECT_NO_THROW(store.AddTrigger(trigger_name, fmt::format("RETURN {}", keyword), {}, event_type,
memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba, &antlr_lock, memgraph::query::TriggerPhase::BEFORE_COMMIT, &ast_cache, &*dba,
memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker)); memgraph::query::InterpreterConfig::Query{}, std::nullopt, &auth_checker));
store.DropTrigger(trigger_name); store.DropTrigger(trigger_name);
} }
@ -1199,23 +1198,23 @@ TEST_F(TriggerStoreTest, AuthCheckerUsage) {
ASSERT_NO_THROW(store->AddTrigger("successfull_trigger_1", "CREATE (n:VERTEX) RETURN n", {}, ASSERT_NO_THROW(store->AddTrigger("successfull_trigger_1", "CREATE (n:VERTEX) RETURN n", {},
memgraph::query::TriggerEventType::EDGE_UPDATE, memgraph::query::TriggerEventType::EDGE_UPDATE,
memgraph::query::TriggerPhase::AFTER_COMMIT, &ast_cache, &*dba, &antlr_lock, memgraph::query::TriggerPhase::AFTER_COMMIT, &ast_cache, &*dba,
memgraph::query::InterpreterConfig::Query{}, std::nullopt, &mock_checker)); memgraph::query::InterpreterConfig::Query{}, std::nullopt, &mock_checker));
ASSERT_NO_THROW(store->AddTrigger("successfull_trigger_2", "CREATE (n:VERTEX) RETURN n", {}, ASSERT_NO_THROW(store->AddTrigger("successfull_trigger_2", "CREATE (n:VERTEX) RETURN n", {},
memgraph::query::TriggerEventType::EDGE_UPDATE, memgraph::query::TriggerEventType::EDGE_UPDATE,
memgraph::query::TriggerPhase::AFTER_COMMIT, &ast_cache, &*dba, &antlr_lock, memgraph::query::TriggerPhase::AFTER_COMMIT, &ast_cache, &*dba,
memgraph::query::InterpreterConfig::Query{}, owner, &mock_checker)); memgraph::query::InterpreterConfig::Query{}, owner, &mock_checker));
EXPECT_CALL(mock_checker, IsUserAuthorized(std::optional<std::string>{}, ElementsAre(Privilege::MATCH))) EXPECT_CALL(mock_checker, IsUserAuthorized(std::optional<std::string>{}, ElementsAre(Privilege::MATCH)))
.Times(1) .Times(1)
.WillOnce(Return(false)); .WillOnce(Return(false));
ASSERT_THROW(store->AddTrigger("unprivileged_trigger", "MATCH (n:VERTEX) RETURN n", {}, ASSERT_THROW(
memgraph::query::TriggerEventType::EDGE_UPDATE, store->AddTrigger("unprivileged_trigger", "MATCH (n:VERTEX) RETURN n", {},
memgraph::query::TriggerPhase::AFTER_COMMIT, &ast_cache, &*dba, &antlr_lock, memgraph::query::TriggerEventType::EDGE_UPDATE, memgraph::query::TriggerPhase::AFTER_COMMIT,
memgraph::query::InterpreterConfig::Query{}, std::nullopt, &mock_checker); &ast_cache, &*dba, memgraph::query::InterpreterConfig::Query{}, std::nullopt, &mock_checker);
, memgraph::utils::BasicException); , memgraph::utils::BasicException);
store.emplace(testing_directory); store.emplace(testing_directory);
EXPECT_CALL(mock_checker, IsUserAuthorized(std::optional<std::string>{}, ElementsAre(Privilege::CREATE))) EXPECT_CALL(mock_checker, IsUserAuthorized(std::optional<std::string>{}, ElementsAre(Privilege::CREATE)))
@ -1223,8 +1222,8 @@ TEST_F(TriggerStoreTest, AuthCheckerUsage) {
.WillOnce(Return(false)); .WillOnce(Return(false));
EXPECT_CALL(mock_checker, IsUserAuthorized(owner, ElementsAre(Privilege::CREATE))).Times(1).WillOnce(Return(true)); EXPECT_CALL(mock_checker, IsUserAuthorized(owner, ElementsAre(Privilege::CREATE))).Times(1).WillOnce(Return(true));
ASSERT_NO_THROW(store->RestoreTriggers(&ast_cache, &*dba, &antlr_lock, memgraph::query::InterpreterConfig::Query{}, ASSERT_NO_THROW(
&mock_checker)); store->RestoreTriggers(&ast_cache, &*dba, memgraph::query::InterpreterConfig::Query{}, &mock_checker));
const auto triggers = store->GetTriggerInfo(); const auto triggers = store->GetTriggerInfo();
ASSERT_EQ(triggers.size(), 1); ASSERT_EQ(triggers.size(), 1);