Apply a few more updates

In and only in RelWithDebInfo mode the access of the maybe_unused
variable results in segfaults, this change is making sure that does no
happen ever if the maybe_unused variable is nullopt without changing the
overall logic.

.github/workflows/release_debian10.yaml

@@ -178,7 +178,7 @@ jobs:
 
   release_build:
     name: "Release build"
-    runs-on: [self-hosted, Linux, X64, Debian10]
+    runs-on: [self-hosted, Linux, X64, Debian10, BigMemory]
     env:
       THREADS: 24
       MEMGRAPH_ENTERPRISE_LICENSE: ${{ secrets.MEMGRAPH_ENTERPRISE_LICENSE }}

licenses/BSL.txt

@@ -36,7 +36,7 @@ ADDITIONAL USE GRANT:   You may use the Licensed Work in accordance with the
                         3. using the Licensed Work to create a work or solution
                         which competes (or might reasonably be expected to
                         compete) with the Licensed Work.
-CHANGE DATE:            2027-30-10
+CHANGE DATE:            2027-08-12
 CHANGE LICENSE:         Apache License, Version 2.0
 
 For information about alternative licensing arrangements, please visit: https://memgraph.com/legal.

query_modules/schema.cpp

@@ -108,31 +108,83 @@ void Schema::ProcessPropertiesRel(mgp::Record &record, const std::string_view &t
   record.Insert(std::string(kReturnMandatory).c_str(), mandatory);
 }
 
+struct Property {
+  std::string name;
+  mgp::Value value;
+
+  Property(const std::string &name, mgp::Value &&value) : name(name), value(std::move(value)) {}
+};
+
+struct LabelsHash {
+  std::size_t operator()(const std::set<std::string> &set) const {
+    std::size_t seed = set.size();
+    for (const auto &i : set) {
+      seed ^= std::hash<std::string>{}(i) + 0x9e3779b9 + (seed << 6) + (seed >> 2);
+    }
+    return seed;
+  }
+};
+
+struct LabelsComparator {
+  bool operator()(const std::set<std::string> &lhs, const std::set<std::string> &rhs) const { return lhs == rhs; }
+};
+
+struct PropertyComparator {
+  bool operator()(const Property &lhs, const Property &rhs) const { return lhs.name < rhs.name; }
+};
+
+struct PropertyInfo {
+  std::set<Property, PropertyComparator> properties;
+  bool mandatory;
+};
+
 void Schema::NodeTypeProperties(mgp_list * /*args*/, mgp_graph *memgraph_graph, mgp_result *result,
                                 mgp_memory *memory) {
   mgp::MemoryDispatcherGuard guard{memory};
   const auto record_factory = mgp::RecordFactory(result);
   try {
-    const mgp::Graph graph = mgp::Graph(memgraph_graph);
-    for (auto node : graph.Nodes()) {
-      std::string type;
-      mgp::List labels = mgp::List();
+    std::unordered_map<std::set<std::string>, PropertyInfo, LabelsHash, LabelsComparator> node_types_properties;
+
+    for (auto node : mgp::Graph(memgraph_graph).Nodes()) {
+      std::set<std::string> labels_set = {};
       for (auto label : node.Labels()) {
-        labels.AppendExtend(mgp::Value(label));
-        type += ":`" + std::string(label) + "`";
+        labels_set.emplace(label);
+      }
+
+      if (node_types_properties.find(labels_set) == node_types_properties.end()) {
+        node_types_properties[labels_set] = PropertyInfo{std::set<Property, PropertyComparator>(), true};
       }
 
       if (node.Properties().empty()) {
-        auto record = record_factory.NewRecord();
-        ProcessPropertiesNode<std::string>(record, type, labels, "", "", false);
+        node_types_properties[labels_set].mandatory = false;  // if there is node with no property, it is not mandatory
         continue;
       }
 
+      auto &property_info = node_types_properties.at(labels_set);
       for (auto &[key, prop] : node.Properties()) {
-        auto property_type = mgp::List();
+        property_info.properties.emplace(key, std::move(prop));
+        if (property_info.mandatory) {
+          property_info.mandatory =
+              property_info.properties.size() == 1;  // if there is only one property, it is mandatory
+        }
+      }
+    }
+
+    for (auto &[labels, property_info] : node_types_properties) {
+      std::string label_type;
+      mgp::List labels_list = mgp::List();
+      for (auto const &label : labels) {
+        label_type += ":`" + std::string(label) + "`";
+        labels_list.AppendExtend(mgp::Value(label));
+      }
+      for (auto const &prop : property_info.properties) {
         auto record = record_factory.NewRecord();
-        property_type.AppendExtend(mgp::Value(TypeOf(prop.Type())));
-        ProcessPropertiesNode<mgp::List>(record, type, labels, key, property_type, true);
+        ProcessPropertiesNode(record, label_type, labels_list, prop.name, TypeOf(prop.value.Type()),
+                              property_info.mandatory);
+      }
+      if (property_info.properties.empty()) {
+        auto record = record_factory.NewRecord();
+        ProcessPropertiesNode<std::string>(record, label_type, labels_list, "", "", false);
       }
     }
 
@@ -144,23 +196,41 @@ void Schema::NodeTypeProperties(mgp_list * /*args*/, mgp_graph *memgraph_graph,
 
 void Schema::RelTypeProperties(mgp_list * /*args*/, mgp_graph *memgraph_graph, mgp_result *result, mgp_memory *memory) {
   mgp::MemoryDispatcherGuard guard{memory};
+
+  std::unordered_map<std::string, PropertyInfo> rel_types_properties;
   const auto record_factory = mgp::RecordFactory(result);
   try {
     const mgp::Graph graph = mgp::Graph(memgraph_graph);
-
     for (auto rel : graph.Relationships()) {
-      std::string type = ":`" + std::string(rel.Type()) + "`";
+      std::string rel_type = std::string(rel.Type());
+      if (rel_types_properties.find(rel_type) == rel_types_properties.end()) {
+        rel_types_properties[rel_type] = PropertyInfo{std::set<Property, PropertyComparator>(), true};
+      }
+
       if (rel.Properties().empty()) {
-        auto record = record_factory.NewRecord();
-        ProcessPropertiesRel<std::string>(record, type, "", "", false);
+        rel_types_properties[rel_type].mandatory = false;  // if there is rel with no property, it is not mandatory
         continue;
       }
 
+      auto &property_info = rel_types_properties.at(rel_type);
       for (auto &[key, prop] : rel.Properties()) {
-        auto property_type = mgp::List();
+        property_info.properties.emplace(key, std::move(prop));
+        if (property_info.mandatory) {
+          property_info.mandatory =
+              property_info.properties.size() == 1;  // if there is only one property, it is mandatory
+        }
+      }
+    }
+
+    for (auto &[type, property_info] : rel_types_properties) {
+      std::string type_str = ":`" + std::string(type) + "`";
+      for (auto const &prop : property_info.properties) {
         auto record = record_factory.NewRecord();
-        property_type.AppendExtend(mgp::Value(TypeOf(prop.Type())));
-        ProcessPropertiesRel<mgp::List>(record, type, key, property_type, true);
+        ProcessPropertiesRel(record, type_str, prop.name, TypeOf(prop.value.Type()), property_info.mandatory);
+      }
+      if (property_info.properties.empty()) {
+        auto record = record_factory.NewRecord();
+        ProcessPropertiesRel<std::string>(record, type_str, "", "", false);
       }
     }
 

src/query/frontend/ast/cypher_main_visitor.cpp

@@ -1276,28 +1276,59 @@ antlrcpp::Any CypherMainVisitor::visitCallProcedure(MemgraphCypher::CallProcedur
       call_proc->result_identifiers_.push_back(storage_->Create<Identifier>(result_alias));
     }
   } else {
-    const auto &maybe_found =
-        procedure::FindProcedure(procedure::gModuleRegistry, call_proc->procedure_name_, utils::NewDeleteResource());
-    if (!maybe_found) {
-      throw SemanticException("There is no procedure named '{}'.", call_proc->procedure_name_);
+    call_proc->is_write_ = maybe_found->second->info.is_write;
+
+    auto *yield_ctx = ctx->yieldProcedureResults();
+    if (!yield_ctx) {
+      if (!maybe_found->second->results.empty() && !call_proc->void_procedure_) {
+        throw SemanticException(
+            "CALL without YIELD may only be used on procedures which do not "
+            "return any result fields.");
+      }
+      // When we return, we will release the lock on modules. This means that
+      // someone may reload the procedure and change the result signature. But to
+      // keep the implementation simple, we ignore the case as the rest of the
+      // code doesn't really care whether we yield or not, so it should not break.
+      return call_proc;
     }
-    const auto &[module, proc] = *maybe_found;
-    call_proc->result_fields_.reserve(proc->results.size());
-    call_proc->result_identifiers_.reserve(proc->results.size());
-    for (const auto &[result_name, desc] : proc->results) {
-      bool is_deprecated = desc.second;
-      if (is_deprecated) continue;
-      call_proc->result_fields_.emplace_back(result_name);
-      call_proc->result_identifiers_.push_back(storage_->Create<Identifier>(std::string(result_name)));
+    if (yield_ctx->getTokens(MemgraphCypher::ASTERISK).empty()) {
+      call_proc->result_fields_.reserve(yield_ctx->procedureResult().size());
+      call_proc->result_identifiers_.reserve(yield_ctx->procedureResult().size());
+      for (auto *result : yield_ctx->procedureResult()) {
+        MG_ASSERT(result->variable().size() == 1 || result->variable().size() == 2);
+        call_proc->result_fields_.push_back(std::any_cast<std::string>(result->variable()[0]->accept(this)));
+        std::string result_alias;
+        if (result->variable().size() == 2) {
+          result_alias = std::any_cast<std::string>(result->variable()[1]->accept(this));
+        } else {
+          result_alias = std::any_cast<std::string>(result->variable()[0]->accept(this));
+        }
+        call_proc->result_identifiers_.push_back(storage_->Create<Identifier>(result_alias));
+      }
+    } else {
+      const auto &maybe_found =
+          procedure::FindProcedure(procedure::gModuleRegistry, call_proc->procedure_name_, utils::NewDeleteResource());
+      if (!maybe_found) {
+        throw SemanticException("There is no procedure named '{}'.", call_proc->procedure_name_);
+      }
+      const auto &[module, proc] = *maybe_found;
+      call_proc->result_fields_.reserve(proc->results.size());
+      call_proc->result_identifiers_.reserve(proc->results.size());
+      for (const auto &[result_name, desc] : proc->results) {
+        bool is_deprecated = desc.second;
+        if (is_deprecated) continue;
+        call_proc->result_fields_.emplace_back(result_name);
+        call_proc->result_identifiers_.push_back(storage_->Create<Identifier>(std::string(result_name)));
+      }
+      // When we leave the scope, we will release the lock on modules. This means
+      // that someone may reload the procedure and change its result signature. We
+      // are fine with this, because if new result fields were added then we yield
+      // the subset of those and that will appear to a user as if they used the
+      // procedure before reload. Any subsequent `CALL ... YIELD *` will fetch the
+      // new fields as well. In case the result signature has had some result
+      // fields removed, then the query execution will report an error that we are
+      // yielding missing fields. The user can then just retry the query.
     }
-    // When we leave the scope, we will release the lock on modules. This means
-    // that someone may reload the procedure and change its result signature. We
-    // are fine with this, because if new result fields were added then we yield
-    // the subset of those and that will appear to a user as if they used the
-    // procedure before reload. Any subsequent `CALL ... YIELD *` will fetch the
-    // new fields as well. In case the result signature has had some result
-    // fields removed, then the query execution will report an error that we are
-    // yielding missing fields. The user can then just retry the query.
   }
 
   return call_proc;

tests/drivers/java/v5_8/run.sh

@@ -9,8 +9,8 @@ fi
 if [ -d "/usr/lib/jvm/java-17-openjdk-amd64" ]; then
   export JAVA_HOME="/usr/lib/jvm/java-17-openjdk-amd64"
 fi
-if [ -d "/opt/apache-maven-3.9.2" ]; then
-  export M2_HOME="/opt/apache-maven-3.9.2"
+if [ -d "/opt/apache-maven-3.9.3" ]; then
+  export M2_HOME="/opt/apache-maven-3.9.3"
 fi
 export PATH="$JAVA_HOME/bin:$M2_HOME/bin:$PATH"
 

tests/e2e/interactive_mg_runner.py

@@ -80,6 +80,8 @@ ACTIONS = {
     "quit": lambda _: sys.exit(1),
 }
 
+CLEANUP_DIRECTORIES_ON_EXIT = False
+
 log = logging.getLogger("memgraph.tests.e2e")
 
 
@@ -109,10 +111,11 @@ def _start_instance(name, args, log_file, setup_queries, use_ssl, procdir, data_
     assert not is_port_in_use(
         extract_bolt_port(args)
     ), "If this raises, you are trying to start an instance on a port already used by one already running instance."
-    mg_instance = MemgraphInstanceRunner(MEMGRAPH_BINARY, use_ssl)
-    MEMGRAPH_INSTANCES[name] = mg_instance
+
     log_file_path = os.path.join(BUILD_DIR, "logs", log_file)
     data_directory_path = os.path.join(BUILD_DIR, data_directory)
+    mg_instance = MemgraphInstanceRunner(MEMGRAPH_BINARY, use_ssl, {data_directory_path})
+    MEMGRAPH_INSTANCES[name] = mg_instance
     binary_args = args + ["--log-file", log_file_path] + ["--data-directory", data_directory_path]
 
     if len(procdir) != 0:
@@ -122,39 +125,43 @@ def _start_instance(name, args, log_file, setup_queries, use_ssl, procdir, data_
     assert mg_instance.is_running(), "An error occured after starting Memgraph instance: application stopped running."
 
 
-def stop_all():
+def stop_all(keep_directories=True):
     for mg_instance in MEMGRAPH_INSTANCES.values():
-        mg_instance.stop()
+        mg_instance.stop(keep_directories)
     MEMGRAPH_INSTANCES.clear()
 
 
-def stop_instance(context, name):
+def stop_instance(context, name, keep_directories=True):
     for key, _ in context.items():
         if key != name:
             continue
-        MEMGRAPH_INSTANCES[name].stop()
+        MEMGRAPH_INSTANCES[name].stop(keep_directories)
         MEMGRAPH_INSTANCES.pop(name)
 
 
-def stop(context, name):
+def stop(context, name, keep_directories=True):
     if name != "all":
-        stop_instance(context, name)
+        stop_instance(context, name, keep_directories)
         return
 
     stop_all()
 
 
-def kill(context, name):
+def kill(context, name, keep_directories=True):
     for key in context.keys():
         if key != name:
             continue
-        MEMGRAPH_INSTANCES[name].kill()
+        MEMGRAPH_INSTANCES[name].kill(keep_directories)
         MEMGRAPH_INSTANCES.pop(name)
 
 
+def cleanup_directories_on_exit(value=True):
+    CLEANUP_DIRECTORIES_ON_EXIT = value
+
+
 @atexit.register
 def cleanup():
-    stop_all()
+    stop_all(CLEANUP_DIRECTORIES_ON_EXIT)
 
 
 def start_instance(context, name, procdir):
@@ -184,8 +191,8 @@ def start_instance(context, name, procdir):
     assert len(mg_instances) == 1
 
 
-def start_all(context, procdir=""):
-    stop_all()
+def start_all(context, procdir="", keep_directories=True):
+    stop_all(keep_directories)
     for key, _ in context.items():
         start_instance(context, key, procdir)
 

tests/e2e/memgraph.py

@@ -11,6 +11,7 @@
 
 import copy
 import os
+import shutil
 import subprocess
 import sys
 import time
@@ -56,13 +57,14 @@ def replace_paths(path):
 
 
 class MemgraphInstanceRunner:
-    def __init__(self, binary_path=MEMGRAPH_BINARY, use_ssl=False):
+    def __init__(self, binary_path=MEMGRAPH_BINARY, use_ssl=False, delete_on_stop=None):
         self.host = "127.0.0.1"
         self.bolt_port = None
         self.binary_path = binary_path
         self.args = None
         self.proc_mg = None
         self.ssl = use_ssl
+        self.delete_on_stop = delete_on_stop
 
     def execute_setup_queries(self, setup_queries):
         if setup_queries is None:
@@ -128,7 +130,7 @@ class MemgraphInstanceRunner:
             return False
         return True
 
-    def stop(self):
+    def stop(self, keep_directories=False):
         if not self.is_running():
             return
 
@@ -140,9 +142,16 @@ class MemgraphInstanceRunner:
 
         time.sleep(1)
 
-    def kill(self):
+        if not keep_directories:
+            for folder in self.delete_on_stop or {}:
+                shutil.rmtree(folder)
+
+    def kill(self, keep_directories=False):
         if not self.is_running():
             return
         self.proc_mg.kill()
         code = self.proc_mg.wait()
+        if not keep_directories:
+            for folder in self.delete_on_stop or {}:
+                shutil.rmtree(folder)
         assert code == -9, "The killed Memgraph process exited with non-nine!"

tests/e2e/query_modules/schema_test.py

@@ -431,7 +431,7 @@ def test_node_type_properties1():
             f"CALL libschema.node_type_properties() YIELD nodeType, nodeLabels, propertyName, propertyTypes , mandatory RETURN nodeType, nodeLabels, propertyName, propertyTypes , mandatory ORDER BY propertyName, nodeLabels[0];",
         )[0]
     )
-    assert (result) == [":`Activity`", ["Activity"], "location", ["String"], True]
+    assert (result) == [":`Activity`", ["Activity"], "location", "String", False]
 
     result = list(
         execute_and_fetch_all(
@@ -439,7 +439,7 @@ def test_node_type_properties1():
             f"CALL libschema.node_type_properties() YIELD nodeType, nodeLabels, propertyName, propertyTypes , mandatory RETURN nodeType, nodeLabels, propertyName, propertyTypes , mandatory ORDER BY propertyName, nodeLabels[0];",
         )[1]
     )
-    assert (result) == [":`Activity`", ["Activity"], "name", ["String"], True]
+    assert (result) == [":`Activity`", ["Activity"], "name", "String", False]
 
     result = list(
         execute_and_fetch_all(
@@ -447,7 +447,7 @@ def test_node_type_properties1():
             f"CALL libschema.node_type_properties() YIELD nodeType, nodeLabels, propertyName, propertyTypes , mandatory RETURN nodeType, nodeLabels, propertyName, propertyTypes , mandatory ORDER BY propertyName, nodeLabels[0];",
         )[2]
     )
-    assert (result) == [":`Dog`", ["Dog"], "name", ["String"], True]
+    assert (result) == [":`Dog`", ["Dog"], "name", "String", False]
 
     result = list(
         execute_and_fetch_all(
@@ -455,7 +455,81 @@ def test_node_type_properties1():
             f"CALL libschema.node_type_properties() YIELD nodeType, nodeLabels, propertyName, propertyTypes , mandatory RETURN nodeType, nodeLabels, propertyName, propertyTypes , mandatory ORDER BY propertyName, nodeLabels[0];",
         )[3]
     )
-    assert (result) == [":`Dog`", ["Dog"], "owner", ["String"], True]
+    assert (result) == [":`Dog`", ["Dog"], "owner", "String", False]
+
+
+def test_node_type_properties2():
+    cursor = connect().cursor()
+    execute_and_fetch_all(
+        cursor,
+        """
+        CREATE (d:MyNode)
+        CREATE (n:MyNode)
+        """,
+    )
+    result = execute_and_fetch_all(
+        cursor,
+        f"CALL libschema.node_type_properties() YIELD nodeType, nodeLabels, propertyName, propertyTypes , mandatory RETURN nodeType, nodeLabels, propertyName, propertyTypes , mandatory ORDER BY propertyName, nodeLabels[0];",
+    )
+    assert (list(result[0])) == [":`MyNode`", ["MyNode"], "", "", False]
+    assert (result.__len__()) == 1
+
+
+def test_node_type_properties3():
+    cursor = connect().cursor()
+    execute_and_fetch_all(
+        cursor,
+        """
+        CREATE (d:Dog {name: 'Rex', owner: 'Carl'})
+        CREATE (n:Dog)
+        """,
+    )
+    result = execute_and_fetch_all(
+        cursor,
+        f"CALL libschema.node_type_properties() YIELD nodeType, nodeLabels, propertyName, propertyTypes , mandatory RETURN nodeType, nodeLabels, propertyName, propertyTypes , mandatory ORDER BY propertyName, nodeLabels[0];",
+    )
+
+    assert (list(result[0])) == [":`Dog`", ["Dog"], "name", "String", False]
+    assert (list(result[1])) == [":`Dog`", ["Dog"], "owner", "String", False]
+    assert (result.__len__()) == 2
+
+
+def test_node_type_properties4():
+    cursor = connect().cursor()
+    execute_and_fetch_all(
+        cursor,
+        """
+        CREATE (n:Label1:Label2 {property1: 'value1', property2: 'value2'})
+        CREATE (m:Label2:Label1 {property3: 'value3'})
+        """,
+    )
+    result = list(
+        execute_and_fetch_all(
+            cursor,
+            f"CALL libschema.node_type_properties() YIELD nodeType, nodeLabels, propertyName, propertyTypes , mandatory RETURN nodeType, nodeLabels, propertyName, propertyTypes , mandatory ORDER BY propertyName, nodeLabels[0];",
+        )
+    )
+    assert (list(result[0])) == [":`Label1`:`Label2`", ["Label1", "Label2"], "property1", "String", False]
+    assert (list(result[1])) == [":`Label1`:`Label2`", ["Label1", "Label2"], "property2", "String", False]
+    assert (list(result[2])) == [":`Label1`:`Label2`", ["Label1", "Label2"], "property3", "String", False]
+    assert (result.__len__()) == 3
+
+
+def test_node_type_properties5():
+    cursor = connect().cursor()
+    execute_and_fetch_all(
+        cursor,
+        """
+        CREATE (d:Dog {name: 'Rex'})
+        """,
+    )
+    result = execute_and_fetch_all(
+        cursor,
+        f"CALL libschema.node_type_properties() YIELD nodeType, nodeLabels, propertyName, propertyTypes , mandatory RETURN nodeType, nodeLabels, propertyName, propertyTypes , mandatory ORDER BY propertyName, nodeLabels[0];",
+    )
+
+    assert (list(result[0])) == [":`Dog`", ["Dog"], "name", "String", True]
+    assert (result.__len__()) == 1
 
 
 def test_rel_type_properties1():
@@ -473,5 +547,38 @@ def test_rel_type_properties1():
     assert (result) == [":`LOVES`", "", "", False]
 
 
+def test_rel_type_properties2():
+    cursor = connect().cursor()
+    execute_and_fetch_all(
+        cursor,
+        """
+        CREATE (d:Dog {name: 'Rex', owner: 'Carl'})-[l:LOVES]->(a:Activity {name: 'Running', location: 'Zadar'})
+        CREATE (n:Dog {name: 'Simba', owner: 'Lucy'})-[j:LOVES {duration: 30}]->(b:Activity {name: 'Running', location: 'Zadar'})
+        """,
+    )
+    result = execute_and_fetch_all(
+        cursor,
+        f"CALL libschema.rel_type_properties() YIELD relType,propertyName, propertyTypes , mandatory RETURN relType, propertyName, propertyTypes , mandatory;",
+    )
+    assert (list(result[0])) == [":`LOVES`", "duration", "Int", False]
+    assert (result.__len__()) == 1
+
+
+def test_rel_type_properties3():
+    cursor = connect().cursor()
+    execute_and_fetch_all(
+        cursor,
+        """
+        CREATE (n:Dog {name: 'Simba', owner: 'Lucy'})-[j:LOVES {duration: 30}]->(b:Activity {name: 'Running', location: 'Zadar'})
+        """,
+    )
+    result = execute_and_fetch_all(
+        cursor,
+        f"CALL libschema.rel_type_properties() YIELD relType,propertyName, propertyTypes , mandatory RETURN relType, propertyName, propertyTypes , mandatory;",
+    )
+    assert (list(result[0])) == [":`LOVES`", "duration", "Int", True]
+    assert (result.__len__()) == 1
+
+
 if __name__ == "__main__":
     sys.exit(pytest.main([__file__, "-rA"]))

tests/e2e/runner.py

@@ -53,8 +53,8 @@ def run(args):
 
         # Setup.
         @atexit.register
-        def cleanup():
-            interactive_mg_runner.stop_all()
+        def cleanup(keep_directories=True):
+            interactive_mg_runner.stop_all(keep_directories)
 
         if "pre_set_workload" in workload:
             binary = os.path.join(BUILD_DIR, workload["pre_set_workload"])
@@ -92,7 +92,7 @@ def run(args):
                     data = mg_instance.query(validation["query"], conn)[0][0]
                     assert data == validation["expected"]
                 conn.close()
-        cleanup()
+        cleanup(keep_directories=False)
         log.info("%s PASSED.", workload_name)
 
 

tests/mgbench/README.md

@@ -96,7 +96,7 @@ Benchmarking different systems is challenging because the setup, environment, qu
 Listed below are the main scripts used to run the benchmarks:
 
 - `benchmark.py` - The main entry point used for starting and managing the execution of the benchmark. This script initializes all the necessary files, classes, and objects. It starts the database and the benchmark and gathers the results.
-- `base.py` - This is the base workload class. All other workloads are subclasses located in the workloads directory. For example, ldbc_interactive.py defines ldbc interactive dataset and queries (but this is NOT an official LDBC interactive workload). Each workload class can generate the dataset, use custom import ofthe dataset or provide a CYPHERL file for the import process..
+- `base.py` - This is the base workload class. All other workloads are subclasses located in the workloads directory. For example, ldbc_interactive.py defines ldbc interactive dataset and queries (but this is NOT an official LDBC interactive workload). Each workload class can generate the dataset, use custom import of the dataset or provide a CYPHERL file for the import process..
 - `runners.py` - The script that configures, starts, and stops the database.
 - `client.cpp` - Client for querying the database.
 - `graph_bench.py` - Script that starts all tests from Benchgraph.

tests/mgbench/benchmark.py

@@ -239,7 +239,11 @@ def sanitize_args(args):
     assert args.benchmarks != None, helpers.list_available_workloads()
     assert args.num_workers_for_import > 0
     assert args.num_workers_for_benchmark > 0
-    assert args.export_results != None, "Pass where will results be saved"
+    assert (
+        args.export_results != None
+        or args.export_results_on_disk_txn != None
+        or args.export_results_in_memory_analytical != None
+    ), "Pass where will results be saved"
     assert args.single_threaded_runtime_sec >= 1, "Low runtime value, consider extending time for more accurate results"
     assert (
         args.workload_realistic == None or args.workload_mixed == None
@@ -686,25 +690,25 @@ def run_target_workload(benchmark_context, workload, bench_queries, vendor_runne
             run_isolated_workload_with_authorization(vendor_runner, client, bench_queries, group, workload, results)
 
 
-# TODO: (andi) Reorder functions in top-down notion in order to improve readibility
+# TODO: (andi) Reorder functions in top-down notion in order to improve readibility -> does this referes to run_target_workloads function or the code around?
 def run_target_workloads(benchmark_context, target_workloads, bench_results):
     for workload, bench_queries in target_workloads:
         log.info(f"Started running {str(workload.NAME)} workload")
-
         benchmark_context.set_active_workload(workload.NAME)
         benchmark_context.set_active_variant(workload.get_variant())
 
+        # TODO(gitbuda): What's the semantic of --export-results-xyz flags? NOTE: avoid nested if/else statements
         if workload.is_disk_workload() and benchmark_context.export_results_on_disk_txn:
             run_on_disk_transactional_benchmark(benchmark_context, workload, bench_queries, bench_results.disk_results)
-        else:
-            run_in_memory_transactional_benchmark(
-                benchmark_context, workload, bench_queries, bench_results.in_memory_txn_results
+            return
+        if benchmark_context.export_results_in_memory_analytical:
+            run_in_memory_analytical_benchmark(
+                benchmark_context, workload, bench_queries, bench_results.in_memory_analytical_results
             )
-
-            if benchmark_context.export_results_in_memory_analytical:
-                run_in_memory_analytical_benchmark(
-                    benchmark_context, workload, bench_queries, bench_results.in_memory_analytical_results
-                )
+            return
+        run_in_memory_transactional_benchmark(
+            benchmark_context, workload, bench_queries, bench_results.in_memory_txn_results
+        )
 
 
 def run_on_disk_transactional_benchmark(benchmark_context, workload, bench_queries, disk_results):

tests/mgbench/runners.py

@@ -811,7 +811,7 @@ class MemgraphDocker(BaseRunner):
                 "-it",
                 "-p",
                 self._bolt_port + ":" + self._bolt_port,
-                "memgraph/memgraph:2.7.0",
+                "memgraph/memgraph:2.14.0",  # TODO(gitbuda): parametrize & fallback to the latest version.
                 "--storage_wal_enabled=false",
                 "--storage_recover_on_startup=true",
                 "--storage_snapshot_interval_sec",

tests/stress/memory_tracker.py

@@ -260,10 +260,11 @@ def run_monitor_cleanup(repetition_count: int, sleep_sec: float) -> None:
             # Problem with test using detach delete and memory tracker
             # is that memory tracker gets updated immediately
             # whereas RES takes some time
-            cnt_again = 3
+            # Tries 10 times or fails
+            cnt_again = 10
             skip_failure = False
-            # 10% is maximum increment, afterwards is fail
-            multiplier = 1
+            # 10% is maximum diff for this test to pass
+            multiplier = 1.10
             while cnt_again:
                 new_memory_tracker, new_res_data = get_storage_data(session)
 
@@ -277,7 +278,6 @@ def run_monitor_cleanup(repetition_count: int, sleep_sec: float) -> None:
                         f"RES data: {new_res_data}, multiplier: {multiplier}"
                     )
                     break
-                multiplier += 0.05
                 cnt_again -= 1
             if not skip_failure:
                 log.info(memory_tracker, initial_diff, res_data)

tests/unit/dbms_handler.cpp

@@ -58,6 +58,7 @@ class TestEnvironment : public ::testing::Environment {
   void TearDown() override {
     ptr_.reset();
     auth.reset();
+    std::filesystem::remove_all(storage_directory);
   }
 
   static std::unique_ptr<memgraph::dbms::DbmsHandler> ptr_;

tests/unit/dbms_handler_community.cpp

@@ -58,6 +58,7 @@ class TestEnvironment : public ::testing::Environment {
   void TearDown() override {
     ptr_.reset();
     auth.reset();
+    std::filesystem::remove_all(storage_directory);
   }
 
   static std::unique_ptr<memgraph::dbms::DbmsHandler> ptr_;

tests/unit/interpreter.cpp

@@ -101,6 +101,8 @@ class InterpreterTest : public ::testing::Test {
       disk_test_utils::RemoveRocksDbDirs(testSuite);
       disk_test_utils::RemoveRocksDbDirs(testSuiteCsv);
     }
+
+    std::filesystem::remove_all(data_directory);
   }
 
   InterpreterFaker default_interpreter{&interpreter_context, db};

tests/unit/query_dump.cpp

@@ -700,6 +700,11 @@ TYPED_TEST(DumpTest, CheckStateVertexWithMultipleProperties) {
     config.disk = disk_test_utils::GenerateOnDiskConfig("query-dump-s1").disk;
     config.force_on_disk = true;
   }
+  auto on_exit_s1 = memgraph::utils::OnScopeExit{[&]() {
+    if constexpr (std::is_same_v<TypeParam, memgraph::storage::DiskStorage>) {
+      disk_test_utils::RemoveRocksDbDirs("query-dump-s1");
+    }
+  }};
   memgraph::replication::ReplicationState repl_state(ReplicationStateRootPath(config));
 
   memgraph::utils::Gatekeeper<memgraph::dbms::Database> db_gk(config, repl_state);
@@ -814,7 +819,11 @@ TYPED_TEST(DumpTest, CheckStateSimpleGraph) {
     config.disk = disk_test_utils::GenerateOnDiskConfig("query-dump-s2").disk;
     config.force_on_disk = true;
   }
-
+  auto on_exit_s2 = memgraph::utils::OnScopeExit{[&]() {
+    if constexpr (std::is_same_v<TypeParam, memgraph::storage::DiskStorage>) {
+      disk_test_utils::RemoveRocksDbDirs("query-dump-s2");
+    }
+  }};
   memgraph::replication::ReplicationState repl_state{ReplicationStateRootPath(config)};
   memgraph::utils::Gatekeeper<memgraph::dbms::Database> db_gk{config, repl_state};
   auto db_acc_opt = db_gk.access();

tests/unit/storage_v2_isolation_level.cpp

@@ -15,6 +15,7 @@
 #include "storage/v2/disk/storage.hpp"
 #include "storage/v2/inmemory/storage.hpp"
 #include "storage/v2/isolation_level.hpp"
+#include "utils/on_scope_exit.hpp"
 
 namespace {
 int64_t VerticesCount(memgraph::storage::Storage::Accessor *accessor) {
@@ -113,6 +114,7 @@ TEST_P(StorageIsolationLevelTest, VisibilityOnDiskStorage) {
 
   for (const auto override_isolation_level : isolation_levels) {
     std::unique_ptr<memgraph::storage::Storage> storage(new memgraph::storage::DiskStorage(config));
+    auto on_exit = memgraph::utils::OnScopeExit{[&]() { disk_test_utils::RemoveRocksDbDirs(testSuite); }};
     try {
       this->TestVisibility(storage, default_isolation_level, override_isolation_level);
     } catch (memgraph::utils::NotYetImplemented &) {
@@ -120,10 +122,8 @@ TEST_P(StorageIsolationLevelTest, VisibilityOnDiskStorage) {
           override_isolation_level != memgraph::storage::IsolationLevel::SNAPSHOT_ISOLATION) {
         continue;
       }
-      disk_test_utils::RemoveRocksDbDirs(testSuite);
       throw;
     }
-    disk_test_utils::RemoveRocksDbDirs(testSuite);
   }
 }
 

tests/unit/storage_v2_storage_mode.cpp

@@ -75,6 +75,8 @@ class StorageModeMultiTxTest : public ::testing::Test {
     return tmp;
   }();  // iile
 
+  void TearDown() override { std::filesystem::remove_all(data_directory); }
+
   memgraph::storage::Config config{.durability.storage_directory = data_directory,
                                    .disk.main_storage_directory = data_directory / "disk"};