Update pull_request_template.md

* remove default limit on procedures
* fix bug on GraphQL also

.github/pull_request_template.md

@@ -1,13 +1,9 @@
-[master < Epic] PR
-- [ ] Check, and update documentation if necessary
+PR checklist:
+- [ ] Check and update documentation if necessary
 - [ ] Write E2E tests
 - [ ] Compare the [benchmarking results](https://bench-graph.memgraph.com/) between the master branch and the Epic branch
 - [ ] Provide the full content or a guide for the final git message
-
-[master < Task] PR
-- [ ] Check, and update documentation if necessary
-- [ ] Provide the full content or a guide for the final git message
-
+- [ ] Notify everyone downstream (platform, dx, infra, ...) if there is any breaking change and what to do about it
 
 To keep docs changelog up to date, one more thing to do:
 - [ ] Write a release note here, including added/changed clauses

src/query/frontend/ast/cypher_main_visitor.cpp

@@ -1216,10 +1216,6 @@ antlrcpp::Any CypherMainVisitor::visitCallProcedure(MemgraphCypher::CallProcedur
       call_proc->memory_limit_ = memory_limit_info->first;
       call_proc->memory_scale_ = memory_limit_info->second;
     }
-  } else {
-    // Default to 100 MB
-    call_proc->memory_limit_ = storage_->Create<PrimitiveLiteral>(TypedValue(100));
-    call_proc->memory_scale_ = 1024U * 1024U;
   }
 
   const auto &maybe_found =
@@ -1240,11 +1236,13 @@ antlrcpp::Any CypherMainVisitor::visitCallProcedure(MemgraphCypher::CallProcedur
       throw SemanticException("There is no procedure named '{}'.", call_proc->procedure_name_);
     }
   }
-  call_proc->is_write_ = maybe_found->second->info.is_write;
+  if (maybe_found) {
+    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_) {
+    if ((maybe_found && !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.");

src/query/plan/operator.cpp

@@ -3464,7 +3464,7 @@ class AggregateCursor : public Cursor {
     SCOPED_PROFILE_OP_BY_REF(self_);
 
     if (!pulled_all_input_) {
-      if (!ProcessAll(&frame, &context) && self_.AreAllAggregationsForCollecting()) return false;
+      if (!ProcessAll(&frame, &context) && !self_.group_by_.empty()) return false;
       pulled_all_input_ = true;
       aggregation_it_ = aggregation_.begin();
 
@@ -3824,12 +3824,6 @@ UniqueCursorPtr Aggregate::MakeCursor(utils::MemoryResource *mem) const {
   return MakeUniqueCursorPtr<AggregateCursor>(mem, *this, mem);
 }
 
-auto Aggregate::AreAllAggregationsForCollecting() const -> bool {
-  return std::all_of(aggregations_.begin(), aggregations_.end(), [](const auto &agg) {
-    return agg.op == Aggregation::Op::COLLECT_LIST || agg.op == Aggregation::Op::COLLECT_MAP;
-  });
-}
-
 Skip::Skip(const std::shared_ptr<LogicalOperator> &input, Expression *expression)
     : input_(input), expression_(expression) {}
 

src/query/plan/operator.hpp

@@ -1759,8 +1759,6 @@ class Aggregate : public memgraph::query::plan::LogicalOperator {
   Aggregate(const std::shared_ptr<LogicalOperator> &input, const std::vector<Element> &aggregations,
             const std::vector<Expression *> &group_by, const std::vector<Symbol> &remember);
 
-  auto AreAllAggregationsForCollecting() const -> bool;
-
   bool Accept(HierarchicalLogicalOperatorVisitor &visitor) override;
   UniqueCursorPtr MakeCursor(utils::MemoryResource *) const override;
   std::vector<Symbol> ModifiedSymbols(const SymbolTable &) const override;

tests/e2e/garbage_collection/gc_periodic.py

@@ -49,7 +49,7 @@ def test_gc_periodic(connection):
     memory_pre_creation = get_memory(cursor)
     execute_and_fetch_all(cursor, "UNWIND range(1, 1000) AS index CREATE (:Node);")
     memory_after_creation = get_memory(cursor)
-    time.sleep(2)
+    time.sleep(5)
     memory_after_gc = get_memory(cursor)
 
     assert memory_after_gc < memory_pre_creation + (memory_after_creation - memory_pre_creation) / 4 * 3

tests/gql_behave/tests/memgraph_V1/features/aggregations.feature

@@ -425,6 +425,4 @@ Feature: Aggregations
       """
       MATCH (subnet:Subnet) WHERE FALSE WITH subnet, count(subnet.ip) as ips RETURN id(subnet) as id
       """
-      Then the result should be:
-        | id |
-        | null  |
+      Then the result should be empty

tests/gql_behave/tests/memgraph_V1_on_disk/features/aggregations.feature

@@ -425,6 +425,4 @@ Feature: Aggregations
       """
       MATCH (subnet:Subnet) WHERE FALSE WITH subnet, count(subnet.ip) as ips RETURN id(subnet) as id
       """
-      Then the result should be:
-        | id |
-        | null  |
+      Then the result should be empty

tests/unit/cypher_main_visitor.cpp

@@ -2833,18 +2833,6 @@ TEST_P(CypherMainVisitorTest, DumpDatabase) {
   ASSERT_TRUE(query);
 }
 
-namespace {
-template <class TAst>
-void CheckCallProcedureDefaultMemoryLimit(const TAst &ast, const CallProcedure &call_proc) {
-  // Should be 100 MB
-  auto *literal = dynamic_cast<PrimitiveLiteral *>(call_proc.memory_limit_);
-  ASSERT_TRUE(literal);
-  TypedValue value(literal->value_);
-  ASSERT_TRUE(TypedValue::BoolEqual{}(value, TypedValue(100)));
-  ASSERT_EQ(call_proc.memory_scale_, 1024 * 1024);
-}
-}  // namespace
-
 TEST_P(CypherMainVisitorTest, CallProcedureWithDotsInName) {
   AddProc(*mock_module_with_dots_in_name, "proc", {}, {"res"}, ProcedureType::WRITE);
   auto &ast_generator = *GetParam();
@@ -2868,7 +2856,6 @@ TEST_P(CypherMainVisitorTest, CallProcedureWithDotsInName) {
   std::vector<std::string> expected_names{"res"};
   ASSERT_EQ(identifier_names, expected_names);
   ASSERT_EQ(identifier_names, call_proc->result_fields_);
-  CheckCallProcedureDefaultMemoryLimit(ast_generator, *call_proc);
 }
 
 TEST_P(CypherMainVisitorTest, CallProcedureWithDashesInName) {
@@ -2894,7 +2881,6 @@ TEST_P(CypherMainVisitorTest, CallProcedureWithDashesInName) {
   std::vector<std::string> expected_names{"res"};
   ASSERT_EQ(identifier_names, expected_names);
   ASSERT_EQ(identifier_names, call_proc->result_fields_);
-  CheckCallProcedureDefaultMemoryLimit(ast_generator, *call_proc);
 }
 
 TEST_P(CypherMainVisitorTest, CallProcedureWithYieldSomeFields) {
@@ -2926,7 +2912,6 @@ TEST_P(CypherMainVisitorTest, CallProcedureWithYieldSomeFields) {
     std::vector<std::string> expected_names{"fst", "field-with-dashes", "last_field"};
     ASSERT_EQ(identifier_names, expected_names);
     ASSERT_EQ(identifier_names, call_proc->result_fields_);
-    CheckCallProcedureDefaultMemoryLimit(ast_generator, *call_proc);
   };
   check_proc(ProcedureType::READ);
   check_proc(ProcedureType::WRITE);
@@ -2959,7 +2944,6 @@ TEST_P(CypherMainVisitorTest, CallProcedureWithYieldAliasedFields) {
   ASSERT_EQ(identifier_names, aliased_names);
   std::vector<std::string> field_names{"fst", "snd", "thrd"};
   ASSERT_EQ(call_proc->result_fields_, field_names);
-  CheckCallProcedureDefaultMemoryLimit(ast_generator, *call_proc);
 }
 
 TEST_P(CypherMainVisitorTest, CallProcedureWithArguments) {
@@ -2986,7 +2970,6 @@ TEST_P(CypherMainVisitorTest, CallProcedureWithArguments) {
   std::vector<std::string> expected_names{"res"};
   ASSERT_EQ(identifier_names, expected_names);
   ASSERT_EQ(identifier_names, call_proc->result_fields_);
-  CheckCallProcedureDefaultMemoryLimit(ast_generator, *call_proc);
 }
 
 TEST_P(CypherMainVisitorTest, CallProcedureYieldAsterisk) {
@@ -3008,7 +2991,6 @@ TEST_P(CypherMainVisitorTest, CallProcedureYieldAsterisk) {
   }
   ASSERT_THAT(identifier_names, UnorderedElementsAre("name", "signature", "is_write", "path", "is_editable"));
   ASSERT_EQ(identifier_names, call_proc->result_fields_);
-  CheckCallProcedureDefaultMemoryLimit(ast_generator, *call_proc);
 }
 
 TEST_P(CypherMainVisitorTest, CallProcedureYieldAsteriskReturnAsterisk) {
@@ -3033,7 +3015,6 @@ TEST_P(CypherMainVisitorTest, CallProcedureYieldAsteriskReturnAsterisk) {
   }
   ASSERT_THAT(identifier_names, UnorderedElementsAre("name", "signature", "is_write", "path", "is_editable"));
   ASSERT_EQ(identifier_names, call_proc->result_fields_);
-  CheckCallProcedureDefaultMemoryLimit(ast_generator, *call_proc);
 }
 
 TEST_P(CypherMainVisitorTest, CallProcedureWithoutYield) {
@@ -3049,7 +3030,6 @@ TEST_P(CypherMainVisitorTest, CallProcedureWithoutYield) {
   ASSERT_TRUE(call_proc->arguments_.empty());
   ASSERT_TRUE(call_proc->result_fields_.empty());
   ASSERT_TRUE(call_proc->result_identifiers_.empty());
-  CheckCallProcedureDefaultMemoryLimit(ast_generator, *call_proc);
 }
 
 TEST_P(CypherMainVisitorTest, CallProcedureWithMemoryLimitWithoutYield) {
@@ -3183,7 +3163,6 @@ void CheckParsedCallProcedure(const CypherQuery &query, Base &ast_generator,
   EXPECT_EQ(identifier_names, args_as_str);
   EXPECT_EQ(identifier_names, call_proc->result_fields_);
   ASSERT_EQ(call_proc->is_write_, type == ProcedureType::WRITE);
-  CheckCallProcedureDefaultMemoryLimit(ast_generator, *call_proc);
 };
 }  // namespace
 
@@ -3577,7 +3556,6 @@ TEST_P(CypherMainVisitorTest, MemoryLimit) {
     auto *single_query = query->single_query_;
     ASSERT_EQ(single_query->clauses_.size(), 2U);
     auto *call_proc = dynamic_cast<CallProcedure *>(single_query->clauses_[0]);
-    CheckCallProcedureDefaultMemoryLimit(ast_generator, *call_proc);
   }
 
   {
@@ -3638,7 +3616,6 @@ TEST_P(CypherMainVisitorTest, MemoryLimit) {
     auto *single_query = query->single_query_;
     ASSERT_EQ(single_query->clauses_.size(), 1U);
     auto *call_proc = dynamic_cast<CallProcedure *>(single_query->clauses_[0]);
-    CheckCallProcedureDefaultMemoryLimit(ast_generator, *call_proc);
   }
 }
 

tests/unit/query_plan_accumulate_aggregate.cpp

@@ -250,30 +250,23 @@ TYPED_TEST(QueryPlanAggregateOps, WithData) {
 TYPED_TEST(QueryPlanAggregateOps, WithoutDataWithGroupBy) {
   {
     auto results = this->AggregationResults(true, false, {Aggregation::Op::COUNT});
-    EXPECT_EQ(results.size(), 1);
-    EXPECT_EQ(results[0][0].type(), TypedValue::Type::Int);
-    EXPECT_EQ(results[0][0].ValueInt(), 0);
+    EXPECT_EQ(results.size(), 0);
   }
   {
     auto results = this->AggregationResults(true, false, {Aggregation::Op::SUM});
-    EXPECT_EQ(results.size(), 1);
-    EXPECT_EQ(results[0][0].type(), TypedValue::Type::Int);
-    EXPECT_EQ(results[0][0].ValueInt(), 0);
+    EXPECT_EQ(results.size(), 0);
   }
   {
     auto results = this->AggregationResults(true, false, {Aggregation::Op::AVG});
-    EXPECT_EQ(results.size(), 1);
-    EXPECT_EQ(results[0][0].type(), TypedValue::Type::Null);
+    EXPECT_EQ(results.size(), 0);
   }
   {
     auto results = this->AggregationResults(true, false, {Aggregation::Op::MIN});
-    EXPECT_EQ(results.size(), 1);
-    EXPECT_EQ(results[0][0].type(), TypedValue::Type::Null);
+    EXPECT_EQ(results.size(), 0);
   }
   {
     auto results = this->AggregationResults(true, false, {Aggregation::Op::MAX});
-    EXPECT_EQ(results.size(), 1);
-    EXPECT_EQ(results[0][0].type(), TypedValue::Type::Null);
+    EXPECT_EQ(results.size(), 0);
   }
   {
     auto results = this->AggregationResults(true, false, {Aggregation::Op::COLLECT_LIST});
@@ -666,30 +659,23 @@ TYPED_TEST(QueryPlanAggregateOps, WithDataDistinct) {
 TYPED_TEST(QueryPlanAggregateOps, WithoutDataWithDistinctAndWithGroupBy) {
   {
     auto results = this->AggregationResults(true, true, {Aggregation::Op::COUNT});
-    EXPECT_EQ(results.size(), 1);
-    EXPECT_EQ(results[0][0].type(), TypedValue::Type::Int);
-    EXPECT_EQ(results[0][0].ValueInt(), 0);
+    EXPECT_EQ(results.size(), 0);
   }
   {
     auto results = this->AggregationResults(true, true, {Aggregation::Op::SUM});
-    EXPECT_EQ(results.size(), 1);
-    EXPECT_EQ(results[0][0].type(), TypedValue::Type::Int);
-    EXPECT_EQ(results[0][0].ValueInt(), 0);
+    EXPECT_EQ(results.size(), 0);
   }
   {
     auto results = this->AggregationResults(true, true, {Aggregation::Op::AVG});
-    EXPECT_EQ(results.size(), 1);
-    EXPECT_EQ(results[0][0].type(), TypedValue::Type::Null);
+    EXPECT_EQ(results.size(), 0);
   }
   {
     auto results = this->AggregationResults(true, true, {Aggregation::Op::MIN});
-    EXPECT_EQ(results.size(), 1);
-    EXPECT_EQ(results[0][0].type(), TypedValue::Type::Null);
+    EXPECT_EQ(results.size(), 0);
   }
   {
     auto results = this->AggregationResults(true, true, {Aggregation::Op::MAX});
-    EXPECT_EQ(results.size(), 1);
-    EXPECT_EQ(results[0][0].type(), TypedValue::Type::Null);
+    EXPECT_EQ(results.size(), 0);
   }
   {
     auto results = this->AggregationResults(true, true, {Aggregation::Op::COLLECT_LIST});