Query::Plan::Aggregate - removed advance_command

Reviewers: teon.banek

Reviewed By: teon.banek

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D304

src/query/plan/operator.cpp

@@ -972,12 +972,11 @@ bool Accumulate::AccumulateCursor::Pull(Frame &frame,
 Aggregate::Aggregate(const std::shared_ptr<LogicalOperator> &input,
                      const std::vector<Aggregate::Element> &aggregations,
                      const std::vector<Expression *> &group_by,
-                     const std::vector<Symbol> &remember, bool advance_command)
+                     const std::vector<Symbol> &remember)
     : input_(input),
       aggregations_(aggregations),
       group_by_(group_by),
-      remember_(remember),
+      remember_(remember) {}
-      advance_command_(advance_command) {}
 
 void Aggregate::Accept(LogicalOperatorVisitor &visitor) {
   if (visitor.PreVisit(*this)) {
@@ -994,28 +993,14 @@ std::unique_ptr<Cursor> Aggregate::MakeCursor(GraphDbAccessor &db) {
 Aggregate::AggregateCursor::AggregateCursor(Aggregate &self,
                                             GraphDbAccessor &db)
     : self_(self),
-      db_(db),
       input_cursor_(self.input_ ? self_.input_->MakeCursor(db) : nullptr) {}
 
 bool Aggregate::AggregateCursor::Pull(Frame &frame,
                                       const SymbolTable &symbol_table) {
   if (!pulled_all_input_) {
     ProcessAll(frame, symbol_table);
-
     pulled_all_input_ = true;
     aggregation_it_ = aggregation_.begin();
-
-    if (self_.advance_command_) {
-      db_.advance_command();
-      // regarding reconstruction after advance_command
-      // we have to reconstruct only the remember values
-      // because aggregation results are primitives and
-      // group-by elements won't be used directly (possibly re-evaluated
-      // using remember values)
-      for (auto &kv : aggregation_)
-        for (TypedValue &remember : kv.second.remember_)
-          ReconstructTypedValue(remember);
-    }
   }
 
   if (aggregation_it_ == aggregation_.end()) return false;

src/query/plan/operator.hpp

@@ -833,7 +833,7 @@ class Aggregate : public LogicalOperator {
   Aggregate(const std::shared_ptr<LogicalOperator> &input,
             const std::vector<Element> &aggregations,
             const std::vector<Expression *> &group_by,
-            const std::vector<Symbol> &remember, bool advance_command = false);
+            const std::vector<Symbol> &remember);
   void Accept(LogicalOperatorVisitor &visitor) override;
   std::unique_ptr<Cursor> MakeCursor(GraphDbAccessor &db) override;
 
@@ -845,7 +845,6 @@ class Aggregate : public LogicalOperator {
   const std::vector<Element> aggregations_;
   const std::vector<Expression *> group_by_;
   const std::vector<Symbol> remember_;
-  const bool advance_command_;
 
   class AggregateCursor : public Cursor {
    public:
@@ -876,7 +875,6 @@ class Aggregate : public LogicalOperator {
     };
 
     Aggregate &self_;
-    GraphDbAccessor &db_;
     // optional
     std::unique_ptr<Cursor> input_cursor_;
     // storage for aggregated data

tests/unit/query_plan_accumulate_aggregate.cpp

@@ -294,38 +294,6 @@ TEST(QueryPlan, AggregateMultipleGroupBy) {
   EXPECT_EQ(results.size(), 2 * 3 * 5);
 }
 
-TEST(QueryPlan, AggregateAdvance) {
-  // we simulate 'CREATE (n {x: 42}) WITH count(n.x) AS c MATCH (m) RETURN m,
-  // m.x, c'
-  // to get correct results we need to advance the command in aggregation
-  // since we only test aggregation, we'll simplify the logical plan and only
-  // check the count and not all the results
-
-  auto check = [&](bool advance) {
-    Dbms dbms;
-    auto dba = dbms.active();
-    AstTreeStorage storage;
-    SymbolTable symbol_table;
-
-    auto node = NODE("n");
-    auto sym_n = symbol_table.CreateSymbol("n");
-    symbol_table[*node->identifier_] = sym_n;
-    auto create = std::make_shared<CreateNode>(node, nullptr);
-
-    auto aggr_sym = symbol_table.CreateSymbol("aggr_sym");
-    auto n_p = PROPERTY_LOOKUP("n", dba->property("x"));
-    symbol_table[*n_p->expression_] = sym_n;
-    auto aggregate = std::make_shared<Aggregate>(
-        create, std::vector<Aggregate::Element>{Aggregate::Element{
-                    n_p, Aggregation::Op::COUNT, aggr_sym}},
-        std::vector<Expression *>{}, std::vector<Symbol>{}, advance);
-    auto match = MakeScanAll(storage, symbol_table, "m", aggregate);
-    EXPECT_EQ(advance ? 1 : 0, PullAll(match.op_, *dba, symbol_table));
-  };
-  //  check(false);
-  check(true);
-}
-
 TEST(QueryPlan, AggregateNoInput) {
   Dbms dbms;
   auto dba = dbms.active();