From 0866fdcb0c885c225dcf381fe879dcc8a240efbb Mon Sep 17 00:00:00 2001
From: florijan <florijan@memgraph.io>
Date: Sat, 6 May 2017 17:57:39 +0200
Subject: [PATCH] Query:: COUNT(*) added to logical planning and execution

Reviewers: mislav.bradac, buda, teon.banek

Reviewed By: mislav.bradac, teon.banek

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D353
---
 src/query/plan/operator.cpp                   | 12 +++++-
 src/query/plan/planner.cpp                    |  6 ++-
 .../unit/query_plan_accumulate_aggregate.cpp  | 37 +++++++++++--------
 3 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp
index 73128fa57..78fb20c60 100644
--- a/src/query/plan/operator.cpp
+++ b/src/query/plan/operator.cpp
@@ -1059,7 +1059,17 @@ void Aggregate::AggregateCursor::Update(
   auto agg_elem_it = self_.aggregations_.begin();
   for (; count_it < agg_value.counts_.end();
        count_it++, value_it++, agg_elem_it++) {
-    std::get<0>(*agg_elem_it)->Accept(evaluator);
+
+    // COUNT(*) is the only case where input expression is optional
+    // handle it here
+    auto input_expr_ptr = std::get<0>(*agg_elem_it);
+    if (!input_expr_ptr) {
+      *count_it += 1;
+      *value_it = *count_it;
+      continue;
+    }
+
+    input_expr_ptr->Accept(evaluator);
     TypedValue input_value = evaluator.PopBack();
 
     // Aggregations skip Null input values.
diff --git a/src/query/plan/planner.cpp b/src/query/plan/planner.cpp
index c1d387cbd..dd4b8bc11 100644
--- a/src/query/plan/planner.cpp
+++ b/src/query/plan/planner.cpp
@@ -310,7 +310,11 @@ class ReturnBodyContext : public TreeVisitorBase {
     // Aggregation contains a virtual symbol, where the result will be stored.
     const auto &symbol = symbol_table_.at(aggr);
     aggregations_.emplace_back(aggr.expression_, aggr.op_, symbol);
-    has_aggregation_.back() = true;
+    // aggregation expression_ is opional in COUNT(*) so it's possible the has_aggregation_ stack is empty
+    if (aggr.expression_)
+      has_aggregation_.back() = true;
+    else
+      has_aggregation_.emplace_back(true);
     // Possible optimization is to skip remembering symbols inside aggregation.
     // If and when implementing this, don't forget that Accumulate needs *all*
     // the symbols, including those inside aggregation.
diff --git a/tests/unit/query_plan_accumulate_aggregate.cpp b/tests/unit/query_plan_accumulate_aggregate.cpp
index 5e9bacab2..19e69c03e 100644
--- a/tests/unit/query_plan_accumulate_aggregate.cpp
+++ b/tests/unit/query_plan_accumulate_aggregate.cpp
@@ -152,10 +152,10 @@ TEST(QueryPlan, AggregateOps) {
   // we will take the sum, avg, min, max and count
   // we won't group by anything
   auto prop = dba->property("prop");
-  dba->insert_vertex().PropsSet(prop, 4);
+  dba->insert_vertex().PropsSet(prop, 5);
   dba->insert_vertex().PropsSet(prop, 7);
   dba->insert_vertex().PropsSet(prop, 12);
-  // a missing property (null) gets ignored by all aggregations
+  // a missing property (null) gets ignored by all aggregations except COUNT(*)
   dba->insert_vertex();
   dba->advance_command();
 
@@ -167,31 +167,36 @@ TEST(QueryPlan, AggregateOps) {
   auto n_p = PROPERTY_LOOKUP("n", prop);
   symbol_table[*n_p->expression_] = n.sym_;
 
+  std::vector<Expression *> aggregation_expressions(6, n_p);
+  aggregation_expressions[0] = nullptr;
   auto produce = MakeAggregationProduce(
-      n.op_, symbol_table, storage, std::vector<Expression *>(5, n_p),
-      {Aggregation::Op::COUNT, Aggregation::Op::MIN, Aggregation::Op::MAX,
-       Aggregation::Op::SUM, Aggregation::Op::AVG},
+      n.op_, symbol_table, storage, aggregation_expressions,
+      {Aggregation::Op::COUNT, Aggregation::Op::COUNT, Aggregation::Op::MIN,
+       Aggregation::Op::MAX, Aggregation::Op::SUM, Aggregation::Op::AVG},
       {}, {});
 
   // checks
   auto results = CollectProduce(produce, symbol_table, *dba).GetResults();
   ASSERT_EQ(results.size(), 1);
-  ASSERT_EQ(results[0].size(), 5);
-  // count
+  ASSERT_EQ(results[0].size(), 6);
+  // count(*)
   ASSERT_EQ(results[0][0].type(), TypedValue::Type::Int);
-  EXPECT_EQ(results[0][0].Value<int64_t>(), 3);
-  // min
+  EXPECT_EQ(results[0][0].Value<int64_t>(), 4);
+  // count
   ASSERT_EQ(results[0][1].type(), TypedValue::Type::Int);
-  EXPECT_EQ(results[0][1].Value<int64_t>(), 4);
-  // max
+  EXPECT_EQ(results[0][1].Value<int64_t>(), 3);
+  // min
   ASSERT_EQ(results[0][2].type(), TypedValue::Type::Int);
-  EXPECT_EQ(results[0][2].Value<int64_t>(), 12);
-  // sum
+  EXPECT_EQ(results[0][2].Value<int64_t>(), 5);
+  // max
   ASSERT_EQ(results[0][3].type(), TypedValue::Type::Int);
-  EXPECT_EQ(results[0][3].Value<int64_t>(), 23);
+  EXPECT_EQ(results[0][3].Value<int64_t>(), 12);
+  // sum
+  ASSERT_EQ(results[0][4].type(), TypedValue::Type::Int);
+  EXPECT_EQ(results[0][4].Value<int64_t>(), 24);
   // avg
-  ASSERT_EQ(results[0][4].type(), TypedValue::Type::Double);
-  EXPECT_FLOAT_EQ(results[0][4].Value<double>(), 23 / 3.0);
+  ASSERT_EQ(results[0][5].type(), TypedValue::Type::Double);
+  EXPECT_FLOAT_EQ(results[0][5].Value<double>(), 24 / 3.0);
 }
 
 TEST(QueryPlan, AggregateGroupByValues) {