diff --git a/CHANGELOG.md b/CHANGELOG.md index 952fe0aba..c6f2b1acc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * CASE construct (without aggregations). * `rand` function added. * Maps can now be stored as vertex/edge properties. +* `collect` aggregation now supports Map collection. ### Bug Fixes and Other Changes diff --git a/docs/user_technical/open-cypher.md b/docs/user_technical/open-cypher.md index 8c8ffffe9..4c8678857 100644 --- a/docs/user_technical/open-cypher.md +++ b/docs/user_technical/open-cypher.md @@ -190,16 +190,24 @@ openCypher has functions for aggregating data. Memgraph currently supports the following aggregating functions. * `avg`, for calculating the average. - * `collect`, for collecting multiple values into a single list. + * `collect`, for collecting multiple values into a single list or map. If given a single expression values are collected into a list. If given two expressions, values are collected into a map where the first expression denotes map keys (must be string values) and the second expression denotes map values. * `count`, for counting the resulting values. * `max`, for calculating the maximum result. * `min`, for calculating the minimum result. * `sum`, for getting the sum of numeric results. -Example, calculating the average age. +Example, calculating the average age: MATCH (n :Person) RETURN avg(n.age) AS averageAge +Collecting items into a list: + + MATCH (n :Person) RETURN collect(n.name) AS list_of_names + +Collecting items into a map: + + MATCH (n :Person) RETURN collect(n.name, n.age) AS map_name_to_age + Click [here](https://neo4j.com/docs/developer-manual/current/cypher/functions/aggregating/) for additional details on how aggregations work. diff --git a/src/query/frontend/ast/ast.hpp b/src/query/frontend/ast/ast.hpp index b507061c4..84166259f 100644 --- a/src/query/frontend/ast/ast.hpp +++ b/src/query/frontend/ast/ast.hpp @@ -797,11 +797,11 @@ class Function : public Expression { : Expression(uid), function_(function), arguments_(arguments) {} }; -class Aggregation : public UnaryOperator { +class Aggregation : public BinaryOperator { friend class AstTreeStorage; public: - enum class Op { COUNT, MIN, MAX, SUM, AVG, COLLECT }; + enum class Op { COUNT, MIN, MAX, SUM, AVG, COLLECT_LIST, COLLECT_MAP }; static const constexpr char *const kCount = "COUNT"; static const constexpr char *const kMin = "MIN"; static const constexpr char *const kMax = "MAX"; @@ -812,26 +812,31 @@ class Aggregation : public UnaryOperator { DEFVISITABLE(TreeVisitor); bool Accept(HierarchicalTreeVisitor &visitor) override { if (visitor.PreVisit(*this)) { - if (expression_) { - expression_->Accept(visitor); - } + if (expression1_) expression1_->Accept(visitor); + if (expression2_) expression2_->Accept(visitor); } return visitor.PostVisit(*this); } Aggregation *Clone(AstTreeStorage &storage) const override { return storage.Create( - expression_ ? expression_->Clone(storage) : nullptr, op_); + expression1_ ? expression1_->Clone(storage) : nullptr, + expression2_ ? expression2_->Clone(storage) : nullptr, op_); } Op op_; protected: - Aggregation(int uid, Expression *expression, Op op) - : UnaryOperator(uid, expression), op_(op) { + /** Aggregation's first expression is the value being aggregated. The second + * expression is the key used only in COLLECT_MAP. */ + Aggregation(int uid, Expression *expression1, Expression *expression2, Op op) + : BinaryOperator(uid, expression1, expression2), op_(op) { // COUNT without expression denotes COUNT(*) in cypher. - debug_assert(expression || op == Aggregation::Op::COUNT, + debug_assert(expression1 || op == Aggregation::Op::COUNT, "All aggregations, except COUNT require expression"); + debug_assert(expression2 == nullptr ^ op == Aggregation::Op::COLLECT_MAP, + "The second expression is obligatory in COLLECT_MAP and " + "invalid otherwise"); } }; diff --git a/src/query/frontend/ast/cypher_main_visitor.cpp b/src/query/frontend/ast/cypher_main_visitor.cpp index 05e2f81f4..fa1cf46e0 100644 --- a/src/query/frontend/ast/cypher_main_visitor.cpp +++ b/src/query/frontend/ast/cypher_main_visitor.cpp @@ -785,7 +785,7 @@ antlrcpp::Any CypherMainVisitor::visitAtom(CypherParser::AtomContext *ctx) { // visitFunctionInvocation with other aggregations. This is visible in // functionInvocation and atom producions in opencypher grammar. return static_cast( - storage_.Create(nullptr, Aggregation::Op::COUNT)); + storage_.Create(nullptr, nullptr, Aggregation::Op::COUNT)); } else if (ctx->ALL()) { auto *ident = storage_.Create(ctx->filterExpression() ->idInColl() @@ -874,30 +874,36 @@ antlrcpp::Any CypherMainVisitor::visitFunctionInvocation( } if (expressions.size() == 1U) { if (function_name == Aggregation::kCount) { - return static_cast( - storage_.Create(expressions[0], Aggregation::Op::COUNT)); + return static_cast(storage_.Create( + expressions[0], nullptr, Aggregation::Op::COUNT)); } if (function_name == Aggregation::kMin) { - return static_cast( - storage_.Create(expressions[0], Aggregation::Op::MIN)); + return static_cast(storage_.Create( + expressions[0], nullptr, Aggregation::Op::MIN)); } if (function_name == Aggregation::kMax) { - return static_cast( - storage_.Create(expressions[0], Aggregation::Op::MAX)); + return static_cast(storage_.Create( + expressions[0], nullptr, Aggregation::Op::MAX)); } if (function_name == Aggregation::kSum) { - return static_cast( - storage_.Create(expressions[0], Aggregation::Op::SUM)); + return static_cast(storage_.Create( + expressions[0], nullptr, Aggregation::Op::SUM)); } if (function_name == Aggregation::kAvg) { - return static_cast( - storage_.Create(expressions[0], Aggregation::Op::AVG)); + return static_cast(storage_.Create( + expressions[0], nullptr, Aggregation::Op::AVG)); } if (function_name == Aggregation::kCollect) { return static_cast(storage_.Create( - expressions[0], Aggregation::Op::COLLECT)); + expressions[0], nullptr, Aggregation::Op::COLLECT_LIST)); } } + + if (expressions.size() == 2U && function_name == Aggregation::kCollect) { + return static_cast(storage_.Create( + expressions[1], expressions[0], Aggregation::Op::COLLECT_MAP)); + } + auto function = NameToFunction(function_name); if (!function) throw SemanticException("Function '{}' doesn't exist.", function_name); diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index dfe56c52d..75cfa15fc 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -1587,12 +1587,12 @@ Aggregate::AggregateCursor::AggregateCursor(Aggregate &self, : self_(self), db_(db), input_cursor_(self_.input_->MakeCursor(db)) {} namespace { -/** Returns the default TypedValue for an Aggregation operation. +/** Returns the default TypedValue for an Aggregation element. * This value is valid both for returning when where are no inputs * to the aggregation op, and for initializing an aggregation result * when there are */ -TypedValue DefaultAggregationOpValue(Aggregation::Op op) { - switch (op) { +TypedValue DefaultAggregationOpValue(const Aggregate::Element &element) { + switch (element.op) { case Aggregation::Op::COUNT: return TypedValue(0); case Aggregation::Op::SUM: @@ -1600,8 +1600,10 @@ TypedValue DefaultAggregationOpValue(Aggregation::Op op) { case Aggregation::Op::MAX: case Aggregation::Op::AVG: return TypedValue::Null; - case Aggregation::Op::COLLECT: + case Aggregation::Op::COLLECT_LIST: return TypedValue(std::vector()); + case Aggregation::Op::COLLECT_MAP: + return TypedValue(std::map()); } } } @@ -1618,7 +1620,7 @@ bool Aggregate::AggregateCursor::Pull(Frame &frame, if (aggregation_.empty() && self_.group_by_.empty()) { // place default aggregation values on the frame for (const auto &elem : self_.aggregations_) - frame[std::get<2>(elem)] = DefaultAggregationOpValue(std::get<1>(elem)); + frame[elem.output_sym] = DefaultAggregationOpValue(elem); // place null as remember values on the frame for (const Symbol &remember_sym : self_.remember_) frame[remember_sym] = TypedValue::Null; @@ -1631,7 +1633,7 @@ bool Aggregate::AggregateCursor::Pull(Frame &frame, // place aggregation values on the frame auto aggregation_values_it = aggregation_it_->second.values_.begin(); for (const auto &aggregation_elem : self_.aggregations_) - frame[std::get<2>(aggregation_elem)] = *aggregation_values_it++; + frame[aggregation_elem.output_sym] = *aggregation_values_it++; // place remember values on the frame auto remember_values_it = aggregation_it_->second.remember_.begin(); @@ -1650,7 +1652,7 @@ void Aggregate::AggregateCursor::ProcessAll(Frame &frame, // calculate AVG aggregations (so far they have only been summed) for (int pos = 0; pos < static_cast(self_.aggregations_.size()); ++pos) { - if (std::get<1>(self_.aggregations_[pos]) != Aggregation::Op::AVG) continue; + if (self_.aggregations_[pos].op != Aggregation::Op::AVG) continue; for (auto &kv : aggregation_) { AggregationValue &agg_value = kv.second; int count = agg_value.counts_[pos]; @@ -1680,8 +1682,7 @@ void Aggregate::AggregateCursor::EnsureInitialized( if (agg_value.values_.size() > 0) return; for (const auto &agg_elem : self_.aggregations_) - agg_value.values_.emplace_back( - DefaultAggregationOpValue(std::get<1>(agg_elem))); + agg_value.values_.emplace_back(DefaultAggregationOpValue(agg_elem)); agg_value.counts_.resize(self_.aggregations_.size(), 0); for (const Symbol &remember_sym : self_.remember_) @@ -1706,7 +1707,7 @@ void Aggregate::AggregateCursor::Update( count_it++, value_it++, agg_elem_it++) { // COUNT(*) is the only case where input expression is optional // handle it here - auto input_expr_ptr = std::get<0>(*agg_elem_it); + auto input_expr_ptr = agg_elem_it->value; if (!input_expr_ptr) { *count_it += 1; *value_it = *count_it; @@ -1717,8 +1718,7 @@ void Aggregate::AggregateCursor::Update( // Aggregations skip Null input values. if (input_value.IsNull()) continue; - - const auto &agg_op = std::get<1>(*agg_elem_it); + const auto &agg_op = agg_elem_it->op; *count_it += 1; if (*count_it == 1) { // first value, nothing to aggregate. check type, set and continue. @@ -1736,8 +1736,15 @@ void Aggregate::AggregateCursor::Update( case Aggregation::Op::COUNT: *value_it = 1; break; - case Aggregation::Op::COLLECT: - value_it->Value>().push_back(input_value); + case Aggregation::Op::COLLECT_LIST: + value_it->Value>().push_back(input_value); + break; + case Aggregation::Op::COLLECT_MAP: + auto key = agg_elem_it->key->Accept(evaluator); + if (key.type() != TypedValue::Type::String) + throw QueryRuntimeException("Map key must be a string"); + value_it->Value>().emplace( + key.Value(), input_value); break; } continue; @@ -1781,8 +1788,15 @@ void Aggregate::AggregateCursor::Update( EnsureOkForAvgSum(input_value); *value_it = *value_it + input_value; break; - case Aggregation::Op::COLLECT: - value_it->Value>().push_back(input_value); + case Aggregation::Op::COLLECT_LIST: + value_it->Value>().push_back(input_value); + break; + case Aggregation::Op::COLLECT_MAP: + auto key = agg_elem_it->key->Accept(evaluator); + if (key.type() != TypedValue::Type::String) + throw QueryRuntimeException("Map key must be a string"); + value_it->Value>().emplace( + key.Value(), input_value); break; } // end switch over Aggregation::Op enum } // end loop over all aggregations diff --git a/src/query/plan/operator.hpp b/src/query/plan/operator.hpp index d0a08c906..fcd20f50c 100644 --- a/src/query/plan/operator.hpp +++ b/src/query/plan/operator.hpp @@ -1146,9 +1146,15 @@ struct TypedValueListEqual { class Aggregate : public LogicalOperator { public: /** @brief An aggregation element, contains: - * (input data expression, type of aggregation, output symbol). + * (input data expression, key expression - only used in COLLECT_MAP, type of + * aggregation, output symbol). */ - using Element = std::tuple; + struct Element { + Expression *value; + Expression *key; + Aggregation::Op op; + Symbol output_sym; + }; Aggregate(const std::shared_ptr &input, const std::vector &aggregations, diff --git a/src/query/plan/rule_based_planner.cpp b/src/query/plan/rule_based_planner.cpp index 33089dc64..4c85b7040 100644 --- a/src/query/plan/rule_based_planner.cpp +++ b/src/query/plan/rule_based_planner.cpp @@ -352,10 +352,14 @@ class ReturnBodyContext : public HierarchicalTreeVisitor { bool PostVisit(Aggregation &aggr) override { // 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); - // aggregation expression_ is optional in COUNT(*), so it's possible the - // has_aggregation_ stack is empty - if (aggr.expression_) + aggregations_.emplace_back(Aggregate::Element{ + aggr.expression1_, aggr.expression2_, aggr.op_, symbol}); + // Aggregation expression1_ is optional in COUNT(*), and COLLECT_MAP uses + // two expressions, so we can have 0, 1 or 2 elements on the + // has_aggregation_stack for this Aggregation expression. + if (aggr.op_ == Aggregation::Op::COLLECT_MAP) + has_aggregation_.pop_back(); + if (aggr.expression1_) has_aggregation_.back() = true; else has_aggregation_.emplace_back(true); diff --git a/tests/qa/tck_engine/tests/memgraph_V1/features/aggregations.feature b/tests/qa/tck_engine/tests/memgraph_V1/features/aggregations.feature index 1d9ab75e2..f4b0e1deb 100644 --- a/tests/qa/tck_engine/tests/memgraph_V1/features/aggregations.feature +++ b/tests/qa/tck_engine/tests/memgraph_V1/features/aggregations.feature @@ -250,3 +250,18 @@ Feature: Aggregations Then the result should be (ignoring element order for lists) | n | | [0, true, 'asdf'] | + + Scenario: Collect test 03: + Given an empty graph + And having executed + """ + CREATE ({k: "a", v: 3}), ({k: "b", v: 1}), ({k: "c", v: 2}) + """ + When executing query: + """ + MATCH (a) RETURN collect(a.k + "_key", a.v + 10) AS n + """ + Then the result should be + | n | + | {a_key: 13, b_key: 11, c_key: 12} | + diff --git a/tests/unit/cypher_main_visitor.cpp b/tests/unit/cypher_main_visitor.cpp index 07b521419..40cea055a 100644 --- a/tests/unit/cypher_main_visitor.cpp +++ b/tests/unit/cypher_main_visitor.cpp @@ -670,16 +670,17 @@ TYPED_TEST(CypherMainVisitorTest, Aggregation) { auto *query = ast_generator.query_; auto *return_clause = dynamic_cast(query->clauses_[0]); ASSERT_EQ(return_clause->body_.named_expressions.size(), 7U); - Aggregation::Op ops[] = {Aggregation::Op::COUNT, Aggregation::Op::MIN, - Aggregation::Op::MAX, Aggregation::Op::SUM, - Aggregation::Op::AVG, Aggregation::Op::COLLECT}; + Aggregation::Op ops[] = { + Aggregation::Op::COUNT, Aggregation::Op::MIN, + Aggregation::Op::MAX, Aggregation::Op::SUM, + Aggregation::Op::AVG, Aggregation::Op::COLLECT_LIST}; std::string ids[] = {"a", "b", "c", "d", "e", "f"}; for (int i = 0; i < 6; ++i) { auto *aggregation = dynamic_cast( return_clause->body_.named_expressions[i]->expression_); ASSERT_TRUE(aggregation); ASSERT_EQ(aggregation->op_, ops[i]); - auto *identifier = dynamic_cast(aggregation->expression_); + auto *identifier = dynamic_cast(aggregation->expression1_); ASSERT_TRUE(identifier); ASSERT_EQ(identifier->name_, ids[i]); } @@ -687,7 +688,7 @@ TYPED_TEST(CypherMainVisitorTest, Aggregation) { return_clause->body_.named_expressions[6]->expression_); ASSERT_TRUE(aggregation); ASSERT_EQ(aggregation->op_, Aggregation::Op::COUNT); - ASSERT_FALSE(aggregation->expression_); + ASSERT_FALSE(aggregation->expression1_); } TYPED_TEST(CypherMainVisitorTest, UndefinedFunction) { diff --git a/tests/unit/query_common.hpp b/tests/unit/query_common.hpp index de8752ec8..e1b10c5e3 100644 --- a/tests/unit/query_common.hpp +++ b/tests/unit/query_common.hpp @@ -25,6 +25,7 @@ #include #include +#include #include "database/dbms.hpp" #include "database/graph_db_datatypes.hpp" @@ -45,6 +46,14 @@ auto ToList(const TypedValue &t) { return list; }; +template +auto ToMap(const TypedValue &t) { + std::map map; + for (const auto &kv : t.Value>()) + map.emplace(kv.first, kv.second.Value()); + return map; +}; + // Custom types for ORDER BY, SKIP, LIMIT, ON MATCH and ON CREATE expressions, // so that they can be used to resolve function calls. struct OrderBy { @@ -497,10 +506,12 @@ auto GetMerge(AstTreeStorage &storage, Pattern *pattern, OnMatch on_match, storage.Create((expr1), (expr2)) #define GREATER_EQ(expr1, expr2) \ storage.Create((expr1), (expr2)) -#define SUM(expr) \ - storage.Create((expr), query::Aggregation::Op::SUM) -#define COUNT(expr) \ - storage.Create((expr), query::Aggregation::Op::COUNT) +#define SUM(expr) \ + storage.Create((expr), nullptr, \ + query::Aggregation::Op::SUM) +#define COUNT(expr) \ + storage.Create((expr), nullptr, \ + query::Aggregation::Op::COUNT) #define EQ(expr1, expr2) storage.Create((expr1), (expr2)) #define NEQ(expr1, expr2) \ storage.Create((expr1), (expr2)) diff --git a/tests/unit/query_expression_evaluator.cpp b/tests/unit/query_expression_evaluator.cpp index 6d4e5cb7c..22977cf91 100644 --- a/tests/unit/query_expression_evaluator.cpp +++ b/tests/unit/query_expression_evaluator.cpp @@ -18,7 +18,6 @@ #include "query_common.hpp" using namespace query; -using testing::Pair; using testing::UnorderedElementsAre; using testing::ElementsAre; using query::test_common::ToList; @@ -661,7 +660,7 @@ TEST(ExpressionEvaluator, EdgeTypeTest) { TEST(ExpressionEvaluator, Aggregation) { AstTreeStorage storage; auto aggr = storage.Create(storage.Create(42), - Aggregation::Op::COUNT); + nullptr, Aggregation::Op::COUNT); SymbolTable symbol_table; auto aggr_sym = symbol_table.CreateSymbol("aggr", true); symbol_table[*aggr] = aggr_sym; @@ -751,9 +750,11 @@ TEST(ExpressionEvaluator, FunctionProperties) { return properties; }; ASSERT_THAT(prop_values_to_int(EvaluateFunction("PROPERTIES", {v1})), - UnorderedElementsAre(Pair("height", 5), Pair("age", 10))); + UnorderedElementsAre(testing::Pair("height", 5), + testing::Pair("age", 10))); ASSERT_THAT(prop_values_to_int(EvaluateFunction("PROPERTIES", {e})), - UnorderedElementsAre(Pair("height", 3), Pair("age", 15))); + UnorderedElementsAre(testing::Pair("height", 3), + testing::Pair("age", 15))); ASSERT_THROW(EvaluateFunction("PROPERTIES", {2}), QueryRuntimeException); } diff --git a/tests/unit/query_plan_accumulate_aggregate.cpp b/tests/unit/query_plan_accumulate_aggregate.cpp index e1d8f5d96..4e0559515 100644 --- a/tests/unit/query_plan_accumulate_aggregate.cpp +++ b/tests/unit/query_plan_accumulate_aggregate.cpp @@ -23,6 +23,7 @@ using namespace query; using namespace query::plan; using testing::UnorderedElementsAre; using query::test_common::ToList; +using query::test_common::ToMap; TEST(QueryPlan, Accumulate) { // simulate the following two query execution on an empty db @@ -129,12 +130,16 @@ std::shared_ptr MakeAggregationProduce( symbol_table.CreateSymbol("aggregation", true); symbol_table[*named_expr] = symbol_table.CreateSymbol("named_expression", true); - aggregates.emplace_back(*aggr_inputs_it++, aggr_op, - symbol_table[*named_expr->expression_]); + // the key expression is only used in COLLECT_MAP + Expression *key_expr_ptr = + aggr_op == Aggregation::Op::COLLECT_MAP ? LITERAL("key") : nullptr; + aggregates.emplace_back( + Aggregate::Element{*aggr_inputs_it++, key_expr_ptr, aggr_op, + symbol_table[*named_expr->expression_]}); } - // Produce will also evaluate group_by expressions - // and return them after the aggregations + // Produce will also evaluate group_by expressions and return them after the + // aggregations. for (auto group_by_expr : group_by_exprs) { auto named_expr = NEXPR("", group_by_expr); named_expressions.push_back(named_expr); @@ -146,7 +151,7 @@ std::shared_ptr MakeAggregationProduce( return std::make_shared(aggregation, named_expressions); } -/** Test fixture for all the aggregation ops in one return */ +/** Test fixture for all the aggregation ops in one return. */ class QueryPlanAggregateOps : public ::testing::Test { protected: Dbms dbms; @@ -174,13 +179,14 @@ class QueryPlanAggregateOps : public ::testing::Test { Aggregation::Op::COUNT, Aggregation::Op::COUNT, Aggregation::Op::MIN, Aggregation::Op::MAX, Aggregation::Op::SUM, Aggregation::Op::AVG, - Aggregation::Op::COLLECT}) { + Aggregation::Op::COLLECT_LIST, + Aggregation::Op::COLLECT_MAP}) { // match all nodes and perform aggregations auto n = MakeScanAll(storage, symbol_table, "n"); auto n_p = PROPERTY_LOOKUP("n", prop); symbol_table[*n_p->expression_] = n.sym_; - std::vector aggregation_expressions(7, n_p); + std::vector aggregation_expressions(ops.size(), n_p); std::vector group_bys; if (with_group_by) group_bys.push_back(n_p); aggregation_expressions[0] = nullptr; @@ -196,7 +202,7 @@ TEST_F(QueryPlanAggregateOps, WithData) { auto results = AggregationResults(false); ASSERT_EQ(results.size(), 1); - ASSERT_EQ(results[0].size(), 7); + ASSERT_EQ(results[0].size(), 8); // count(*) ASSERT_EQ(results[0][0].type(), TypedValue::Type::Int); EXPECT_EQ(results[0][0].Value(), 4); @@ -215,9 +221,15 @@ TEST_F(QueryPlanAggregateOps, WithData) { // avg ASSERT_EQ(results[0][5].type(), TypedValue::Type::Double); EXPECT_FLOAT_EQ(results[0][5].Value(), 24 / 3.0); - // collect + // collect list ASSERT_EQ(results[0][6].type(), TypedValue::Type::List); EXPECT_THAT(ToList(results[0][6]), UnorderedElementsAre(5, 7, 12)); + // collect map + ASSERT_EQ(results[0][7].type(), TypedValue::Type::Map); + auto map = ToMap(results[0][7]); + ASSERT_EQ(map.size(), 1); + EXPECT_EQ(map.begin()->first, "key"); + EXPECT_FALSE(std::set({5, 7, 12}).insert(map.begin()->second).second); } TEST_F(QueryPlanAggregateOps, WithoutDataWithGroupBy) { @@ -242,7 +254,11 @@ TEST_F(QueryPlanAggregateOps, WithoutDataWithGroupBy) { EXPECT_EQ(results.size(), 0); } { - auto results = AggregationResults(true, {Aggregation::Op::COLLECT}); + auto results = AggregationResults(true, {Aggregation::Op::COLLECT_LIST}); + EXPECT_EQ(results.size(), 0); + } + { + auto results = AggregationResults(true, {Aggregation::Op::COLLECT_MAP}); EXPECT_EQ(results.size(), 0); } } @@ -250,7 +266,7 @@ TEST_F(QueryPlanAggregateOps, WithoutDataWithGroupBy) { TEST_F(QueryPlanAggregateOps, WithoutDataWithoutGroupBy) { auto results = AggregationResults(false); ASSERT_EQ(results.size(), 1); - ASSERT_EQ(results[0].size(), 7); + ASSERT_EQ(results[0].size(), 8); // count(*) ASSERT_EQ(results[0][0].type(), TypedValue::Type::Int); EXPECT_EQ(results[0][0].Value(), 0); @@ -265,17 +281,18 @@ TEST_F(QueryPlanAggregateOps, WithoutDataWithoutGroupBy) { EXPECT_TRUE(results[0][4].IsNull()); // avg EXPECT_TRUE(results[0][5].IsNull()); - // collect + // collect list ASSERT_EQ(results[0][6].type(), TypedValue::Type::List); - EXPECT_THAT(ToList(results[0][6]), UnorderedElementsAre()); + EXPECT_EQ(ToList(results[0][6]).size(), 0); + // collect map + ASSERT_EQ(results[0][7].type(), TypedValue::Type::Map); + EXPECT_EQ(ToMap(results[0][7]).size(), 0); } TEST(QueryPlan, AggregateGroupByValues) { - // tests that distinct groups are aggregated properly - // for values of all types - // also test the "remember" part of the Aggregation API - // as final results are obtained via a property lookup of - // a remembered node + // Tests that distinct groups are aggregated properly for values of all types. + // Also test the "remember" part of the Aggregation API as final results are + // obtained via a property lookup of a remembered node. Dbms dbms; auto dba = dbms.active(); @@ -475,7 +492,7 @@ TEST(QueryPlan, AggregateFirstValueTypes) { CollectProduce(produce.get(), symbol_table, *dba); }; - // everything except for COUNT fails on a Vertex + // everything except for COUNT and COLLECT fails on a Vertex aggregate(n_id, Aggregation::Op::COUNT); EXPECT_THROW(aggregate(n_id, Aggregation::Op::MIN), QueryRuntimeException); EXPECT_THROW(aggregate(n_id, Aggregation::Op::MAX), QueryRuntimeException); @@ -497,6 +514,8 @@ TEST(QueryPlan, AggregateFirstValueTypes) { aggregate(n_prop_int, Aggregation::Op::MAX); aggregate(n_prop_int, Aggregation::Op::AVG); aggregate(n_prop_int, Aggregation::Op::SUM); + aggregate(n_prop_int, Aggregation::Op::COLLECT_LIST); + aggregate(n_prop_int, Aggregation::Op::COLLECT_MAP); } TEST(QueryPlan, AggregateTypes) { @@ -530,9 +549,11 @@ TEST(QueryPlan, AggregateTypes) { CollectProduce(produce.get(), symbol_table, *dba); }; - // everything except for COUNT fails on a Vertex + // everything except for COUNT and COLLECT fails on a Vertex auto n_id = n_p1->expression_; aggregate(n_id, Aggregation::Op::COUNT); + aggregate(n_id, Aggregation::Op::COLLECT_LIST); + aggregate(n_id, Aggregation::Op::COLLECT_MAP); EXPECT_THROW(aggregate(n_id, Aggregation::Op::MIN), QueryRuntimeException); EXPECT_THROW(aggregate(n_id, Aggregation::Op::MAX), QueryRuntimeException); EXPECT_THROW(aggregate(n_id, Aggregation::Op::AVG), QueryRuntimeException); @@ -540,13 +561,17 @@ TEST(QueryPlan, AggregateTypes) { // on strings AVG and SUM fail aggregate(n_p1, Aggregation::Op::COUNT); + aggregate(n_p1, Aggregation::Op::COLLECT_LIST); + aggregate(n_p1, Aggregation::Op::COLLECT_MAP); aggregate(n_p1, Aggregation::Op::MIN); aggregate(n_p1, Aggregation::Op::MAX); EXPECT_THROW(aggregate(n_p1, Aggregation::Op::AVG), QueryRuntimeException); EXPECT_THROW(aggregate(n_p1, Aggregation::Op::SUM), QueryRuntimeException); - // combination of int and bool, everything except count fails + // combination of int and bool, everything except COUNT and COLLECT fails aggregate(n_p2, Aggregation::Op::COUNT); + aggregate(n_p2, Aggregation::Op::COLLECT_LIST); + aggregate(n_p2, Aggregation::Op::COLLECT_MAP); EXPECT_THROW(aggregate(n_p2, Aggregation::Op::MIN), QueryRuntimeException); EXPECT_THROW(aggregate(n_p2, Aggregation::Op::MAX), QueryRuntimeException); EXPECT_THROW(aggregate(n_p2, Aggregation::Op::AVG), QueryRuntimeException); diff --git a/tests/unit/query_planner.cpp b/tests/unit/query_planner.cpp index e491ed160..6992c5c6c 100644 --- a/tests/unit/query_planner.cpp +++ b/tests/unit/query_planner.cpp @@ -168,9 +168,10 @@ class ExpectAggregate : public OpChecker { for (const auto &aggr_elem : op.aggregations()) { ASSERT_NE(aggr_it, aggregations_.end()); auto aggr = *aggr_it++; - auto expected = - std::make_tuple(aggr->expression_, aggr->op_, symbol_table.at(*aggr)); - EXPECT_EQ(expected, aggr_elem); + EXPECT_EQ(aggr_elem.value, aggr->expression1_); + EXPECT_EQ(aggr_elem.key, aggr->expression2_); + EXPECT_EQ(aggr_elem.op, aggr->op_); + EXPECT_EQ(aggr_elem.output_sym, symbol_table.at(*aggr)); } EXPECT_EQ(aggr_it, aggregations_.end()); auto got_group_by = std::unordered_set(