From 21770e2aca29ebe3c32abe6e930cb355414248fa Mon Sep 17 00:00:00 2001
From: florijan <florijan@memgraph.io>
Date: Fri, 14 Apr 2017 17:14:14 +0200
Subject: [PATCH] Query::Plan::Aggregate - first value type check bugfix
Reviewers: teon.banek
Reviewed By: teon.banek
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D285
---
src/query/plan/operator.cpp | 14 ++++-
.../unit/query_plan_accumulate_aggregate.cpp | 52 +++++++++++++++++++
2 files changed, 65 insertions(+), 1 deletion(-)
diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp
index c3ac44330..8cdc5f3e7 100644
--- a/src/query/plan/operator.cpp
+++ b/src/query/plan/operator.cpp
@@ -1108,7 +1108,19 @@ void Aggregate::AggregateCursor::Update(
const auto &agg_op = std::get<1>(*agg_elem_it);
*count_it += 1;
if (*count_it == 1) {
- // first value, nothing to aggregate. set and continue.
+ // first value, nothing to aggregate. check type, set and continue.
+ switch (agg_op) {
+ case Aggregation::Op::MIN:
+ case Aggregation::Op::MAX:
+ EnsureOkForMinMax(input_value);
+ break;
+ case Aggregation::Op::SUM:
+ case Aggregation::Op::AVG:
+ EnsureOkForAvgSum(input_value);
+ break;
+ case Aggregation::Op::COUNT:
+ break;
+ }
*value_it = agg_op == Aggregation::Op::COUNT ? 1 : input_value;
continue;
}
diff --git a/tests/unit/query_plan_accumulate_aggregate.cpp b/tests/unit/query_plan_accumulate_aggregate.cpp
index 6d6a0aaeb..08d5db26e 100644
--- a/tests/unit/query_plan_accumulate_aggregate.cpp
+++ b/tests/unit/query_plan_accumulate_aggregate.cpp
@@ -403,6 +403,58 @@ TEST(QueryPlan, AggregateCountEdgeCases) {
EXPECT_EQ(2, count());
}
+TEST(QueryPlan, AggregateFirstValueTypes) {
+ // testing exceptions that get emitted by the first-value
+ // type check
+
+ Dbms dbms;
+ auto dba = dbms.active();
+
+ auto v1 = dba->insert_vertex();
+ auto prop_string = dba->property("string");
+ v1.PropsSet(prop_string, "johhny");
+ auto prop_int = dba->property("int");
+ v1.PropsSet(prop_int, 12);
+ dba->advance_command();
+
+ AstTreeStorage storage;
+ SymbolTable symbol_table;
+
+ auto n = MakeScanAll(storage, symbol_table, "n");
+ auto n_prop_string = PROPERTY_LOOKUP("n", prop_string);
+ symbol_table[*n_prop_string->expression_] = n.sym_;
+ auto n_prop_int = PROPERTY_LOOKUP("n", prop_int);
+ symbol_table[*n_prop_int->expression_] = n.sym_;
+ auto n_id = n_prop_string->expression_;
+
+ auto aggregate = [&](Expression *expression, Aggregation::Op aggr_op) {
+ auto produce = MakeAggregationProduce(n.op_, symbol_table, storage,
+ {expression}, {aggr_op}, {}, {});
+ CollectProduce(produce, symbol_table, *dba).GetResults();
+ };
+
+ // everything except for COUNT fails on a Vertex
+ aggregate(n_id, Aggregation::Op::COUNT);
+ EXPECT_THROW(aggregate(n_id, Aggregation::Op::MIN), TypedValueException);
+ EXPECT_THROW(aggregate(n_id, Aggregation::Op::MAX), TypedValueException);
+ EXPECT_THROW(aggregate(n_id, Aggregation::Op::AVG), TypedValueException);
+ EXPECT_THROW(aggregate(n_id, Aggregation::Op::SUM), TypedValueException);
+
+ // on strings AVG and SUM fail
+ aggregate(n_prop_string, Aggregation::Op::COUNT);
+ aggregate(n_prop_string, Aggregation::Op::MIN);
+ aggregate(n_prop_string, Aggregation::Op::MAX);
+ EXPECT_THROW(aggregate(n_prop_string, Aggregation::Op::AVG), TypedValueException);
+ EXPECT_THROW(aggregate(n_prop_string, Aggregation::Op::SUM), TypedValueException);
+
+ // on ints nothing fails
+ aggregate(n_prop_int, Aggregation::Op::COUNT);
+ aggregate(n_prop_int, Aggregation::Op::MIN);
+ aggregate(n_prop_int, Aggregation::Op::MAX);
+ aggregate(n_prop_int, Aggregation::Op::AVG);
+ aggregate(n_prop_int, Aggregation::Op::SUM);
+}
+
TEST(QueryPlan, AggregateTypes) {
// testing exceptions that can get emitted by an aggregation
// does not check all combinations that can result in an exception