From dabdeeff2dfee3d858fbfc8ea9ffc52a4ff0757d Mon Sep 17 00:00:00 2001
From: "kyle.cao" <kyle.cao@vesoft.com>
Date: Mon, 11 Jan 2021 10:36:37 +0800
Subject: [PATCH] groupby refactoring (#503)
* refactor groupBy impl
tmp commit
modify parser
modify subgraph
graphd compiled
complied
fix empty collect
fix UT
add bug
fix count
add visitors
add projCol
compiled
add parser ut
fix ut
plan validated
add pushUp flag
small change
fix ut memory leak
fix memory leak
handle yield validator
implicit groupby impl
add check
fix memory leak
fix memory leak
small change
fix memory leak
add agg check
* implicit groupBy refactoring
bak
add tck test cases
fix ut
implicit agg refactoring
fix agg Project
* small change
* fix memory leak
* add agg checks
fix agg check
small change
add test cases
* add tck test cases
add tck test cases
add group by tck test
small change
* fix test
modify visitors
small change
small change
fix avg
small change
small change
small change
* review
* delete AggFun cmake
* small change
* small change
* add multiVar check
* fix conflict
refactor implicit groupBy
fix
* add implicit groupBy where test cases
* fix ci
fix ci
* add case
* refactor GroupByValidator
fix case
small change
small change
small change
small change
small change
fix memory leak
fmt
small change
* review
fix ut
small change
* modify group by list check
* move test_groupby.py cases to tck
* add multiple user var check
multiple user var check
Co-authored-by: Yee <2520865+yixinglu@users.noreply.github.com>
---
.gitignore | 2 +
src/context/test/CMakeLists.txt | 1 -
src/daemons/CMakeLists.txt | 1 -
src/executor/admin/SubmitJobExecutor.cpp | 2 +-
src/executor/query/AggregateExecutor.cpp | 33 +-
src/executor/test/AggregateTest.cpp | 220 +++----
src/executor/test/CMakeLists.txt | 1 -
src/optimizer/test/CMakeLists.txt | 1 -
src/parser/AdminSentences.cpp | 2 +-
src/parser/Clauses.cpp | 13 +-
src/parser/Clauses.h | 35 +-
src/parser/TraverseSentences.cpp | 12 +-
src/parser/TraverseSentences.h | 19 +-
src/parser/parser.yy | 46 +-
src/parser/test/CMakeLists.txt | 1 -
src/parser/test/ParserTest.cpp | 62 +-
src/planner/Query.cpp | 6 +-
src/planner/Query.h | 18 +-
src/planner/match/StartVidFinder.cpp | 2 +-
src/util/ExpressionUtils.h | 1 +
src/util/test/CMakeLists.txt | 1 -
src/validator/GetSubgraphValidator.cpp | 31 +-
src/validator/GoValidator.cpp | 9 +-
src/validator/GroupByValidator.cpp | 220 +++++--
src/validator/GroupByValidator.h | 17 +-
src/validator/Validator.h | 54 +-
src/validator/YieldValidator.cpp | 128 ++--
src/validator/YieldValidator.h | 8 +-
src/validator/test/CMakeLists.txt | 1 -
src/validator/test/GroupByValidatorTest.cpp | 104 +++-
src/validator/test/QueryValidatorTest.cpp | 5 +-
src/validator/test/YieldValidatorTest.cpp | 26 +-
src/visitor/CMakeLists.txt | 2 +
src/visitor/CollectAllExprsVisitor.cpp | 5 +
src/visitor/CollectAllExprsVisitor.h | 1 +
src/visitor/DeduceTypeVisitor.cpp | 63 +-
src/visitor/DeduceTypeVisitor.h | 1 +
src/visitor/ExprVisitorImpl.cpp | 5 +
src/visitor/ExprVisitorImpl.h | 1 +
src/visitor/FindAnyExprVisitor.cpp | 6 +
src/visitor/FindAnyExprVisitor.h | 1 +
src/visitor/FindAnySubExprVisitor.cpp | 206 +++++++
src/visitor/FindAnySubExprVisitor.h | 83 +++
src/visitor/FoldConstantExprVisitor.cpp | 10 +
src/visitor/FoldConstantExprVisitor.h | 1 +
src/visitor/RewriteAggExprVisitor.cpp | 71 +++
src/visitor/RewriteAggExprVisitor.h | 70 +++
src/visitor/RewriteInputPropVisitor.cpp | 8 +
src/visitor/RewriteInputPropVisitor.h | 1 +
src/visitor/RewriteSymExprVisitor.cpp | 8 +
src/visitor/RewriteSymExprVisitor.h | 1 +
src/visitor/test/CMakeLists.txt | 1 -
tests/query/v1/test_yield.py | 16 +-
tests/tck/features/agg/Agg.feature | 559 ++++++++++++++++++
tests/tck/features/go/GO.feature | 3 +-
.../features/go/GroupbyLimit.IntVid.feature | 8 +-
tests/tck/features/go/GroupbyLimit.feature | 8 +-
57 files changed, 1813 insertions(+), 408 deletions(-)
create mode 100644 src/visitor/FindAnySubExprVisitor.cpp
create mode 100644 src/visitor/FindAnySubExprVisitor.h
create mode 100644 src/visitor/RewriteAggExprVisitor.cpp
create mode 100644 src/visitor/RewriteAggExprVisitor.h
create mode 100644 tests/tck/features/agg/Agg.feature
diff --git a/.gitignore b/.gitignore
index 01f7ec21..c3914e63 100644
--- a/.gitignore
+++ b/.gitignore
@@ -46,6 +46,8 @@ nebula-python
.classpath
cmake-build*/
.vscode/
+core.*
+workspace.*
#py
*.egg-info
diff --git a/src/context/test/CMakeLists.txt b/src/context/test/CMakeLists.txt
index 3c730164..ab775c1d 100644
--- a/src/context/test/CMakeLists.txt
+++ b/src/context/test/CMakeLists.txt
@@ -4,7 +4,6 @@
# attached with Common Clause Condition 1.0, found in the LICENSES directory.
SET(CONTEXT_TEST_LIBS
- $<TARGET_OBJECTS:common_agg_function_obj>
$<TARGET_OBJECTS:common_charset_obj>
$<TARGET_OBJECTS:common_datatypes_obj>
$<TARGET_OBJECTS:common_encryption_obj>
diff --git a/src/daemons/CMakeLists.txt b/src/daemons/CMakeLists.txt
index 201505bc..fb69079a 100644
--- a/src/daemons/CMakeLists.txt
+++ b/src/daemons/CMakeLists.txt
@@ -54,7 +54,6 @@ nebula_add_executable(
$<TARGET_OBJECTS:common_charset_obj>
$<TARGET_OBJECTS:common_encryption_obj>
$<TARGET_OBJECTS:common_function_manager_obj>
- $<TARGET_OBJECTS:common_agg_function_obj>
$<TARGET_OBJECTS:common_conf_obj>
$<TARGET_OBJECTS:common_time_utils_obj>
$<TARGET_OBJECTS:common_graph_obj>
diff --git a/src/executor/admin/SubmitJobExecutor.cpp b/src/executor/admin/SubmitJobExecutor.cpp
index 84ba6191..3e3e5902 100644
--- a/src/executor/admin/SubmitJobExecutor.cpp
+++ b/src/executor/admin/SubmitJobExecutor.cpp
@@ -112,7 +112,7 @@ folly::Future<Status> SubmitJobExecutor::execute() {
// no default so the compiler will warning when lack
}
DLOG(FATAL) << "Unknown job operation " << static_cast<int>(jobOp);
- return Status::Error("Unkown job job operation %d.", static_cast<int>(jobOp));
+ return Status::Error("Unknown job job operation %d.", static_cast<int>(jobOp));
});
}
diff --git a/src/executor/query/AggregateExecutor.cpp b/src/executor/query/AggregateExecutor.cpp
index d1e8316e..4e0f8841 100644
--- a/src/executor/query/AggregateExecutor.cpp
+++ b/src/executor/query/AggregateExecutor.cpp
@@ -7,7 +7,7 @@
#include "executor/query/AggregateExecutor.h"
#include "common/datatypes/List.h"
-#include "common/function/AggregateFunction.h"
+#include "common/expression/AggregateExpression.h"
#include "context/QueryExpressionContext.h"
#include "context/Result.h"
#include "planner/PlanNode.h"
@@ -26,7 +26,7 @@ folly::Future<Status> AggregateExecutor::execute() {
DCHECK(!!iter);
QueryExpressionContext ctx(ectx_);
- std::unordered_map<List, std::vector<std::unique_ptr<AggFun>>> result;
+ std::unordered_map<List, std::vector<std::unique_ptr<AggData>>, std::hash<nebula::List>> result;
for (; iter->valid(); iter->next()) {
List list;
for (auto& key : groupKeys) {
@@ -35,19 +35,22 @@ folly::Future<Status> AggregateExecutor::execute() {
auto it = result.find(list);
if (it == result.end()) {
- std::vector<std::unique_ptr<AggFun>> funs;
- for (auto& item : groupItems) {
- auto fun = AggFun::aggFunMap_[item.func](item.distinct);
- auto& v = item.expr->eval(ctx(iter.get()));
- fun->apply(v);
- funs.emplace_back(std::move(fun));
- }
- result.emplace(std::make_pair(std::move(list), std::move(funs)));
+ std::vector<std::unique_ptr<AggData>> cols;
+ for (size_t i = 0; i < groupItems.size(); ++i) {
+ cols.emplace_back(new AggData());
+ }
+ result.emplace(std::make_pair(list, std::move(cols)));
} else {
DCHECK_EQ(it->second.size(), groupItems.size());
- for (size_t i = 0; i < groupItems.size(); ++i) {
- auto& v = groupItems[i].expr->eval(ctx(iter.get()));
- it->second[i]->apply(v);
+ }
+
+ for (size_t i = 0; i < groupItems.size(); ++i) {
+ auto* item = groupItems[i];
+ if (item->kind() == Expression::Kind::kAggregate) {
+ static_cast<AggregateExpression*>(item)->setAggData(result[list][i].get());
+ item->eval(ctx(iter.get()));
+ } else {
+ result[list][i]->setResult(item->eval(ctx(iter.get())));
}
}
}
@@ -57,8 +60,8 @@ folly::Future<Status> AggregateExecutor::execute() {
ds.rows.reserve(result.size());
for (auto& kv : result) {
Row row;
- for (auto& f : kv.second) {
- row.values.emplace_back(f->getResult());
+ for (auto& v : kv.second) {
+ row.values.emplace_back(v->result());
}
ds.rows.emplace_back(std::move(row));
}
diff --git a/src/executor/test/AggregateTest.cpp b/src/executor/test/AggregateTest.cpp
index 30b4309a..bb12ce0c 100644
--- a/src/executor/test/AggregateTest.cpp
+++ b/src/executor/test/AggregateTest.cpp
@@ -85,11 +85,11 @@ struct RowCmp {
#define TEST_AGG_1(FUN, COL, DISTINCT) \
std::vector<Expression*> groupKeys; \
- std::vector<Aggregate::GroupItem> groupItems; \
+ std::vector<Expression*> groupItems; \
auto expr = \
std::make_unique<InputPropertyExpression>(new std::string("col1")); \
- Aggregate::GroupItem item(expr.get(), FUN, DISTINCT); \
- groupItems.emplace_back(std::move(item)); \
+ AggregateExpression item(new std::string(FUN), expr.release(), DISTINCT); \
+ groupItems.emplace_back(&item); \
auto* agg = Aggregate::make(qctx_.get(), nullptr, std::move(groupKeys), \
std::move(groupItems)); \
agg->setInputVar(*input_); \
@@ -105,12 +105,13 @@ struct RowCmp {
#define TEST_AGG_2(FUN, COL, DISTINCT) \
std::vector<Expression*> groupKeys; \
- std::vector<Aggregate::GroupItem> groupItems; \
- auto expr = \
+ std::vector<Expression*> groupItems; \
+ auto expr1 = \
std::make_unique<InputPropertyExpression>(new std::string("col2")); \
- groupKeys.emplace_back(expr.get()); \
- Aggregate::GroupItem item(expr.get(), FUN, DISTINCT); \
- groupItems.emplace_back(std::move(item)); \
+ auto expr2 = expr1->clone(); \
+ groupKeys.emplace_back(expr1.get()); \
+ AggregateExpression item(new std::string(FUN), expr2.release(), DISTINCT); \
+ groupItems.emplace_back(&item); \
auto* agg = Aggregate::make(qctx_.get(), nullptr, std::move(groupKeys), \
std::move(groupItems)); \
agg->setInputVar(*input_); \
@@ -128,17 +129,18 @@ struct RowCmp {
#define TEST_AGG_3(FUN, COL, DISTINCT) \
std::vector<Expression*> groupKeys; \
- std::vector<Aggregate::GroupItem> groupItems; \
- auto expr = \
- std::make_unique<InputPropertyExpression>(new std::string("col2")); \
- groupKeys.emplace_back(expr.get()); \
- Aggregate::GroupItem item(expr.get(), AggFun::Function::kNone, false); \
- groupItems.emplace_back(std::move(item)); \
+ std::vector<Expression*> groupItems; \
auto expr1 = \
- std::make_unique<InputPropertyExpression>(new std::string("col3")); \
+ std::make_unique<InputPropertyExpression>(new std::string("col2")); \
+ auto expr2 = expr1->clone(); \
groupKeys.emplace_back(expr1.get()); \
- Aggregate::GroupItem item1(expr1.get(), FUN, DISTINCT); \
- groupItems.emplace_back(std::move(item1)); \
+ AggregateExpression item(new std::string(""), expr2.release(), false); \
+ groupItems.emplace_back(&item); \
+ auto expr3 = \
+ std::make_unique<InputPropertyExpression>(new std::string("col3")); \
+ groupKeys.emplace_back(expr3.get()); \
+ AggregateExpression item1(new std::string(FUN), expr3.release(), DISTINCT); \
+ groupItems.emplace_back(&item1); \
auto* agg = Aggregate::make(qctx_.get(), nullptr, std::move(groupKeys), \
std::move(groupItems)); \
agg->setInputVar(*input_); \
@@ -156,10 +158,10 @@ struct RowCmp {
#define TEST_AGG_4(FUN, COL, DISTINCT) \
std::vector<Expression*> groupKeys; \
- std::vector<Aggregate::GroupItem> groupItems; \
+ std::vector<Expression*> groupItems; \
auto expr = std::make_unique<ConstantExpression>(1); \
- Aggregate::GroupItem item(expr.get(), FUN, DISTINCT); \
- groupItems.emplace_back(std::move(item)); \
+ AggregateExpression item(new std::string(FUN), expr.release(), DISTINCT); \
+ groupItems.emplace_back(&item); \
auto* agg = Aggregate::make(qctx_.get(), nullptr, std::move(groupKeys), \
std::move(groupItems)); \
agg->setInputVar(*input_); \
@@ -204,7 +206,7 @@ TEST_F(AggregateTest, Group) {
// key = col2
// items = col2
- TEST_AGG_2(AggFun::Function::kNone, "col2", false)
+ TEST_AGG_2("", "col2", false)
}
{
// ===============
@@ -237,7 +239,7 @@ TEST_F(AggregateTest, Group) {
// key = col2, col3
// items = col2, col3
- TEST_AGG_3(AggFun::Function::kNone, "col3", false)
+ TEST_AGG_3("", "col3", false)
}
}
@@ -260,7 +262,7 @@ TEST_F(AggregateTest, Collect) {
// key =
// items = collect(col1)
- TEST_AGG_1(AggFun::Function::kCollect, "list", false)
+ TEST_AGG_1("COLLECT", "list", false)
}
{
// ========
@@ -301,12 +303,13 @@ TEST_F(AggregateTest, Collect) {
// key = col1
// items = collect(col1)
std::vector<Expression*> groupKeys;
- std::vector<Aggregate::GroupItem> groupItems;
- auto expr =
+ std::vector<Expression*> groupItems;
+ auto expr1 =
std::make_unique<InputPropertyExpression>(new std::string("col1"));
- groupKeys.emplace_back(expr.get());
- Aggregate::GroupItem item(expr.get(), AggFun::Function::kCollect, false);
- groupItems.emplace_back(std::move(item));
+ auto expr2 = expr1->clone();
+ groupKeys.emplace_back(expr1.get());
+ AggregateExpression item(new std::string("COLLECT"), expr2.release(), false);
+ groupItems.emplace_back(std::move(&item));
auto* agg = Aggregate::make(qctx_.get(), nullptr, std::move(groupKeys),
std::move(groupItems));
agg->setInputVar(*input_);
@@ -367,7 +370,7 @@ TEST_F(AggregateTest, Collect) {
// key = col2, col3
// items = col2, collect(col3)
- TEST_AGG_3(AggFun::Function::kCollect, "list", false)
+ TEST_AGG_3("COLLECT", "list", false)
}
{
// ====================================
@@ -387,7 +390,7 @@ TEST_F(AggregateTest, Collect) {
// key =
// items = collect(1)
- TEST_AGG_4(AggFun::Function::kCollect, "list", false)
+ TEST_AGG_4("COLLECT", "list", false)
}
{
// ====================================
@@ -408,11 +411,11 @@ TEST_F(AggregateTest, Collect) {
// key =
// items = collect(distinct col1)
std::vector<Expression*> groupKeys;
- std::vector<Aggregate::GroupItem> groupItems;
+ std::vector<Expression*> groupItems;
auto expr =
std::make_unique<InputPropertyExpression>(new std::string("col1"));
- Aggregate::GroupItem item(expr.get(), AggFun::Function::kCollect, true);
- groupItems.emplace_back(std::move(item));
+ AggregateExpression item(new std::string("COLLECT"), expr.release(), true);
+ groupItems.emplace_back(std::move(&item));
auto* agg = Aggregate::make(qctx_.get(), nullptr, std::move(groupKeys),
std::move(groupItems));
agg->setInputVar(*input_);
@@ -469,12 +472,12 @@ TEST_F(AggregateTest, Collect) {
// key = col1
// items = collect(distinct col1)
std::vector<Expression*> groupKeys;
- std::vector<Aggregate::GroupItem> groupItems;
- auto expr =
+ std::vector<Expression*> groupItems;
+ auto expr1 =
std::make_unique<InputPropertyExpression>(new std::string("col1"));
- groupKeys.emplace_back(expr.get());
- Aggregate::GroupItem item(expr.get(), AggFun::Function::kCollect, true);
- groupItems.emplace_back(std::move(item));
+ auto expr2 = expr1->clone();
+ AggregateExpression item(new std::string("COLLECT"), expr2.release(), true);
+ groupItems.emplace_back(std::move(&item));
auto* agg = Aggregate::make(qctx_.get(), nullptr, std::move(groupKeys),
std::move(groupItems));
agg->setInputVar(*input_);
@@ -535,7 +538,8 @@ TEST_F(AggregateTest, Collect) {
// key = col2, col3
// items = col2, collect(distinct col3)
- TEST_AGG_3(AggFun::Function::kCollect, "list", true)
+
+ TEST_AGG_3("COLLECT", "list", true)
}
{
// ====================================
@@ -553,7 +557,7 @@ TEST_F(AggregateTest, Collect) {
// key =
// items = collect(distinct 1)
- TEST_AGG_4(AggFun::Function::kCollect, "list", true)
+ TEST_AGG_4("COLLECT", "list", true)
}
}
@@ -572,7 +576,7 @@ TEST_F(AggregateTest, Count) {
// key =
// items = count(col1)
- TEST_AGG_1(AggFun::Function::kCount, "count", false)
+ TEST_AGG_1("COUNT", "count", false)
}
{
// ========
@@ -605,7 +609,7 @@ TEST_F(AggregateTest, Count) {
// key = col2
// items = count(col2)
- TEST_AGG_2(AggFun::Function::kCount, "count", false)
+ TEST_AGG_2("COUNT", "count", false)
}
{
// ================
@@ -638,7 +642,7 @@ TEST_F(AggregateTest, Count) {
// key = col2, col3
// items = col2, count(col3)
- TEST_AGG_3(AggFun::Function::kCount, "count", false)
+ TEST_AGG_3("COUNT", "count", false)
}
{
// ========
@@ -653,7 +657,7 @@ TEST_F(AggregateTest, Count) {
// key =
// items = count(1)
- TEST_AGG_4(AggFun::Function::kCount, "count", false)
+ TEST_AGG_4("COUNT", "count", false)
}
{
// ========
@@ -669,7 +673,7 @@ TEST_F(AggregateTest, Count) {
// key =
// items = count(distinct col1)
- TEST_AGG_1(AggFun::Function::kCount, "count", true)
+ TEST_AGG_1("COUNT", "count", true)
}
{
// ========
@@ -702,7 +706,7 @@ TEST_F(AggregateTest, Count) {
// key = col2
// items = count(distinct col2)
- TEST_AGG_2(AggFun::Function::kCount, "count", true)
+ TEST_AGG_2("COUNT", "count", true)
}
{
// ================
@@ -735,7 +739,7 @@ TEST_F(AggregateTest, Count) {
// key = col2, col3
// items = col2, count(distinct col3)
- TEST_AGG_3(AggFun::Function::kCount, "count", true)
+ TEST_AGG_3("COUNT", "count", true)
}
{
// ========
@@ -750,7 +754,7 @@ TEST_F(AggregateTest, Count) {
// key =
// items = count(distinct 1)
- TEST_AGG_4(AggFun::Function::kCount, "count", true)
+ TEST_AGG_4("COUNT", "count", true)
}
}
@@ -769,7 +773,7 @@ TEST_F(AggregateTest, Sum) {
// key =
// items = sum(col1)
- TEST_AGG_1(AggFun::Function::kSum, "sum", false)
+ TEST_AGG_1("SUM", "sum", false)
}
{
// ========
@@ -800,7 +804,7 @@ TEST_F(AggregateTest, Sum) {
// key = col2
// items = sum(col2)
- TEST_AGG_2(AggFun::Function::kSum, "sum", false)
+ TEST_AGG_2("SUM", "sum", false)
}
{
// ================
@@ -833,7 +837,7 @@ TEST_F(AggregateTest, Sum) {
// key = col2, col3
// items = col2, sum(col3)
- TEST_AGG_3(AggFun::Function::kSum, "sum", false)
+ TEST_AGG_3("SUM", "sum", false)
}
{
// ========
@@ -849,7 +853,7 @@ TEST_F(AggregateTest, Sum) {
// key =
// items = sum(1)
- TEST_AGG_4(AggFun::Function::kSum, "sum", false)
+ TEST_AGG_4("SUM", "sum", false)
}
{
// ========
@@ -865,7 +869,7 @@ TEST_F(AggregateTest, Sum) {
// key =
// items = sum(distinct col1)
- TEST_AGG_1(AggFun::Function::kSum, "sum", true)
+ TEST_AGG_1("SUM", "sum", true)
}
{
// ========
@@ -896,7 +900,7 @@ TEST_F(AggregateTest, Sum) {
// key = col2
// items = sum(distinct col2)
- TEST_AGG_2(AggFun::Function::kSum, "sum", true)
+ TEST_AGG_2("SUM", "sum", true)
}
{
// ================
@@ -929,7 +933,7 @@ TEST_F(AggregateTest, Sum) {
// key = col2, col3
// items = col2, sum(distinct col3)
- TEST_AGG_3(AggFun::Function::kSum, "sum", true)
+ TEST_AGG_3("SUM", "sum", true)
}
{
// ========
@@ -945,7 +949,7 @@ TEST_F(AggregateTest, Sum) {
// key =
// items = sum(distinct 1)
- TEST_AGG_4(AggFun::Function::kSum, "sum", true)
+ TEST_AGG_4("SUM", "sum", true)
}
}
@@ -964,7 +968,7 @@ TEST_F(AggregateTest, Avg) {
// key =
// items = avg(col1)
- TEST_AGG_1(AggFun::Function::kAvg, "avg", false)
+ TEST_AGG_1("AVG", "avg", false)
}
{
// ========
@@ -995,7 +999,7 @@ TEST_F(AggregateTest, Avg) {
// key = col2
// items = avg(col2)
- TEST_AGG_2(AggFun::Function::kAvg, "avg", false)
+ TEST_AGG_2("AVG", "avg", false)
}
{
// ================
@@ -1028,7 +1032,7 @@ TEST_F(AggregateTest, Avg) {
// key = col2, col3
// items = col2, sum(col3)
- TEST_AGG_3(AggFun::Function::kAvg, "avg", false)
+ TEST_AGG_3("AVG", "avg", false)
}
{
// ========
@@ -1044,7 +1048,7 @@ TEST_F(AggregateTest, Avg) {
// key =
// items = avg(1)
- TEST_AGG_4(AggFun::Function::kAvg, "avg", false)
+ TEST_AGG_4("AVG", "avg", false)
}
{
// ========
@@ -1060,7 +1064,7 @@ TEST_F(AggregateTest, Avg) {
// key =
// items = avg(col1)
- TEST_AGG_1(AggFun::Function::kAvg, "avg", true)
+ TEST_AGG_1("AVG", "avg", true)
}
{
// ========
@@ -1091,7 +1095,7 @@ TEST_F(AggregateTest, Avg) {
// key = col2
// items = avg(col2)
- TEST_AGG_2(AggFun::Function::kAvg, "avg", true)
+ TEST_AGG_2("AVG", "avg", true)
}
{
// ================
@@ -1124,7 +1128,7 @@ TEST_F(AggregateTest, Avg) {
// key = col2, col3
// items = col2, sum(col3)
- TEST_AGG_3(AggFun::Function::kAvg, "avg", true)
+ TEST_AGG_3("AVG", "avg", true)
}
{
// ========
@@ -1140,7 +1144,7 @@ TEST_F(AggregateTest, Avg) {
// key =
// items = avg(1)
- TEST_AGG_4(AggFun::Function::kAvg, "avg", true)
+ TEST_AGG_4("AVG", "avg", true)
}
}
@@ -1159,7 +1163,7 @@ TEST_F(AggregateTest, Max) {
// key =
// items = max(col1)
- TEST_AGG_1(AggFun::Function::kMax, "max", false)
+ TEST_AGG_1("MAX", "max", false)
}
{
// ================
@@ -1192,7 +1196,7 @@ TEST_F(AggregateTest, Max) {
// key = col2, col3
// items = col2, max(col3)
- TEST_AGG_3(AggFun::Function::kMax, "max", false)
+ TEST_AGG_3("MAX", "max", false)
}
{
// ========
@@ -1208,7 +1212,7 @@ TEST_F(AggregateTest, Max) {
// key =
// items = max(1)
- TEST_AGG_4(AggFun::Function::kMax, "max", false)
+ TEST_AGG_4("MAX", "max", false)
}
{
// ========
@@ -1224,7 +1228,7 @@ TEST_F(AggregateTest, Max) {
// key =
// items = max(col1)
- TEST_AGG_1(AggFun::Function::kMax, "max", true)
+ TEST_AGG_1("MAX", "max", true)
}
{
// ================
@@ -1257,7 +1261,7 @@ TEST_F(AggregateTest, Max) {
// key = col2, col3
// items = col2, max(col3)
- TEST_AGG_3(AggFun::Function::kMax, "max", true)
+ TEST_AGG_3("MAX", "max", true)
}
{
// ========
@@ -1273,7 +1277,7 @@ TEST_F(AggregateTest, Max) {
// key =
// items = max(1)
- TEST_AGG_4(AggFun::Function::kMax, "max", true)
+ TEST_AGG_4("MAX", "max", true)
}
}
@@ -1292,7 +1296,7 @@ TEST_F(AggregateTest, Min) {
// key =
// items = min(col1)
- TEST_AGG_1(AggFun::Function::kMin, "min", false)
+ TEST_AGG_1("MIN", "min", false)
}
{
// ================
@@ -1325,7 +1329,7 @@ TEST_F(AggregateTest, Min) {
// key = col2, col3
// items = col2, min(col3)
- TEST_AGG_3(AggFun::Function::kMin, "min", false)
+ TEST_AGG_3("MIN", "min", false)
}
{
// ========
@@ -1341,7 +1345,7 @@ TEST_F(AggregateTest, Min) {
// key =
// items = min(1)
- TEST_AGG_4(AggFun::Function::kMin, "min", false)
+ TEST_AGG_4("MIN", "min", false)
}
{
// ========
@@ -1357,7 +1361,7 @@ TEST_F(AggregateTest, Min) {
// key =
// items = min(distinct col1)
- TEST_AGG_1(AggFun::Function::kMin, "min", true)
+ TEST_AGG_1("MIN", "min", true)
}
{
// ================
@@ -1390,7 +1394,7 @@ TEST_F(AggregateTest, Min) {
// key = col2, col3
// items = col2, min(distinct col3)
- TEST_AGG_3(AggFun::Function::kMin, "min", true)
+ TEST_AGG_3("MIN", "min", true)
}
{
// ========
@@ -1406,7 +1410,7 @@ TEST_F(AggregateTest, Min) {
// key =
// items = min(distinct 1)
- TEST_AGG_4(AggFun::Function::kMin, "min", true)
+ TEST_AGG_4("MIN", "min", true)
}
}
@@ -1425,7 +1429,7 @@ TEST_F(AggregateTest, Stdev) {
// key =
// items = stdev(col1)
- TEST_AGG_1(AggFun::Function::kStdev, "stdev", false)
+ TEST_AGG_1("STD", "stdev", false)
}
{
// ===============
@@ -1455,7 +1459,7 @@ TEST_F(AggregateTest, Stdev) {
// key = col2
// items = stdev(col2)
- TEST_AGG_2(AggFun::Function::kStdev, "stdev", false)
+ TEST_AGG_2("STD", "stdev", false)
}
{
// =======================
@@ -1488,7 +1492,7 @@ TEST_F(AggregateTest, Stdev) {
// key = col2, col3
// items = col2, stdev(col3)
- TEST_AGG_3(AggFun::Function::kStdev, "stdev", false)
+ TEST_AGG_3("STD", "stdev", false)
}
{
// ===============
@@ -1503,7 +1507,7 @@ TEST_F(AggregateTest, Stdev) {
// key =
// items = stdev(1)
- TEST_AGG_4(AggFun::Function::kStdev, "stdev", false)
+ TEST_AGG_4("STD", "stdev", false)
}
{
// ===============
@@ -1519,7 +1523,7 @@ TEST_F(AggregateTest, Stdev) {
// key =
// items = stdev(distinct col1)
- TEST_AGG_1(AggFun::Function::kStdev, "stdev", true)
+ TEST_AGG_1("STD", "stdev", true)
}
{
// ===============
@@ -1549,7 +1553,7 @@ TEST_F(AggregateTest, Stdev) {
// key = col2
// items = stdev(distinct col2)
- TEST_AGG_2(AggFun::Function::kStdev, "stdev", true)
+ TEST_AGG_2("STD", "stdev", true)
}
{
// =======================
@@ -1582,7 +1586,7 @@ TEST_F(AggregateTest, Stdev) {
// key = col2, col3
// items = col2, stdev(distinct col3)
- TEST_AGG_3(AggFun::Function::kStdev, "stdev", true)
+ TEST_AGG_3("STD", "stdev", true)
}
{
// ===============
@@ -1598,7 +1602,7 @@ TEST_F(AggregateTest, Stdev) {
// key =
// items = stdev(distinct 1)
- TEST_AGG_4(AggFun::Function::kStdev, "stdev", true)
+ TEST_AGG_4("STD", "stdev", true)
}
}
@@ -1617,7 +1621,7 @@ TEST_F(AggregateTest, BitAnd) {
// key =
// items = bit_and(col1)
- TEST_AGG_1(AggFun::Function::kBitAnd, "bit_and", false)
+ TEST_AGG_1("BIT_AND", "bit_and", false)
}
{
// ===============
@@ -1648,7 +1652,7 @@ TEST_F(AggregateTest, BitAnd) {
// key = col2
// items = bit_and(col2)
- TEST_AGG_2(AggFun::Function::kBitAnd, "bit_and", false)
+ TEST_AGG_2("BIT_AND", "bit_and", false)
}
{
// =======================
@@ -1681,7 +1685,7 @@ TEST_F(AggregateTest, BitAnd) {
// key = col2, col3
// items = col2, bit_and(col3)
- TEST_AGG_3(AggFun::Function::kBitAnd, "bit_and", false)
+ TEST_AGG_3("BIT_AND", "bit_and", false)
}
{
// ===============
@@ -1696,7 +1700,7 @@ TEST_F(AggregateTest, BitAnd) {
// key =
// items = bit_and(1)
- TEST_AGG_4(AggFun::Function::kBitAnd, "bit_and", false)
+ TEST_AGG_4("BIT_AND", "bit_and", false)
}
{
// ===============
@@ -1712,7 +1716,7 @@ TEST_F(AggregateTest, BitAnd) {
// key =
// items = bit_and(distinct col1)
- TEST_AGG_1(AggFun::Function::kBitAnd, "bit_and", true)
+ TEST_AGG_1("BIT_AND", "bit_and", true)
}
{
// ===============
@@ -1743,7 +1747,7 @@ TEST_F(AggregateTest, BitAnd) {
// key = col2
// items = bit_and(col2)
- TEST_AGG_2(AggFun::Function::kBitAnd, "bit_and", true)
+ TEST_AGG_2("BIT_AND", "bit_and", true)
}
{
// =======================
@@ -1776,7 +1780,7 @@ TEST_F(AggregateTest, BitAnd) {
// key = col2, col3
// items = col2, bit_and(col3)
- TEST_AGG_3(AggFun::Function::kBitAnd, "bit_and", true)
+ TEST_AGG_3("BIT_AND", "bit_and", true)
}
{
// ===============
@@ -1792,7 +1796,7 @@ TEST_F(AggregateTest, BitAnd) {
// key =
// items = bit_and(1)
- TEST_AGG_4(AggFun::Function::kBitAnd, "bit_and", true)
+ TEST_AGG_4("BIT_AND", "bit_and", true)
}
}
@@ -1811,7 +1815,7 @@ TEST_F(AggregateTest, BitOr) {
// key =
// items = bit_or(col1)
- TEST_AGG_1(AggFun::Function::kBitOr, "bit_or", false)
+ TEST_AGG_1("BIT_OR", "bit_or", false)
}
{
// ===============
@@ -1842,7 +1846,7 @@ TEST_F(AggregateTest, BitOr) {
// key = col2
// items = bit_or(col2)
- TEST_AGG_2(AggFun::Function::kBitOr, "bit_or", false)
+ TEST_AGG_2("BIT_OR", "bit_or", false)
}
{
// =======================
@@ -1875,7 +1879,7 @@ TEST_F(AggregateTest, BitOr) {
// key = col2, col3
// items = col2, bit_or(col3)
- TEST_AGG_3(AggFun::Function::kBitOr, "bit_or", false)
+ TEST_AGG_3("BIT_OR", "bit_or", false)
}
{
// ===============
@@ -1891,7 +1895,7 @@ TEST_F(AggregateTest, BitOr) {
// key =
// items = bit_or(1)
- TEST_AGG_4(AggFun::Function::kBitOr, "bit_or", false)
+ TEST_AGG_4("BIT_OR", "bit_or", false)
}
{
// ===============
@@ -1907,7 +1911,7 @@ TEST_F(AggregateTest, BitOr) {
// key =
// items = bit_or(distinct col1)
- TEST_AGG_1(AggFun::Function::kBitOr, "bit_or", true)
+ TEST_AGG_1("BIT_OR", "bit_or", true)
}
{
// ===============
@@ -1938,7 +1942,7 @@ TEST_F(AggregateTest, BitOr) {
// key = col2
// items = bit_or(distinct col2)
- TEST_AGG_2(AggFun::Function::kBitOr, "bit_or", true)
+ TEST_AGG_2("BIT_OR", "bit_or", true)
}
{
// =======================
@@ -1971,7 +1975,7 @@ TEST_F(AggregateTest, BitOr) {
// key = col2, col3
// items = col2, bit_or(distinct col3)
- TEST_AGG_3(AggFun::Function::kBitOr, "bit_or", true)
+ TEST_AGG_3("BIT_OR", "bit_or", true)
}
{
// ===============
@@ -1987,7 +1991,7 @@ TEST_F(AggregateTest, BitOr) {
// key =
// items = bit_or(distinct 1)
- TEST_AGG_4(AggFun::Function::kBitOr, "bit_or", true)
+ TEST_AGG_4("BIT_OR", "bit_or", true)
}
}
@@ -2006,7 +2010,7 @@ TEST_F(AggregateTest, BitXor) {
// key =
// items = bit_xor(col1)
- TEST_AGG_1(AggFun::Function::kBitXor, "bit_xor", false)
+ TEST_AGG_1("BIT_XOR", "bit_xor", false)
}
{
// ===============
@@ -2037,7 +2041,7 @@ TEST_F(AggregateTest, BitXor) {
// key = col2
// items = bit_xor(col2)
- TEST_AGG_2(AggFun::Function::kBitXor, "bit_xor", false)
+ TEST_AGG_2("BIT_XOR", "bit_xor", false)
}
{
// =======================
@@ -2070,7 +2074,7 @@ TEST_F(AggregateTest, BitXor) {
// key = col2, col3
// items = col2, bit_xor(col3)
- TEST_AGG_3(AggFun::Function::kBitXor, "bit_xor", false)
+ TEST_AGG_3("BIT_XOR", "bit_xor", false)
}
{
// ===============
@@ -2086,7 +2090,7 @@ TEST_F(AggregateTest, BitXor) {
// key =
// items = bit_xor(1)
- TEST_AGG_4(AggFun::Function::kBitXor, "bit_xor", false)
+ TEST_AGG_4("BIT_XOR", "bit_xor", false)
}
{
// ===============
@@ -2102,7 +2106,7 @@ TEST_F(AggregateTest, BitXor) {
// key =
// items = bit_xor(distinct col1)
- TEST_AGG_1(AggFun::Function::kBitXor, "bit_xor", true)
+ TEST_AGG_1("BIT_XOR", "bit_xor", true)
}
{
// ===============
@@ -2133,7 +2137,7 @@ TEST_F(AggregateTest, BitXor) {
// key = col2
// items = bit_xor(distinct col2)
- TEST_AGG_2(AggFun::Function::kBitXor, "bit_xor", true)
+ TEST_AGG_2("BIT_XOR", "bit_xor", true)
}
{
// =======================
@@ -2166,7 +2170,7 @@ TEST_F(AggregateTest, BitXor) {
// key = col2, col3
// items = col2, bit_xor(distinct col3)
- TEST_AGG_3(AggFun::Function::kBitXor, "bit_xor", true)
+ TEST_AGG_3("BIT_XOR", "bit_xor", true)
}
{
// ===============
@@ -2182,7 +2186,7 @@ TEST_F(AggregateTest, BitXor) {
// key =
// items = bit_xor(distinct 1)
- TEST_AGG_4(AggFun::Function::kBitXor, "bit_xor", true)
+ TEST_AGG_4("BIT_XOR", "bit_xor", true)
}
}
} // namespace graph
diff --git a/src/executor/test/CMakeLists.txt b/src/executor/test/CMakeLists.txt
index a814808d..f2b17693 100644
--- a/src/executor/test/CMakeLists.txt
+++ b/src/executor/test/CMakeLists.txt
@@ -27,7 +27,6 @@ SET(EXEC_QUERY_TEST_OBJS
$<TARGET_OBJECTS:common_file_based_cluster_id_man_obj>
$<TARGET_OBJECTS:common_charset_obj>
$<TARGET_OBJECTS:common_function_manager_obj>
- $<TARGET_OBJECTS:common_agg_function_obj>
$<TARGET_OBJECTS:common_encryption_obj>
$<TARGET_OBJECTS:common_http_client_obj>
$<TARGET_OBJECTS:common_time_utils_obj>
diff --git a/src/optimizer/test/CMakeLists.txt b/src/optimizer/test/CMakeLists.txt
index 14fe3950..ba3c961f 100644
--- a/src/optimizer/test/CMakeLists.txt
+++ b/src/optimizer/test/CMakeLists.txt
@@ -26,7 +26,6 @@ set(OPTIMIZER_TEST_LIB
$<TARGET_OBJECTS:common_encryption_obj>
$<TARGET_OBJECTS:common_http_client_obj>
$<TARGET_OBJECTS:common_process_obj>
- $<TARGET_OBJECTS:common_agg_function_obj>
$<TARGET_OBJECTS:common_time_utils_obj>
$<TARGET_OBJECTS:common_graph_obj>
$<TARGET_OBJECTS:common_ft_es_graph_adapter_obj>
diff --git a/src/parser/AdminSentences.cpp b/src/parser/AdminSentences.cpp
index bd036555..d00285ae 100644
--- a/src/parser/AdminSentences.cpp
+++ b/src/parser/AdminSentences.cpp
@@ -185,7 +185,7 @@ std::string AdminJobSentence::toString() const {
case meta::cpp2::AdminJobOp::RECOVER:
return "recover job";
}
- LOG(FATAL) << "Unkown job operation " << static_cast<uint8_t>(op_);
+ LOG(FATAL) << "Unknown job operation " << static_cast<uint8_t>(op_);
}
meta::cpp2::AdminJobOp AdminJobSentence::getOp() const {
diff --git a/src/parser/Clauses.cpp b/src/parser/Clauses.cpp
index 9044188c..73104504 100644
--- a/src/parser/Clauses.cpp
+++ b/src/parser/Clauses.cpp
@@ -115,14 +115,7 @@ std::string WhereClause::toString() const {
std::string YieldColumn::toString() const {
std::string buf;
buf.reserve(256);
- if (aggFunName_ != nullptr) {
- buf += *aggFunName_;
- buf += "(";
- buf += expr_->toString();
- buf += ")";
- } else {
- buf += expr_->toString();
- }
+ buf += expr_->toString();
if (alias_ != nullptr) {
buf += " AS ";
@@ -162,10 +155,6 @@ bool operator==(const YieldColumn &l, const YieldColumn &r) {
return false;
}
- if (l.getAggFunName() != r.getAggFunName()) {
- return false;
- }
-
return true;
}
diff --git a/src/parser/Clauses.h b/src/parser/Clauses.h
index f20b7fb8..856f6ca5 100644
--- a/src/parser/Clauses.h
+++ b/src/parser/Clauses.h
@@ -203,6 +203,10 @@ public:
filter_.reset(expr);
}
+ std::unique_ptr<WhereClause> clone() const {
+ return std::make_unique<WhereClause>(filter_->clone().release());
+ }
+
std::string toString() const;
private:
@@ -223,9 +227,6 @@ public:
if (alias_ != nullptr) {
col->setAlias(new std::string(*alias_));
}
- if (aggFunName_ != nullptr) {
- col->setAggFunction(new std::string(*aggFunName_));
- }
return col;
}
@@ -245,26 +246,11 @@ public:
return alias_.get();
}
- void setAggFunction(std::string* fun = nullptr) {
- if (fun == nullptr) {
- return;
- }
- aggFunName_.reset(fun);
- }
-
- std::string getAggFunName() const {
- if (aggFunName_ == nullptr) {
- return "";
- }
- return *aggFunName_;
- }
-
std::string toString() const;
private:
std::unique_ptr<Expression> expr_;
std::unique_ptr<std::string> alias_;
- std::unique_ptr<std::string> aggFunName_{nullptr};
};
bool operator==(const YieldColumn &l, const YieldColumn &r);
@@ -294,6 +280,14 @@ public:
return columns_.empty();
}
+ std::unique_ptr<YieldColumns> clone() const {
+ auto cols = std::make_unique<YieldColumns>();
+ for (auto& col : columns_) {
+ cols->addColumn(col->clone().release());
+ }
+ return cols;
+ }
+
std::string toString() const;
const YieldColumn* back() const {
@@ -328,6 +322,11 @@ public:
return distinct_;
}
+ std::unique_ptr<YieldClause> clone() const {
+ auto cols = yieldColumns_->clone();
+ return std::make_unique<YieldClause>(cols.release(), distinct_);
+ }
+
std::string toString() const;
private:
diff --git a/src/parser/TraverseSentences.cpp b/src/parser/TraverseSentences.cpp
index 9c689d63..c4312ddc 100644
--- a/src/parser/TraverseSentences.cpp
+++ b/src/parser/TraverseSentences.cpp
@@ -6,6 +6,7 @@
#include "common/base/Base.h"
#include "parser/TraverseSentences.h"
+#include "util/ExpressionUtils.h"
namespace nebula {
@@ -104,7 +105,7 @@ std::string OrderFactor::toString() const {
case DESCEND:
return folly::stringPrintf("%s DESC,", expr_->toString().c_str());
default:
- LOG(FATAL) << "Unkown Order Type: " << orderType_;
+ LOG(FATAL) << "Unknown Order Type: " << orderType_;
}
}
@@ -214,6 +215,15 @@ std::string LimitSentence::toString() const {
return folly::stringPrintf("LIMIT %ld,%ld", offset_, count_);
}
+bool YieldSentence::hasAgg() const {
+ for (auto* col : columns()) {
+ if (graph::ExpressionUtils::findAny(col->expr(), {Expression::Kind::kAggregate})) {
+ return true;
+ }
+ }
+ return false;
+}
+
std::string YieldSentence::toString() const {
std::string buf;
buf.reserve(256);
diff --git a/src/parser/TraverseSentences.h b/src/parser/TraverseSentences.h
index f3baece9..e23d4797 100644
--- a/src/parser/TraverseSentences.h
+++ b/src/parser/TraverseSentences.h
@@ -554,6 +554,7 @@ public:
return yieldClause_.get();
}
+ bool hasAgg() const;
std::string toString() const override;
private:
@@ -563,14 +564,23 @@ private:
class GroupBySentence final : public Sentence {
public:
- GroupBySentence() {
+ GroupBySentence(YieldClause* yield = nullptr,
+ WhereClause* having = nullptr,
+ GroupClause* groupby = nullptr) {
kind_ = Kind::kGroupBy;
+ yieldClause_.reset(yield);
+ havingClause_.reset(having);
+ groupClause_.reset(groupby);
}
void setGroupClause(GroupClause *clause) {
groupClause_.reset(clause);
}
+ void setHavingClause(WhereClause *clause) {
+ havingClause_.reset(clause);
+ }
+
void setYieldClause(YieldClause *clause) {
yieldClause_.reset(clause);
}
@@ -579,6 +589,10 @@ public:
return groupClause_.get();
}
+ const WhereClause* havingClause() const {
+ return havingClause_.get();
+ }
+
const YieldClause* yieldClause() const {
return yieldClause_.get();
}
@@ -586,8 +600,9 @@ public:
std::string toString() const override;
private:
- std::unique_ptr<GroupClause> groupClause_;
std::unique_ptr<YieldClause> yieldClause_;
+ std::unique_ptr<WhereClause> havingClause_;
+ std::unique_ptr<GroupClause> groupClause_;
};
class GetSubgraphSentence final : public Sentence {
diff --git a/src/parser/parser.yy b/src/parser/parser.yy
index 814adbdb..6f1c2833 100644
--- a/src/parser/parser.yy
+++ b/src/parser/parser.yy
@@ -24,6 +24,8 @@
#include "common/expression/CaseExpression.h"
#include "common/expression/TextSearchExpression.h"
#include "common/expression/ListComprehensionExpression.h"
+#include "common/expression/AggregateExpression.h"
+
#include "util/SchemaUtil.h"
#include "util/ParserUtil.h"
#include "context/QueryContext.h"
@@ -213,6 +215,7 @@ static constexpr size_t MAX_ABS_INTEGER = 9223372036854775808ULL;
%type <expr> case_expression
%type <expr> list_comprehension_expression
%type <expr> compound_expression
+%type <expr> aggregate_expression
%type <expr> text_search_expression
%type <argument_list> argument_list opt_argument_list
%type <type> type_spec
@@ -490,7 +493,6 @@ unreserved_keyword
agg_function
: KW_COUNT { $$ = new std::string("COUNT"); }
- | KW_COUNT_DISTINCT { $$ = new std::string("COUNT_DISTINCT"); }
| KW_SUM { $$ = new std::string("SUM"); }
| KW_AVG { $$ = new std::string("AVG"); }
| KW_MAX { $$ = new std::string("MAX"); }
@@ -527,6 +529,9 @@ expression
| compound_expression {
$$ = $1;
}
+ | aggregate_expression {
+ $$ = $1;
+ }
| MINUS {
scanner.setUnaryMinus(true);
} expression %prec UNARY_MINUS {
@@ -834,6 +839,23 @@ function_call_expression
}
;
+aggregate_expression
+ : agg_function L_PAREN expression R_PAREN {
+ $$ = new AggregateExpression($1, $3, false/*distinct*/);
+ }
+ | agg_function L_PAREN KW_DISTINCT expression R_PAREN {
+ $$ = new AggregateExpression($1, $4, true/*distinct*/);
+ }
+ | agg_function L_PAREN STAR R_PAREN {
+ auto star = new ConstantExpression(std::string("*"));
+ $$ = new AggregateExpression($1, star, false/*distinct*/);
+ }
+ | agg_function L_PAREN KW_DISTINCT STAR R_PAREN {
+ auto star = new ConstantExpression(std::string("*"));
+ $$ = new AggregateExpression($1, star, true/*distinct*/);
+ }
+ ;
+
uuid_expression
: KW_UUID L_PAREN STRING R_PAREN {
$$ = new UUIDExpression($3);
@@ -1148,28 +1170,6 @@ yield_column
| expression KW_AS name_label {
$$ = new YieldColumn($1, $3);
}
- | agg_function L_PAREN expression R_PAREN {
- auto yield = new YieldColumn($3);
- yield->setAggFunction($1);
- $$ = yield;
- }
- | agg_function L_PAREN expression R_PAREN KW_AS name_label {
- auto yield = new YieldColumn($3, $6);
- yield->setAggFunction($1);
- $$ = yield;
- }
- | agg_function L_PAREN STAR R_PAREN {
- auto expr = new ConstantExpression(std::string("*"));
- auto yield = new YieldColumn(expr);
- yield->setAggFunction($1);
- $$ = yield;
- }
- | agg_function L_PAREN STAR R_PAREN KW_AS name_label {
- auto expr = new ConstantExpression(std::string("*"));
- auto yield = new YieldColumn(expr, $6);
- yield->setAggFunction($1);
- $$ = yield;
- }
;
group_clause
diff --git a/src/parser/test/CMakeLists.txt b/src/parser/test/CMakeLists.txt
index 7d64edd4..7eba4edd 100644
--- a/src/parser/test/CMakeLists.txt
+++ b/src/parser/test/CMakeLists.txt
@@ -17,7 +17,6 @@ set(PARSER_TEST_LIBS
$<TARGET_OBJECTS:common_datatypes_obj>
$<TARGET_OBJECTS:common_base_obj>
$<TARGET_OBJECTS:common_function_manager_obj>
- $<TARGET_OBJECTS:common_agg_function_obj>
$<TARGET_OBJECTS:common_meta_thrift_obj>
$<TARGET_OBJECTS:common_graph_thrift_obj>
$<TARGET_OBJECTS:common_http_client_obj>
diff --git a/src/parser/test/ParserTest.cpp b/src/parser/test/ParserTest.cpp
index 49da92d4..5fd931e9 100644
--- a/src/parser/test/ParserTest.cpp
+++ b/src/parser/test/ParserTest.cpp
@@ -2061,15 +2061,67 @@ TEST(Parser, GroupBy) {
{
GQLParser parser;
std::string query = "GO FROM \"1\" OVER work "
- "YIELD $$.company.name, $^.person.name "
- "| GROUP BY $$.company.name "
- "YIELD $$.company.name as name, "
+ "YIELD $$.company.name, $^.person.name AS name "
+ "| GROUP BY $$.company.name , abs($$.company.age+1)"
+ "YIELD $$.company.name AS name, "
+ "COUNT($^.person.name ), "
+ "SUM($^.person.name ), "
+ "AVG($^.person.name ), "
+ "(INT)abs($$.company.age+1), "
+ "(INT)COUNT(DISTINCT $^.person.name ), "
+ "COUNT(DISTINCT $-.name )+1, "
+ "abs(SUM(DISTINCT $$.person.name )), "
+ "AVG(DISTINCT $^.person.name ), "
+ "AVG(DISTINCT $-.name ), "
+ "COUNT(*), "
+ "COUNT(DISTINCT *), "
+ "MAX($^.person.name ), "
+ "MIN($$.person.name ), "
+ "STD($^.person.name ), "
+ "BIT_AND($^.person.name ), "
+ "BIT_OR($$.person.name ), "
+ "BIT_XOR($^.person.name ),"
+ "STD(DISTINCT $^.person.name ), "
+ "BIT_AND(DISTINCT $$.person.name ), "
+ "BIT_OR(DISTINCT $^.person.name ), "
+ "BIT_XOR(DISTINCT $$.person.name ),"
+ "BIT_XOR(DISTINCT $-.name ),"
+ "F_STD($^.person.name ), "
+ "F_BIT_AND($^.person.name ), "
+ "F_BIT_OR($^.person.name ), "
+ "F_BIT_XOR($^.person.name )";
+
+ auto result = parser.parse(query);
+ ASSERT_TRUE(result.ok()) << result.status();
+ }
+ {
+ GQLParser parser;
+ std::string query = "GO FROM \"1\" OVER work "
+ "YIELD $$.company.name, $^.person.name AS name "
+ "| YIELD $$.company.name AS name, "
+ " abs($$.company.age+1), "
"COUNT($^.person.name ), "
- "COUNT_DISTINCT($^.person.name ), "
"SUM($^.person.name ), "
"AVG($^.person.name ), "
+ "(INT)abs($$.company.age+1), "
+ "(INT)COUNT(DISTINCT $^.person.name ), "
+ "COUNT(DISTINCT $-.name )+1, "
+ "abs(SUM(DISTINCT $$.person.name )), "
+ "AVG(DISTINCT $^.person.name ), "
+ "AVG(DISTINCT $-.name ), "
+ "COUNT(*), "
+ "COUNT(DISTINCT *), "
"MAX($^.person.name ), "
- "MIN($^.person.name ), "
+ "MIN($$.person.name ), "
+ "STD($^.person.name ), "
+ "BIT_AND($^.person.name ), "
+ "BIT_OR($$.person.name ), "
+ "BIT_XOR($^.person.name ),"
+ "STD(DISTINCT $^.person.name ), "
+ "BIT_AND(DISTINCT $$.person.name ), "
+ "BIT_OR(DISTINCT $^.person.name ), "
+ "BIT_XOR(DISTINCT $$.person.name ),"
+ "BIT_XOR(DISTINCT $-.name ),"
"F_STD($^.person.name ), "
"F_BIT_AND($^.person.name ), "
"F_BIT_OR($^.person.name ), "
diff --git a/src/planner/Query.cpp b/src/planner/Query.cpp
index c9411c7f..713d752d 100644
--- a/src/planner/Query.cpp
+++ b/src/planner/Query.cpp
@@ -200,11 +200,9 @@ std::unique_ptr<PlanNodeDescription> Aggregate::explain() const {
auto desc = SingleInputNode::explain();
addDescription("groupKeys", folly::toJson(util::toJson(groupKeys_)), desc.get());
folly::dynamic itemArr = folly::dynamic::array();
- for (const auto& item : groupItems_) {
+ for (auto* item : groupItems_) {
folly::dynamic itemObj = folly::dynamic::object();
- itemObj.insert("distinct", util::toJson(item.distinct));
- itemObj.insert("funcType", static_cast<uint8_t>(item.func));
- itemObj.insert("expr", item.expr ? item.expr->toString() : "");
+ itemObj.insert("expr", item? item->toString() : "");
itemArr.push_back(itemObj);
}
addDescription("groupItems", folly::toJson(itemArr), desc.get());
diff --git a/src/planner/Query.h b/src/planner/Query.h
index 223ea642..ab6751c0 100644
--- a/src/planner/Query.h
+++ b/src/planner/Query.h
@@ -8,7 +8,7 @@
#define PLANNER_QUERY_H_
#include "common/base/Base.h"
-#include "common/function/AggregateFunction.h"
+#include "common/expression/AggregateExpression.h"
#include "common/interface/gen-cpp2/storage_types.h"
#include "context/QueryContext.h"
#include "parser/Clauses.h"
@@ -804,18 +804,10 @@ private:
*/
class Aggregate final : public SingleInputNode {
public:
- struct GroupItem {
- GroupItem(Expression* e, AggFun::Function f, bool d)
- : expr(e), func(f), distinct(d) {}
- Expression* expr;
- AggFun::Function func;
- bool distinct = false;
- };
-
static Aggregate* make(QueryContext* qctx,
PlanNode* input,
std::vector<Expression*>&& groupKeys,
- std::vector<GroupItem>&& groupItems) {
+ std::vector<Expression*>&& groupItems) {
return qctx->objPool()->add(
new Aggregate(qctx, input, std::move(groupKeys), std::move(groupItems)));
}
@@ -824,7 +816,7 @@ public:
return groupKeys_;
}
- const std::vector<GroupItem>& groupItems() const {
+ const std::vector<Expression*>& groupItems() const {
return groupItems_;
}
@@ -834,7 +826,7 @@ private:
Aggregate(QueryContext* qctx,
PlanNode* input,
std::vector<Expression*>&& groupKeys,
- std::vector<GroupItem>&& groupItems)
+ std::vector<Expression*>&& groupItems)
: SingleInputNode(qctx, Kind::kAggregate, input) {
groupKeys_ = std::move(groupKeys);
groupItems_ = std::move(groupItems);
@@ -842,7 +834,7 @@ private:
private:
std::vector<Expression*> groupKeys_;
- std::vector<GroupItem> groupItems_;
+ std::vector<Expression*> groupItems_;
};
class SwitchSpace final : public SingleInputNode {
diff --git a/src/planner/match/StartVidFinder.cpp b/src/planner/match/StartVidFinder.cpp
index a8db5e6d..eeba83d0 100644
--- a/src/planner/match/StartVidFinder.cpp
+++ b/src/planner/match/StartVidFinder.cpp
@@ -33,7 +33,7 @@ StatusOr<SubPlan> StartVidFinder::transform(PatternContext* patternCtx) {
return transformEdge(edgeCtx);
}
- return Status::Error("Unkown pattern kind.");
+ return Status::Error("Unknown pattern kind.");
}
} // namespace graph
diff --git a/src/util/ExpressionUtils.h b/src/util/ExpressionUtils.h
index 9e549863..2523024c 100644
--- a/src/util/ExpressionUtils.h
+++ b/src/util/ExpressionUtils.h
@@ -10,6 +10,7 @@
#include "common/expression/BinaryExpression.h"
#include "common/expression/Expression.h"
#include "common/expression/FunctionCallExpression.h"
+#include "common/expression/AggregateExpression.h"
#include "common/expression/LabelExpression.h"
#include "common/expression/PropertyExpression.h"
#include "common/expression/TypeCastingExpression.h"
diff --git a/src/util/test/CMakeLists.txt b/src/util/test/CMakeLists.txt
index 98ae1729..d71c6c17 100644
--- a/src/util/test/CMakeLists.txt
+++ b/src/util/test/CMakeLists.txt
@@ -26,7 +26,6 @@ nebula_add_test(
$<TARGET_OBJECTS:common_encryption_obj>
$<TARGET_OBJECTS:common_http_client_obj>
$<TARGET_OBJECTS:common_process_obj>
- $<TARGET_OBJECTS:common_agg_function_obj>
$<TARGET_OBJECTS:common_time_utils_obj>
$<TARGET_OBJECTS:common_graph_obj>
$<TARGET_OBJECTS:common_ft_es_graph_adapter_obj>
diff --git a/src/validator/GetSubgraphValidator.cpp b/src/validator/GetSubgraphValidator.cpp
index ea176d82..d45746db 100644
--- a/src/validator/GetSubgraphValidator.cpp
+++ b/src/validator/GetSubgraphValidator.cpp
@@ -225,15 +225,14 @@ Status GetSubgraphValidator::zeroStep(PlanNode* depend, const std::string& input
getVertex->setInputVar(inputVar);
auto var = vctx_->anonVarGen()->getVar();
- auto* column = new YieldColumn(new VertexExpression(), new std::string("_vertices"));
- qctx_->objPool()->add(column);
- column->setAggFunction(new std::string("COLLECT"));
- auto fun = column->getAggFunName();
+ auto* column = new VertexExpression();
+ auto* func = new AggregateExpression(new std::string("COLLECT"), column, false);
+ qctx_->objPool()->add(func);
auto* collectVertex =
Aggregate::make(qctx_,
getVertex,
{},
- {Aggregate::GroupItem(column->expr(), AggFun::nameIdMap_[fun], false)});
+ {func});
collectVertex->setInputVar(getVertex->outputVar());
collectVertex->setColNames({"_vertices"});
@@ -259,17 +258,14 @@ Status GetSubgraphValidator::toPlan() {
startVidsVar = dedupStartVid->outputVar();
// collect runtime startVids
auto var = vctx_->anonVarGen()->getVar();
- auto* column = new YieldColumn(
- new VariablePropertyExpression(new std::string(var), new std::string(kVid)),
- new std::string(kVid));
- qctx_->objPool()->add(column);
- column->setAggFunction(new std::string("COLLECT_SET"));
- auto fun = column->getAggFunName();
+ auto* column = new VariablePropertyExpression(new std::string(var), new std::string(kVid));
+ auto* func = new AggregateExpression(new std::string("COLLECT_SET"), column, true);
+ qctx_->objPool()->add(func);
collectRunTimeStartVids =
Aggregate::make(qctx_,
dedupStartVid,
{},
- {Aggregate::GroupItem(column->expr(), AggFun::nameIdMap_[fun], true)});
+ {func});
collectRunTimeStartVids->setInputVar(dedupStartVid->outputVar());
collectRunTimeStartVids->setColNames({kVid});
runtimeStartVar_ = collectRunTimeStartVids->outputVar();
@@ -298,17 +294,14 @@ Status GetSubgraphValidator::toPlan() {
auto* projectVids = projectDstVidsFromGN(gn, startVidsVar);
auto var = vctx_->anonVarGen()->getVar();
- auto* column =
- new YieldColumn(new VariablePropertyExpression(new std::string(var), new std::string(kVid)),
- new std::string(kVid));
- qctx_->objPool()->add(column);
- column->setAggFunction(new std::string("COLLECT_SET"));
- auto fun = column->getAggFunName();
+ auto* column = new VariablePropertyExpression(new std::string(var), new std::string(kVid));
+ auto* func = new AggregateExpression(new std::string("COLLECT_SET"), column, true);
+ qctx_->objPool()->add(func);
auto* collect =
Aggregate::make(qctx_,
projectVids,
{},
- {Aggregate::GroupItem(column->expr(), AggFun::nameIdMap_[fun], true)});
+ {func});
collect->setInputVar(projectVids->outputVar());
collect->setColNames({kVid});
collectVar_ = collect->outputVar();
diff --git a/src/validator/GoValidator.cpp b/src/validator/GoValidator.cpp
index 6f241a89..ae0ffdc6 100644
--- a/src/validator/GoValidator.cpp
+++ b/src/validator/GoValidator.cpp
@@ -51,7 +51,11 @@ Status GoValidator::validateWhere(WhereClause* where) {
}
filter_ = where->filter();
-
+ if (graph::ExpressionUtils::findAny(filter_, {Expression::Kind::kAggregate})) {
+ return Status::SemanticError(
+ "`%s', not support aggregate function in where sentence.",
+ filter_->toString().c_str());
+ }
if (filter_->kind() == Expression::Kind::kLabelAttribute) {
auto laExpr = static_cast<LabelAttributeExpression*>(filter_);
where->setFilter(ExpressionUtils::rewriteLabelAttribute<EdgePropertyExpression>(laExpr));
@@ -107,8 +111,7 @@ Status GoValidator::validateYield(YieldClause* yield) {
} else {
ExpressionUtils::rewriteLabelAttribute<EdgePropertyExpression>(col->expr());
}
-
- if (!col->getAggFunName().empty()) {
+ if (graph::ExpressionUtils::findAny(col->expr(), {Expression::Kind::kAggregate})) {
return Status::SemanticError(
"`%s', not support aggregate function in go sentence.",
col->toString().c_str());
diff --git a/src/validator/GroupByValidator.cpp b/src/validator/GroupByValidator.cpp
index 9885a465..b5928116 100644
--- a/src/validator/GroupByValidator.cpp
+++ b/src/validator/GroupByValidator.cpp
@@ -6,6 +6,11 @@
#include "validator/GroupByValidator.h"
#include "planner/Query.h"
+#include "util/ExpressionUtils.h"
+#include "util/AnonColGenerator.h"
+#include "util/AnonVarGenerator.h"
+#include "visitor/RewriteAggExprVisitor.h"
+#include "visitor/FindAnySubExprVisitor.h"
namespace nebula {
@@ -15,13 +20,8 @@ Status GroupByValidator::validateImpl() {
auto *groupBySentence = static_cast<GroupBySentence*>(sentence_);
NG_RETURN_IF_ERROR(validateGroup(groupBySentence->groupClause()));
NG_RETURN_IF_ERROR(validateYield(groupBySentence->yieldClause()));
+ NG_RETURN_IF_ERROR(groupClauseSemanticCheck());
- if (!exprProps_.srcTagProps().empty() || !exprProps_.dstTagProps().empty()) {
- return Status::SemanticError("Only support input and variable in GroupBy sentence.");
- }
- if (!exprProps_.inputProps().empty() && !exprProps_.varProps().empty()) {
- return Status::SemanticError("Not support both input and variable in GroupBy sentence.");
- }
return Status::OK();
}
@@ -34,80 +34,87 @@ Status GroupByValidator::validateYield(const YieldClause *yieldClause) {
return Status::SemanticError("Yield cols is Empty");
}
+ projCols_ = qctx_->objPool()->add(new YieldColumns);
for (auto* col : columns) {
- auto fun = col->getAggFunName();
- if (!fun.empty()) {
- auto iter = AggFun::nameIdMap_.find(fun);
- if (iter == AggFun::nameIdMap_.end()) {
- return Status::SemanticError("Unkown aggregate function `%s`", fun.c_str());
- }
- if (iter->second != AggFun::Function::kCount && col->expr()->toString() == "*") {
- return Status::SemanticError("`%s` invaild, * valid in count.",
- col->toString().c_str());
- }
+ auto rewrited = false;
+ auto colOldName = deduceColName(col);
+ if (col->expr()->kind() != Expression::Kind::kAggregate) {
+ NG_RETURN_IF_ERROR(rewriteInnerAggExpr(col, rewrited));
+ }
+ auto colExpr = col->expr();
+ // collect exprs for check later
+ if (colExpr->kind() == Expression::Kind::kAggregate) {
+ auto* aggExpr = static_cast<AggregateExpression*>(colExpr);
+ NG_RETURN_IF_ERROR(checkAggExpr(aggExpr));
+ yieldAggs_.emplace_back(colExpr);
+ } else {
+ yieldCols_.emplace_back(colExpr);
}
- // todo(jmq) count(distinct)
- groupItems_.emplace_back(Aggregate::GroupItem{col->expr(), AggFun::nameIdMap_[fun], false});
-
- auto status = deduceExprType(col->expr());
+ groupItems_.emplace_back(colExpr);
+ auto status = deduceExprType(colExpr);
NG_RETURN_IF_ERROR(status);
auto type = std::move(status).value();
- auto name = deduceColName(col);
+ std::string name;
+ if (!rewrited) {
+ name = deduceColName(col);
+ projCols_->addColumn(new YieldColumn(
+ new VariablePropertyExpression(new std::string(),
+ new std::string(name)),
+ new std::string(colOldName)));
+ } else {
+ name = colExpr->toString();
+ }
+ projOutputColumnNames_.emplace_back(colOldName);
outputs_.emplace_back(name, type);
- outputColumnNames_.emplace_back(std::move(name));
- // todo(jmq) extend $-.*
+ outputColumnNames_.emplace_back(name);
- yieldCols_.emplace_back(col);
- if (col->alias() != nullptr) {
+ if (col->alias() != nullptr && !rewrited) {
aliases_.emplace(*col->alias(), col);
}
- // check input yield filed without agg function and not in group cols
ExpressionProps yieldProps;
- NG_RETURN_IF_ERROR(deduceProps(col->expr(), yieldProps));
- if (col->getAggFunName().empty()) {
- if (!yieldProps.inputProps().empty()) {
- if (!exprProps_.isSubsetOfInput(yieldProps.inputProps())) {
- return Status::SemanticError("Yield `%s` isn't in output fields",
- col->toString().c_str());
- }
- } else if (!yieldProps.varProps().empty()) {
- if (!exprProps_.isSubsetOfVar(yieldProps.varProps())) {
- return Status::SemanticError("Yield `%s` isn't in output fields",
- col->toString().c_str());
- }
- }
- }
+ NG_RETURN_IF_ERROR(deduceProps(colExpr, yieldProps));
exprProps_.unionProps(std::move(yieldProps));
}
+
return Status::OK();
}
-
Status GroupByValidator::validateGroup(const GroupClause *groupClause) {
+ if (!groupClause) return Status::OK();
std::vector<YieldColumn*> columns;
if (groupClause != nullptr) {
columns = groupClause->columns();
}
- if (columns.empty()) {
- return Status::SemanticError("Group cols is Empty");
- }
+ auto groupByValid = [](Expression::Kind kind)->bool {
+ return std::unordered_set<Expression::Kind>{
+ Expression::Kind::kAdd ,
+ Expression::Kind::kMinus,
+ Expression::Kind::kMultiply,
+ Expression::Kind::kDivision,
+ Expression::Kind::kMod,
+ Expression::Kind::kTypeCasting,
+ Expression::Kind::kFunctionCall,
+ Expression::Kind::kInputProperty,
+ Expression::Kind::kVarProperty,
+ Expression::Kind::kCase }
+ .count(kind);};
for (auto* col : columns) {
- if (col->expr()->kind() != Expression::Kind::kInputProperty &&
- col->expr()->kind() != Expression::Kind::kVarProperty &&
- col->expr()->kind() != Expression::Kind::kFunctionCall) {
- return Status::SemanticError("Group `%s` invalid", col->expr()->toString().c_str());
+ if (graph::ExpressionUtils::findAny(col->expr(), {Expression::Kind::kAggregate})
+ || !graph::ExpressionUtils::findAny(col->expr(),
+ {Expression::Kind::kInputProperty,
+ Expression::Kind::kVarProperty})) {
+ return Status::SemanticError("Group `%s' invalid", col->expr()->toString().c_str());
}
- if (!col->getAggFunName().empty()) {
- return Status::SemanticError("Use invalid group function `%s`",
- col->getAggFunName().c_str());
+ if (!groupByValid(col->expr()->kind())) {
+ return Status::SemanticError("Group `%s' invalid", col->expr()->toString().c_str());
}
+
NG_RETURN_IF_ERROR(deduceExprType(col->expr()));
NG_RETURN_IF_ERROR(deduceProps(col->expr(), exprProps_));
- groupCols_.emplace_back(col);
groupKeys_.emplace_back(col->expr());
}
return Status::OK();
@@ -116,10 +123,117 @@ Status GroupByValidator::validateGroup(const GroupClause *groupClause) {
Status GroupByValidator::toPlan() {
auto *groupBy = Aggregate::make(qctx_, nullptr, std::move(groupKeys_), std::move(groupItems_));
groupBy->setColNames(std::vector<std::string>(outputColumnNames_));
- root_ = groupBy;
+ if (needGenProject_) {
+ // rewrite Expr which has inner aggExpr and push it up to Project.
+ auto *project = Project::make(qctx_, groupBy, projCols_);
+ project->setInputVar(groupBy->outputVar());
+ project->setColNames(projOutputColumnNames_);
+ root_ = project;
+ } else {
+ root_ = groupBy;
+ }
tail_ = groupBy;
return Status::OK();
}
+Status GroupByValidator::groupClauseSemanticCheck() {
+ // check exprProps
+ if (!exprProps_.srcTagProps().empty() || !exprProps_.dstTagProps().empty()) {
+ return Status::SemanticError("Only support input and variable in GroupBy sentence.");
+ }
+ if (!exprProps_.inputProps().empty() && !exprProps_.varProps().empty()) {
+ return Status::SemanticError("Not support both input and variable in GroupBy sentence.");
+ }
+ if (!exprProps_.varProps().empty() && exprProps_.varProps().size() > 1) {
+ return Status::SemanticError("Only one variable allowed to use.");
+ }
+
+ if (groupKeys_.empty()) {
+ groupKeys_ = yieldCols_;
+ } else {
+ std::unordered_set<Expression*> groupSet(groupKeys_.begin(), groupKeys_.end());
+ FindAnySubExprVisitor groupVisitor(groupSet, true);
+ for (auto* expr : yieldCols_) {
+ if (evaluableExpr(expr)) {
+ continue;
+ }
+ expr->accept(&groupVisitor);
+ if (!groupVisitor.found()) {
+ return Status::SemanticError("Yield non-agg expression `%s' must be"
+ " functionally dependent on items in GROUP BY clause", expr->toString().c_str());
+ }
+ }
+ }
+ return Status::OK();
+}
+
+Status GroupByValidator::rewriteInnerAggExpr(YieldColumn* col, bool& rewrited) {
+ auto colOldName = deduceColName(col);
+ auto* colExpr = col->expr();
+ // agg col need not rewrite
+ DCHECK(colExpr->kind() != Expression::Kind::kAggregate);
+ auto collectAggCol = colExpr->clone();
+ auto aggs = ExpressionUtils::collectAll(collectAggCol.get(),
+ {Expression::Kind::kAggregate});
+ if (aggs.size() > 1) {
+ return Status::SemanticError("Aggregate function nesting is not allowed: `%s'",
+ collectAggCol->toString().c_str());
+ }
+ if (aggs.size() == 1) {
+ auto colRewrited = colExpr->clone();
+ // set aggExpr
+ col->setExpr(aggs[0]->clone().release());
+ auto aggColName = col->expr()->toString();
+ // rewrite to VariablePropertyExpression
+ RewriteAggExprVisitor rewriteAggVisitor(new std::string(),
+ new std::string(aggColName));
+ colRewrited->accept(&rewriteAggVisitor);
+ rewrited = true;
+ needGenProject_ = true;
+ projCols_->addColumn(new YieldColumn(colRewrited.release(),
+ new std::string(colOldName)));
+ }
+
+ return Status::OK();
+}
+
+Status GroupByValidator::checkAggExpr(AggregateExpression* aggExpr) {
+ auto func = aggExpr->name();
+ if (!func) {
+ return Status::SemanticError("`%s' aggregate function not set.",
+ aggExpr->toString().c_str());
+ }
+
+ auto iter = AggregateExpression::NAME_ID_MAP.find(func->c_str());
+ if (iter == AggregateExpression::NAME_ID_MAP.end()) {
+ return Status::SemanticError("Unknown aggregate function `%s'", func->c_str());
+ }
+
+ auto* aggArg = aggExpr->arg();
+ if (graph::ExpressionUtils::findAny(aggArg,
+ {Expression::Kind::kAggregate})) {
+ return Status::SemanticError("Aggregate function nesting is not allowed: `%s'",
+ aggExpr->toString().c_str());
+ }
+
+ if (iter->second != AggregateExpression::Function::kCount) {
+ if (aggArg->toString() == "*") {
+ return Status::SemanticError("Could not apply aggregation function `%s' on `*`",
+ aggExpr->toString().c_str());
+ }
+ if (aggArg->kind() == Expression::Kind::kInputProperty
+ || aggArg->kind() == Expression::Kind::kVarProperty) {
+ auto propExpr = static_cast<PropertyExpression*>(aggArg);
+ if (*propExpr->prop() == "*") {
+ return Status::SemanticError(
+ "Could not apply aggregation function `%s' on `%s'",
+ aggExpr->toString().c_str(), propExpr->toString().c_str());
+ }
+ }
+ }
+
+ return Status::OK();
+}
+
} // namespace graph
} // namespace nebula
diff --git a/src/validator/GroupByValidator.h b/src/validator/GroupByValidator.h
index 56d5cf35..c8ac22c0 100644
--- a/src/validator/GroupByValidator.h
+++ b/src/validator/GroupByValidator.h
@@ -20,26 +20,35 @@ public:
GroupByValidator(Sentence *sentence, QueryContext *context)
: Validator(sentence, context) {}
-private:
Status validateImpl() override;
Status toPlan() override;
+private:
Status validateGroup(const GroupClause *groupClause);
Status validateYield(const YieldClause *yieldClause);
+ Status groupClauseSemanticCheck();
+ Status rewriteInnerAggExpr(YieldColumn* col, bool& rewrited);
+ Status checkAggExpr(AggregateExpression* aggExpr);
+
private:
- std::vector<YieldColumn*> groupCols_;
- std::vector<YieldColumn*> yieldCols_;
+ std::vector<Expression*> yieldCols_;
+ std::vector<Expression*> yieldAggs_;
// key: alias, value: input name
std::unordered_map<std::string, YieldColumn*> aliases_;
+ bool needGenProject_{false};
std::vector<std::string> outputColumnNames_;
+ std::vector<std::string> projOutputColumnNames_;
+
+ // used to generate Project node when there is an internally nested aggregateExpression
+ YieldColumns* projCols_;
std::vector<Expression*> groupKeys_;
- std::vector<Aggregate::GroupItem> groupItems_;
+ std::vector<Expression*> groupItems_;
};
diff --git a/src/validator/Validator.h b/src/validator/Validator.h
index c14a759c..c3ac76f1 100644
--- a/src/validator/Validator.h
+++ b/src/validator/Validator.h
@@ -42,10 +42,6 @@ public:
inputVarName_ = std::move(name);
}
- void setInputCols(ColsDef&& inputs) {
- inputs_ = std::move(inputs);
- }
-
QueryContext* qctx() {
return qctx_;
}
@@ -62,10 +58,50 @@ public:
return outputs_;
}
+ void setOutputCols(const ColsDef&& outputCols) {
+ outputs_ = std::move(outputCols);
+ }
+
+ void setOutputCols(ColsDef& outputCols) {
+ outputs_ = outputCols;
+ }
+
ColsDef inputCols() const {
return inputs_;
}
+ void setInputCols(ColsDef&& inputCols) {
+ inputs_ = std::move(inputCols);
+ }
+
+ void setInputCols(const ColsDef& inputCols) {
+ inputs_ = inputCols;
+ }
+
+ ExpressionProps exprProps() const {
+ return exprProps_;
+ }
+
+ void setExprProps(ExpressionProps&& exprProps) {
+ exprProps_ = std::move(exprProps);
+ }
+
+ void setExprProps(const ExpressionProps& exprProps) {
+ exprProps_ = exprProps;
+ }
+
+ const std::set<std::string>& userDefinedVarNameList() const {
+ return userDefinedVarNameList_;
+ }
+
+ void setUserDefinedVarNameList(std::set<std::string>&& userDefinedVarNameList) {
+ userDefinedVarNameList_ = std::move(userDefinedVarNameList);
+ }
+
+ void setUserDefinedVarNameList(const std::set<std::string>& userDefinedVarNameList) {
+ userDefinedVarNameList_ = userDefinedVarNameList;
+ }
+
void setNoSpaceRequired() {
noSpaceRequired_ = true;
}
@@ -141,19 +177,19 @@ protected:
Sentence* sentence_{nullptr};
QueryContext* qctx_{nullptr};
ValidateContext* vctx_{nullptr};
- // The input columns and output columns of a sentence.
- ColsDef outputs_;
- ColsDef inputs_;
// The variable name of the input node.
std::string inputVarName_;
// Admin sentences do not requires a space to be chosen.
bool noSpaceRequired_{false};
+ // The input columns and output columns of a sentence.
+ ColsDef outputs_;
+ ColsDef inputs_;
+ ExpressionProps exprProps_;
+
// root and tail of a subplan.
PlanNode* root_{nullptr};
PlanNode* tail_{nullptr};
-
- ExpressionProps exprProps_;
// user define Variable name list
std::set<std::string> userDefinedVarNameList_;
// vid's Type
diff --git a/src/validator/YieldValidator.cpp b/src/validator/YieldValidator.cpp
index 8d175de3..4de8f571 100644
--- a/src/validator/YieldValidator.cpp
+++ b/src/validator/YieldValidator.cpp
@@ -28,8 +28,15 @@ Status YieldValidator::validateImpl() {
}
auto yield = static_cast<YieldSentence *>(sentence_);
- NG_RETURN_IF_ERROR(validateYieldAndBuildOutputs(yield->yield()));
+ if (yield->hasAgg()) {
+ NG_RETURN_IF_ERROR(makeImplicitGroupByValidator());
+ }
NG_RETURN_IF_ERROR(validateWhere(yield->where()));
+ if (groupByValidator_) {
+ NG_RETURN_IF_ERROR(validateImplicitGroupBy());
+ } else {
+ NG_RETURN_IF_ERROR(validateYieldAndBuildOutputs(yield->yield()));
+ }
if (!exprProps_.srcTagProps().empty() || !exprProps_.dstTagProps().empty() ||
!exprProps_.edgeProps().empty()) {
@@ -44,39 +51,23 @@ Status YieldValidator::validateImpl() {
return Status::SemanticError("Only one variable allowed to use.");
}
- if (hasAggFun_) {
- NG_RETURN_IF_ERROR(checkAggFunAndBuildGroupItems(yield->yield()));
- }
-
- if (exprProps_.inputProps().empty() && exprProps_.varProps().empty() && inputVarName_.empty()) {
+ if (!groupByValidator_ && exprProps_.inputProps().empty()
+ && exprProps_.varProps().empty() && inputVarName_.empty()) {
// generate constant expression result into querycontext
genConstantExprValues();
}
if (!exprProps_.varProps().empty() && !userDefinedVarNameList_.empty()) {
+ // TODO: Support Multiple userDefinedVars
+ if (userDefinedVarNameList_.size() != 1) {
+ return Status::SemanticError("Multiple user defined vars not supported yet.");
+ }
userDefinedVarName_ = *userDefinedVarNameList_.begin();
}
return Status::OK();
}
-Status YieldValidator::checkAggFunAndBuildGroupItems(const YieldClause *clause) {
- auto yield = clause->yields();
- for (auto column : yield->columns()) {
- auto expr = column->expr();
- auto fun = column->getAggFunName();
- if (!evaluableExpr(expr) && fun.empty()) {
- return Status::SemanticError(
- "Input columns without aggregation are not supported in YIELD statement "
- "without GROUP BY, near `%s'",
- expr->toString().c_str());
- }
-
- groupItems_.emplace_back(Aggregate::GroupItem{expr, AggFun::nameIdMap_[fun], false});
- }
- return Status::OK();
-}
-
Status YieldValidator::makeOutputColumn(YieldColumn *column) {
columns_->addColumn(column);
@@ -117,6 +108,30 @@ void YieldValidator::genConstantExprValues() {
ResultBuilder().value(Value(std::move(ds))).finish());
}
+Status YieldValidator::makeImplicitGroupByValidator() {
+ auto* groupSentence = qctx()->objPool()->add(
+ new GroupBySentence(
+ static_cast<YieldSentence *>(sentence_)->yield()->clone().release(),
+ nullptr, nullptr));
+ groupByValidator_ = std::make_unique<GroupByValidator>(groupSentence, qctx());
+ groupByValidator_->setInputCols(inputs_);
+
+ return Status::OK();
+}
+
+Status YieldValidator::validateImplicitGroupBy() {
+ NG_RETURN_IF_ERROR(groupByValidator_->validateImpl());
+ inputs_ = groupByValidator_->inputCols();
+ outputs_ = groupByValidator_->outputCols();
+ exprProps_.unionProps(groupByValidator_->exprProps());
+
+ const auto& groupVars = groupByValidator_->userDefinedVarNameList();
+ // TODO: Support Multiple userDefinedVars
+ userDefinedVarNameList_.insert(groupVars.begin(), groupVars.end());
+
+ return Status::OK();
+}
+
Status YieldValidator::validateYieldAndBuildOutputs(const YieldClause *clause) {
auto columns = clause->columns();
columns_ = qctx_->objPool()->add(new YieldColumns);
@@ -133,9 +148,6 @@ Status YieldValidator::validateYieldAndBuildOutputs(const YieldClause *clause) {
auto newExpr = new InputPropertyExpression(new std::string(colDef.name));
NG_RETURN_IF_ERROR(makeOutputColumn(new YieldColumn(newExpr)));
}
- if (!column->getAggFunName().empty()) {
- return Status::SemanticError("could not apply aggregation function on `$-.*'");
- }
continue;
}
} else if (expr->kind() == Expression::Kind::kVarProperty) {
@@ -152,23 +164,10 @@ Status YieldValidator::validateYieldAndBuildOutputs(const YieldClause *clause) {
new std::string(colDef.name));
NG_RETURN_IF_ERROR(makeOutputColumn(new YieldColumn(newExpr)));
}
- if (!column->getAggFunName().empty()) {
- return Status::SemanticError("could not apply aggregation function on `$%s.*'",
- var->c_str());
- }
continue;
}
}
- auto fun = column->getAggFunName();
- if (!fun.empty()) {
- auto foundAgg = AggFun::nameIdMap_.find(fun);
- if (foundAgg == AggFun::nameIdMap_.end()) {
- return Status::SemanticError("Unkown aggregate function: `%s'", fun.c_str());
- }
- hasAggFun_ = true;
- }
-
NG_RETURN_IF_ERROR(makeOutputColumn(column->clone().release()));
}
@@ -208,28 +207,43 @@ Status YieldValidator::toPlan() {
}
SingleInputNode *dedupDep = nullptr;
- if (!hasAggFun_) {
- dedupDep = Project::make(qctx_, filter, columns_);
- } else {
- // We do not use group items later, so move it is safe
- dedupDep = Aggregate::make(qctx_, filter, {}, std::move(groupItems_));
- }
- if (filter == nullptr && !inputVar.empty()) {
- dedupDep->setInputVar(inputVar);
- }
- dedupDep->setColNames(std::move(outputColumnNames_));
- if (filter != nullptr) {
- dedupDep->setInputVar(filter->outputVar());
- tail_ = filter;
+ if (groupByValidator_) {
+ groupByValidator_->toPlan();
+ auto* groupByValidatorRoot = groupByValidator_->root();
+ auto* groupByValidatorTail = groupByValidator_->tail();
+ // groupBy validator only gen Project or Aggregate Node
+ DCHECK(groupByValidatorRoot->isSingleInput());
+ DCHECK(groupByValidatorTail->isSingleInput());
+ dedupDep = static_cast<SingleInputNode*>(groupByValidatorRoot);
+ if (filter != nullptr) {
+ if (!inputVar.empty()) {
+ filter->setInputVar(inputVar);
+ }
+ static_cast<SingleInputNode*>(groupByValidatorTail)->dependsOn(filter);
+ static_cast<SingleInputNode*>(groupByValidatorTail)->setInputVar(filter->outputVar());
+ tail_ = filter;
+ } else {
+ tail_ = groupByValidatorTail;
+ if (!inputVar.empty()) {
+ static_cast<SingleInputNode*>(tail_)->setInputVar(inputVar);
+ }
+ }
} else {
- tail_ = dedupDep;
+ dedupDep = Project::make(qctx_, filter, columns_);
+ dedupDep->setColNames(std::move(outputColumnNames_));
+ if (filter != nullptr) {
+ dedupDep->setInputVar(filter->outputVar());
+ tail_ = filter;
+ } else {
+ tail_ = dedupDep;
+ }
+ // Otherwise the input of tail_ would be set by pipe.
+ if (!inputVar.empty()) {
+ static_cast<SingleInputNode *>(tail_)->setInputVar(inputVar);
+ }
}
- // Otherwise the input of tail_ would be set by pipe.
- if (!inputVar.empty()) {
- static_cast<SingleInputNode *>(tail_)->setInputVar(inputVar);
- }
if (yield->yield()->isDistinct()) {
auto dedup = Dedup::make(qctx_, dedupDep);
diff --git a/src/validator/YieldValidator.h b/src/validator/YieldValidator.h
index f5b50a91..93b7c108 100644
--- a/src/validator/YieldValidator.h
+++ b/src/validator/YieldValidator.h
@@ -12,6 +12,7 @@
#include "common/base/Status.h"
#include "planner/Query.h"
#include "validator/Validator.h"
+#include "validator/GroupByValidator.h"
namespace nebula {
@@ -37,18 +38,19 @@ public:
private:
Status validateYieldAndBuildOutputs(const YieldClause *clause);
Status validateWhere(const WhereClause *clause);
- Status checkAggFunAndBuildGroupItems(const YieldClause *clause);
Status makeOutputColumn(YieldColumn *column);
+ Status makeImplicitGroupByValidator();
+ Status validateImplicitGroupBy();
void genConstantExprValues();
private:
- bool hasAggFun_{false};
YieldColumns *columns_{nullptr};
std::vector<std::string> outputColumnNames_;
- std::vector<Aggregate::GroupItem> groupItems_;
std::string constantExprVar_;
std::string userDefinedVarName_;
Expression *filterCondition_{nullptr};
+ // validate for agg
+ std::unique_ptr<GroupByValidator> groupByValidator_{nullptr};
};
} // namespace graph
diff --git a/src/validator/test/CMakeLists.txt b/src/validator/test/CMakeLists.txt
index af6ea318..243bff73 100644
--- a/src/validator/test/CMakeLists.txt
+++ b/src/validator/test/CMakeLists.txt
@@ -41,7 +41,6 @@ set(VALIDATOR_TEST_LIBS
$<TARGET_OBJECTS:common_meta_client_obj>
$<TARGET_OBJECTS:common_file_based_cluster_id_man_obj>
$<TARGET_OBJECTS:common_function_manager_obj>
- $<TARGET_OBJECTS:common_agg_function_obj>
$<TARGET_OBJECTS:common_conf_obj>
$<TARGET_OBJECTS:common_encryption_obj>
$<TARGET_OBJECTS:common_http_client_obj>
diff --git a/src/validator/test/GroupByValidatorTest.cpp b/src/validator/test/GroupByValidatorTest.cpp
index 70023d12..501051d0 100644
--- a/src/validator/test/GroupByValidatorTest.cpp
+++ b/src/validator/test/GroupByValidatorTest.cpp
@@ -21,7 +21,7 @@ TEST_F(GroupByValidatorTest, TestGroupBy) {
{
std::string query =
"GO FROM \"1\" OVER like YIELD like._dst AS id, $^.person.age AS age "
- "| GROUP BY $-.age YIELD COUNT($-.id)";
+ "| GROUP BY $-.age YIELD $-.age, COUNT($-.id)";
std::vector<PlanNode::Kind> expected = {
PK::kAggregate,
PK::kProject,
@@ -42,6 +42,19 @@ TEST_F(GroupByValidatorTest, TestGroupBy) {
};
EXPECT_TRUE(checkResult(query, expected));
}
+ {
+ std::string query =
+ "GO FROM \"NoExist\" OVER like YIELD like._dst AS id, $^.person.age AS age "
+ "| GROUP BY $-.id YIELD $-.id AS id, abs(avg($-.age)) AS age";
+ std::vector<PlanNode::Kind> expected = {
+ PK::kProject,
+ PK::kAggregate,
+ PK::kProject,
+ PK::kGetNeighbors,
+ PK::kStart
+ };
+ EXPECT_TRUE(checkResult(query, expected));
+ }
{
std::string query = "GO FROM \"1\", \"2\" OVER like "
"YIELD $$.person.name as name, "
@@ -71,6 +84,36 @@ TEST_F(GroupByValidatorTest, TestGroupBy) {
};
EXPECT_TRUE(checkResult(query, expected));
}
+ {
+ std::string query = "GO FROM \"1\", \"2\" OVER like "
+ "YIELD $$.person.name as name, "
+ "$$.person.age AS dst_age, "
+ "$$.person.age AS src_age"
+ "| GROUP BY $-.name "
+ "YIELD $-.name AS name, "
+ "SUM($-.dst_age) AS sum_dst_age, "
+ "abs(AVG($-.dst_age)) AS avg_dst_age, "
+ "MAX($-.src_age) AS max_src_age, "
+ "MIN($-.src_age) AS min_src_age, "
+ "STD($-.src_age) AS std_src_age, "
+ "BIT_AND(1) AS bit_and, "
+ "BIT_OR(2) AS bit_or, "
+ "BIT_XOR(3) AS bit_xor";
+ std::vector<PlanNode::Kind> expected = {
+ PK::kProject,
+ PK::kAggregate,
+ PK::kProject,
+ PK::kDataJoin,
+ PK::kProject,
+ PK::kGetVertices,
+ PK::kDedup,
+ PK::kProject,
+ PK::kProject,
+ PK::kGetNeighbors,
+ PK::kStart
+ };
+ EXPECT_TRUE(checkResult(query, expected));
+ }
{
// group one col
std::string query = "GO FROM \"1\" OVER like "
@@ -103,7 +146,7 @@ TEST_F(GroupByValidatorTest, TestGroupBy) {
"like._dst AS id, "
"like.start AS start_year, "
"like.end AS end_year"
- "| GROUP BY $-.name, abs(5) "
+ "| GROUP BY $-.name, abs($-.end_year) "
"YIELD $-.name AS name, "
"SUM(1.5) AS sum, "
"COUNT(*) AS count, "
@@ -154,7 +197,7 @@ TEST_F(GroupByValidatorTest, VariableTest) {
{
std::string query =
"$a = GO FROM \"1\" OVER like YIELD like._dst AS id, $^.person.age AS age; "
- "GROUP BY $a.age YIELD COUNT($a.id)";
+ "GROUP BY $a.age YIELD $a.age, COUNT($a.id)";
std::vector<PlanNode::Kind> expected = {
PK::kAggregate, PK::kProject, PK::kGetNeighbors, PK::kStart};
EXPECT_TRUE(checkResult(query, expected));
@@ -192,7 +235,7 @@ TEST_F(GroupByValidatorTest, VariableTest) {
"like._dst AS id, "
"like.start AS start_year, "
"like.end AS end_year;"
- "GROUP BY $a.name, abs(5) "
+ "GROUP BY $a.name, abs($a.end_year) "
"YIELD $a.name AS name, "
"SUM(1.5) AS sum, "
"COUNT(*) AS count, "
@@ -220,7 +263,21 @@ TEST_F(GroupByValidatorTest, InvalidTest) {
std::string query = "GO FROM \"1\" OVER like YIELD like._dst AS id, $^.person.age AS age "
"| GROUP BY 1+1 YIELD COUNT(1), 1+1";
auto result = checkResult(query);
- EXPECT_EQ(std::string(result.message()), "SemanticError: Group `(1+1)` invalid");
+ EXPECT_EQ(std::string(result.message()), "SemanticError: Group `(1+1)' invalid");
+ }
+ {
+ // use groupby without input
+ std::string query = "GO FROM \"1\" OVER like YIELD like._dst AS id, $^.person.age AS age "
+ "| GROUP BY count(*)+1 YIELD COUNT(1), 1+1";
+ auto result = checkResult(query);
+ EXPECT_EQ(std::string(result.message()), "SemanticError: Group `(COUNT(*)+1)' invalid");
+ }
+ {
+ // use groupby without input
+ std::string query = "GO FROM \"1\" OVER like YIELD like._dst AS id, $^.person.age AS age "
+ "| GROUP BY abs(1)+1 YIELD COUNT(1), 1+1";
+ auto result = checkResult(query);
+ EXPECT_EQ(std::string(result.message()), "SemanticError: Group `(abs(1)+1)' invalid");
}
{
// use dst
@@ -243,7 +300,8 @@ TEST_F(GroupByValidatorTest, InvalidTest) {
std::string query = "GO FROM \"1\" OVER like YIELD like._dst AS id, $^.person.age AS age "
"| GROUP BY noexist YIELD COUNT($-.age)";
auto result = checkResult(query);
- EXPECT_EQ(std::string(result.message()), "SemanticError: Group `noexist` invalid");
+ EXPECT_EQ(std::string(result.message()),
+ "SemanticError: Group `noexist' invalid");
}
{
// use sum(*)
@@ -251,7 +309,7 @@ TEST_F(GroupByValidatorTest, InvalidTest) {
"| GROUP BY $-.id YIELD SUM(*)";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
- "SemanticError: `SUM(*)` invaild, * valid in count.");
+ "SemanticError: Could not apply aggregation function `SUM(*)' on `*`");
}
{
// use agg fun has more than two inputs
@@ -265,7 +323,7 @@ TEST_F(GroupByValidatorTest, InvalidTest) {
std::string query = "GO FROM \"1\" OVER like YIELD like._dst AS id, $^.person.age AS age "
"| GROUP BY $-.id, SUM($-.age) YIELD $-.id, SUM($-.age)";
auto result = checkResult(query);
- EXPECT_EQ(std::string(result.message()), "SemanticError: Use invalid group function `SUM`");
+ EXPECT_EQ(std::string(result.message()), "SemanticError: Group `SUM($-.age)' invalid");
}
{
// yield without group by
@@ -273,24 +331,44 @@ TEST_F(GroupByValidatorTest, InvalidTest) {
"COUNT(like._dst) AS id ";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
- "SemanticError: `COUNT(like._dst) AS id', not support "
- "aggregate function in go sentence.");
+ "SemanticError: `COUNT(like._dst) AS id', "
+ "not support aggregate function in go sentence.");
+ }
+ {
+ // yield without group by
+ std::string query = "GO FROM \"1\" OVER like YIELD $^.person.age AS age, "
+ "COUNT(like._dst)+1 AS id ";
+ auto result = checkResult(query);
+ EXPECT_EQ(std::string(result.message()),
+ "SemanticError: `(COUNT(like._dst)+1) AS id', "
+ "not support aggregate function in go sentence.");
+ }
+ {
+ // yield without group by
+ std::string query = "GO FROM \"1\" OVER like WHERE count(*) + 1 >3 "
+ "YIELD $^.person.age AS age, "
+ "COUNT(like._dst)+1 AS id ";
+ auto result = checkResult(query);
+ EXPECT_EQ(std::string(result.message()),
+ "SemanticError: `((COUNT(*)+1)>3)', "
+ "not support aggregate function in where sentence.");
}
- {
+ {
// yield col not in group output
std::string query = "GO FROM \"1\" OVER like "
"YIELD $$.person.name as name, "
"like._dst AS id, "
"like.start AS start_year, "
"like.end AS end_year"
- "| GROUP BY $-.start_year, abs(5) "
+ "| GROUP BY $-.start_year, abs($-.end_year) "
"YIELD $-.name AS name, "
"SUM(1.5) AS sum, "
"COUNT(*) AS count, "
"1+1 AS cal";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
- "SemanticError: Yield `$-.name AS name` isn't in output fields");
+ "SemanticError: Yield non-agg expression `$-.name' "
+ "must be functionally dependent on items in GROUP BY clause");
}
{
// duplicate col
diff --git a/src/validator/test/QueryValidatorTest.cpp b/src/validator/test/QueryValidatorTest.cpp
index 084add46..260c226f 100644
--- a/src/validator/test/QueryValidatorTest.cpp
+++ b/src/validator/test/QueryValidatorTest.cpp
@@ -1086,8 +1086,11 @@ TEST_F(QueryValidatorTest, GoInvalid) {
EXPECT_FALSE(checkResult(query));
}
{
+ // yield agg without groupBy is not supported
std::string query = "GO FROM \"2\" OVER like YIELD COUNT(123);";
- EXPECT_FALSE(checkResult(query));
+ auto result = checkResult(query);
+ EXPECT_EQ(std::string(result.message()), "SemanticError: `COUNT(123)', "
+ "not support aggregate function in go sentence.");
}
{
std::string query = "GO FROM \"1\" OVER like YIELD like._dst AS id, like._src AS id | GO "
diff --git a/src/validator/test/YieldValidatorTest.cpp b/src/validator/test/YieldValidatorTest.cpp
index 88087a25..6458c349 100644
--- a/src/validator/test/YieldValidatorTest.cpp
+++ b/src/validator/test/YieldValidatorTest.cpp
@@ -113,19 +113,19 @@ TEST_F(YieldValidatorTest, FuncitonCall) {
std::string query = "YIELD abs(true)";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
- "SemanticError: `abs(true)` is not a valid expression : Parameter's type error");
+ "SemanticError: `abs(true)' is not a valid expression : Parameter's type error");
}
{
std::string query = "YIELD abs(\"test\")";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
- "SemanticError: `abs(test)` is not a valid expression : Parameter's type error");
+ "SemanticError: `abs(test)' is not a valid expression : Parameter's type error");
}
{
std::string query = "YIELD noexist(12)";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
- "SemanticError: `noexist(12)` is not a valid expression : Function `noexist' not "
+ "SemanticError: `noexist(12)' is not a valid expression : Function `noexist' not "
"defined");
}
}
@@ -164,13 +164,13 @@ TEST_F(YieldValidatorTest, TypeCastTest) {
std::string query = "YIELD (int)\"123abc\"";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
- "SemanticError: `(INT)123abc` is not a valid expression ");
+ "SemanticError: `(INT)123abc' is not a valid expression ");
}
{
std::string query = "YIELD (int)\"abc123\"";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
- "SemanticError: `(INT)abc123` is not a valid expression ");
+ "SemanticError: `(INT)abc123' is not a valid expression ");
}
{
std::string query = "YIELD (doublE)\"123\"";
@@ -184,7 +184,7 @@ TEST_F(YieldValidatorTest, TypeCastTest) {
std::string query = "YIELD (doublE)\".a123\"";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
- "SemanticError: `(FLOAT).a123` is not a valid expression ");
+ "SemanticError: `(FLOAT).a123' is not a valid expression ");
}
{
std::string query = "YIELD (STRING)1.23";
@@ -485,12 +485,8 @@ TEST_F(YieldValidatorTest, AggCall) {
"YIELD $^.person.age AS age, "
"like.likeness AS like"
"| YIELD COUNT(*), $-.age";
- auto result = checkResult(query);
- EXPECT_EQ(std::string(result.message()),
- "SemanticError: Input columns without aggregation are not supported in YIELD "
- "statement without GROUP BY, near `$-.age'");
+ EXPECT_TRUE(checkResult(query));
}
- // Test input
{
auto query = "GO FROM \"1\" OVER like "
"YIELD $^.person.age AS age, "
@@ -537,7 +533,7 @@ TEST_F(YieldValidatorTest, AggCall) {
"YIELD $^.person.age AS age, "
"like.likeness AS like"
"| YIELD AVG($-.age), SUM($-.like), COUNT(*), $-.age + 1";
- EXPECT_FALSE(checkResult(query));
+ EXPECT_TRUE(checkResult(query));
}
{
auto query = "GO FROM \"1\" OVER like "
@@ -546,7 +542,7 @@ TEST_F(YieldValidatorTest, AggCall) {
"| YIELD AVG($-.*)";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
- "SemanticError: could not apply aggregation function on `$-.*'");
+ "SemanticError: Could not apply aggregation function `AVG($-.*)' on `$-.*'");
}
// Yield field has not input
{
@@ -589,7 +585,7 @@ TEST_F(YieldValidatorTest, AggCall) {
"YIELD $^.person.age AS age, "
"like.likeness AS like;"
"YIELD AVG($var.age), SUM($var.like), COUNT(*), $var.age + 1";
- EXPECT_FALSE(checkResult(query));
+ EXPECT_TRUE(checkResult(query));
}
{
auto query = "$var = GO FROM \"1\" OVER like "
@@ -626,7 +622,7 @@ TEST_F(YieldValidatorTest, AggCall) {
"YIELD AVG($var.*)";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
- "SemanticError: could not apply aggregation function on `$var.*'");
+ "SemanticError: Could not apply aggregation function `AVG($var.*)' on `$var.*'");
}
}
diff --git a/src/visitor/CMakeLists.txt b/src/visitor/CMakeLists.txt
index a7e61a16..01a11588 100644
--- a/src/visitor/CMakeLists.txt
+++ b/src/visitor/CMakeLists.txt
@@ -12,8 +12,10 @@ nebula_add_library(
ExtractPropExprVisitor.cpp
ExtractFilterExprVisitor.cpp
FindAnyExprVisitor.cpp
+ FindAnySubExprVisitor.cpp
FoldConstantExprVisitor.cpp
RewriteLabelAttrVisitor.cpp
+ RewriteAggExprVisitor.cpp
RewriteInputPropVisitor.cpp
RewriteSymExprVisitor.cpp
RewriteMatchLabelVisitor.cpp
diff --git a/src/visitor/CollectAllExprsVisitor.cpp b/src/visitor/CollectAllExprsVisitor.cpp
index 7f6366d1..e1599c1e 100644
--- a/src/visitor/CollectAllExprsVisitor.cpp
+++ b/src/visitor/CollectAllExprsVisitor.cpp
@@ -30,6 +30,11 @@ void CollectAllExprsVisitor::visit(FunctionCallExpression *expr) {
}
}
+void CollectAllExprsVisitor::visit(AggregateExpression *expr) {
+ collectExpr(expr);
+ expr->arg()->accept(this);
+}
+
void CollectAllExprsVisitor::visit(ListExpression *expr) {
collectExpr(expr);
for (auto &item : expr->items()) {
diff --git a/src/visitor/CollectAllExprsVisitor.h b/src/visitor/CollectAllExprsVisitor.h
index ba6aa8da..fa6b16c2 100644
--- a/src/visitor/CollectAllExprsVisitor.h
+++ b/src/visitor/CollectAllExprsVisitor.h
@@ -32,6 +32,7 @@ private:
void visit(TypeCastingExpression* expr) override;
void visit(UnaryExpression* expr) override;
void visit(FunctionCallExpression* expr) override;
+ void visit(AggregateExpression* expr) override;
void visit(ListExpression* expr) override;
void visit(SetExpression* expr) override;
void visit(MapExpression* expr) override;
diff --git a/src/visitor/DeduceTypeVisitor.cpp b/src/visitor/DeduceTypeVisitor.cpp
index 49f2258c..1bc42b21 100644
--- a/src/visitor/DeduceTypeVisitor.cpp
+++ b/src/visitor/DeduceTypeVisitor.cpp
@@ -182,7 +182,7 @@ void DeduceTypeVisitor::visit(TypeCastingExpression *expr) {
auto val = expr->eval(ctx(nullptr));
if (val.isNull()) {
status_ =
- Status::SemanticError("`%s` is not a valid expression ", expr->toString().c_str());
+ Status::SemanticError("`%s' is not a valid expression ", expr->toString().c_str());
return;
}
type_ = val.type();
@@ -386,7 +386,7 @@ void DeduceTypeVisitor::visit(FunctionCallExpression *expr) {
}
auto result = FunctionManager::getReturnType(funName, argsTypeList);
if (!result.ok()) {
- status_ = Status::SemanticError("`%s` is not a valid expression : %s",
+ status_ = Status::SemanticError("`%s' is not a valid expression : %s",
expr->toString().c_str(),
result.status().toString().c_str());
return;
@@ -394,6 +394,65 @@ void DeduceTypeVisitor::visit(FunctionCallExpression *expr) {
type_ = result.value();
}
+void DeduceTypeVisitor::visit(AggregateExpression *expr) {
+ expr->arg()->accept(this);
+ if (!ok()) return;
+ auto arg_type = type_;
+
+ auto func = AggregateExpression::NAME_ID_MAP[expr->name()->c_str()];
+ switch (func) {
+ case AggregateExpression::Function::kCount: {
+ type_ = Value::Type::INT;
+ break;
+ }
+ case AggregateExpression::Function::kSum: {
+ type_ = arg_type;
+ break;
+ }
+ case AggregateExpression::Function::kAvg: {
+ type_ = Value::Type::FLOAT;
+ break;
+ }
+ case AggregateExpression::Function::kMax: {
+ type_ = arg_type;
+ break;
+ }
+ case AggregateExpression::Function::kMin: {
+ type_ = arg_type;
+ break;
+ }
+ case AggregateExpression::Function::kStdev: {
+ type_ = Value::Type::FLOAT;
+ break;
+ }
+ case AggregateExpression::Function::kBitAnd: {
+ type_ = Value::Type::INT;
+ break;
+ }
+ case AggregateExpression::Function::kBitOr: {
+ type_ = Value::Type::INT;
+ break;
+ }
+ case AggregateExpression::Function::kBitXor: {
+ type_ = Value::Type::INT;
+ break;
+ }
+ case AggregateExpression::Function::kCollect: {
+ type_ = Value::Type::LIST;
+ break;
+ }
+ case AggregateExpression::Function::kCollectSet: {
+ type_ = Value::Type::SET;
+ break;
+ }
+ default: {
+ LOG(FATAL) << "Invalid Aggregate expression kind: "
+ << expr->name()->c_str();
+ break;
+ }
+ }
+}
+
void DeduceTypeVisitor::visit(UUIDExpression *) {
type_ = Value::Type::STRING;
}
diff --git a/src/visitor/DeduceTypeVisitor.h b/src/visitor/DeduceTypeVisitor.h
index ed35c9cb..fe3e0a9a 100644
--- a/src/visitor/DeduceTypeVisitor.h
+++ b/src/visitor/DeduceTypeVisitor.h
@@ -55,6 +55,7 @@ private:
void visit(LogicalExpression *expr) override;
// function call
void visit(FunctionCallExpression *expr) override;
+ void visit(AggregateExpression *expr) override;
void visit(UUIDExpression *expr) override;
// variable expression
void visit(VariableExpression *expr) override;
diff --git a/src/visitor/ExprVisitorImpl.cpp b/src/visitor/ExprVisitorImpl.cpp
index 2a2cfd0a..9f549e99 100644
--- a/src/visitor/ExprVisitorImpl.cpp
+++ b/src/visitor/ExprVisitorImpl.cpp
@@ -64,6 +64,11 @@ void ExprVisitorImpl::visit(FunctionCallExpression *expr) {
}
}
+void ExprVisitorImpl::visit(AggregateExpression *expr) {
+ DCHECK(ok());
+ expr->arg()->accept(this);
+}
+
// container expression
void ExprVisitorImpl::visit(ListExpression *expr) {
DCHECK(ok());
diff --git a/src/visitor/ExprVisitorImpl.h b/src/visitor/ExprVisitorImpl.h
index 4ebf4db0..2f24b5d7 100644
--- a/src/visitor/ExprVisitorImpl.h
+++ b/src/visitor/ExprVisitorImpl.h
@@ -25,6 +25,7 @@ public:
void visit(LogicalExpression *expr) override;
// function call
void visit(FunctionCallExpression *expr) override;
+ void visit(AggregateExpression *expr) override;
// container expression
void visit(ListExpression *expr) override;
void visit(SetExpression *expr) override;
diff --git a/src/visitor/FindAnyExprVisitor.cpp b/src/visitor/FindAnyExprVisitor.cpp
index c3abfb42..6b85c5d6 100644
--- a/src/visitor/FindAnyExprVisitor.cpp
+++ b/src/visitor/FindAnyExprVisitor.cpp
@@ -35,6 +35,12 @@ void FindAnyExprVisitor::visit(FunctionCallExpression *expr) {
}
}
+void FindAnyExprVisitor::visit(AggregateExpression *expr) {
+ findExpr(expr);
+ if (found_) return;
+ expr->arg()->accept(this);
+}
+
void FindAnyExprVisitor::visit(ListExpression *expr) {
findExpr(expr);
if (found_) return;
diff --git a/src/visitor/FindAnyExprVisitor.h b/src/visitor/FindAnyExprVisitor.h
index 8f91bc22..fb53103b 100644
--- a/src/visitor/FindAnyExprVisitor.h
+++ b/src/visitor/FindAnyExprVisitor.h
@@ -34,6 +34,7 @@ private:
void visit(TypeCastingExpression* expr) override;
void visit(UnaryExpression* expr) override;
void visit(FunctionCallExpression* expr) override;
+ void visit(AggregateExpression* expr) override;
void visit(ListExpression* expr) override;
void visit(SetExpression* expr) override;
void visit(MapExpression* expr) override;
diff --git a/src/visitor/FindAnySubExprVisitor.cpp b/src/visitor/FindAnySubExprVisitor.cpp
new file mode 100644
index 00000000..79dbf8df
--- /dev/null
+++ b/src/visitor/FindAnySubExprVisitor.cpp
@@ -0,0 +1,206 @@
+/* Copyright (c) 2020 vesoft inc. All rights reserved.
+ *
+ * This source code is licensed under Apache 2.0 License,
+ * attached with Common Clause Condition 1.0, found in the LICENSES directory.
+ */
+
+#include "visitor/FindAnySubExprVisitor.h"
+
+namespace nebula {
+namespace graph {
+
+FindAnySubExprVisitor::FindAnySubExprVisitor(std::unordered_set<Expression*> &subExprs,
+ bool needRecursiveSearch)
+ : subExprs_(subExprs), needRecursiveSearch_(needRecursiveSearch) {
+ DCHECK(!subExprs_.empty());
+}
+
+void FindAnySubExprVisitor::visit(TypeCastingExpression *expr) {
+ compareWithSubExprs<TypeCastingExpression>(expr);
+ if (needRecursiveSearch_) {
+ expr->operand()->accept(this);
+ }
+}
+
+void FindAnySubExprVisitor::visit(UnaryExpression *expr) {
+ compareWithSubExprs<UnaryExpression>(expr);
+ if (needRecursiveSearch_) {
+ expr->operand()->accept(this);
+ }
+}
+
+void FindAnySubExprVisitor::visit(FunctionCallExpression *expr) {
+ compareWithSubExprs<FunctionCallExpression>(expr);
+ if (needRecursiveSearch_) {
+ for (const auto &arg : expr->args()->args()) {
+ arg->accept(this);
+ if (found_) return;
+ }
+ }
+}
+
+void FindAnySubExprVisitor::visit(AggregateExpression *expr) {
+ compareWithSubExprs<AggregateExpression>(expr);
+ if (needRecursiveSearch_) {
+ expr->arg()->accept(this);
+ }
+}
+
+void FindAnySubExprVisitor::visit(ListExpression *expr) {
+ compareWithSubExprs<ListExpression>(expr);
+ if (needRecursiveSearch_) {
+ for (const auto &item : expr->items()) {
+ item->accept(this);
+ if (found_) return;
+ }
+ }
+}
+
+void FindAnySubExprVisitor::visit(SetExpression *expr) {
+ compareWithSubExprs<SetExpression>(expr);
+ if (needRecursiveSearch_) {
+ for (const auto &item : expr->items()) {
+ item->accept(this);
+ if (found_) return;
+ }
+ }
+}
+
+void FindAnySubExprVisitor::visit(MapExpression *expr) {
+ compareWithSubExprs<MapExpression>(expr);
+ if (needRecursiveSearch_) {
+ for (const auto &pair : expr->items()) {
+ pair.second->accept(this);
+ if (found_) return;
+ }
+ }
+}
+
+void FindAnySubExprVisitor::visit(CaseExpression *expr) {
+ compareWithSubExprs<CaseExpression>(expr);
+ if (needRecursiveSearch_) {
+ if (expr->hasCondition()) {
+ expr->condition()->accept(this);
+ if (found_) return;
+ }
+ if (expr->hasDefault()) {
+ expr->defaultResult()->accept(this);
+ if (found_) return;
+ }
+ for (const auto &whenThen : expr->cases()) {
+ whenThen.when->accept(this);
+ if (found_) return;
+ whenThen.then->accept(this);
+ if (found_) return;
+ }
+ }
+}
+
+void FindAnySubExprVisitor::visit(ConstantExpression *expr) {
+ compareWithSubExprs<ConstantExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(EdgePropertyExpression *expr) {
+ compareWithSubExprs<EdgePropertyExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(TagPropertyExpression *expr) {
+ compareWithSubExprs<TagPropertyExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(InputPropertyExpression *expr) {
+ compareWithSubExprs<InputPropertyExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(VariablePropertyExpression *expr) {
+ compareWithSubExprs<VariablePropertyExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(SourcePropertyExpression *expr) {
+ compareWithSubExprs<SourcePropertyExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(DestPropertyExpression *expr) {
+ compareWithSubExprs<DestPropertyExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(EdgeSrcIdExpression *expr) {
+ compareWithSubExprs<EdgeSrcIdExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(EdgeTypeExpression *expr) {
+ compareWithSubExprs<EdgeTypeExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(EdgeRankExpression *expr) {
+ compareWithSubExprs<EdgeRankExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(EdgeDstIdExpression *expr) {
+ compareWithSubExprs<EdgeDstIdExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(UUIDExpression *expr) {
+ compareWithSubExprs<UUIDExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(VariableExpression *expr) {
+ compareWithSubExprs<VariableExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(VersionedVariableExpression *expr) {
+ compareWithSubExprs<VersionedVariableExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(LabelExpression *expr) {
+ compareWithSubExprs<LabelExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(VertexExpression *expr) {
+ compareWithSubExprs<VertexExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(EdgeExpression *expr) {
+ compareWithSubExprs<EdgeExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visit(ColumnExpression *expr) {
+ compareWithSubExprs<ColumnExpression>(expr);
+}
+
+void FindAnySubExprVisitor::visitBinaryExpr(BinaryExpression *expr) {
+ compareWithSubExprs<BinaryExpression>(expr);
+ if (needRecursiveSearch_) {
+ expr->left()->accept(this);
+ if (found_) return;
+ expr->right()->accept(this);
+ }
+}
+
+void FindAnySubExprVisitor::checkExprKind(const Expression *expr, const Expression *sub_expr) {
+ if (expr == sub_expr) {
+ found_ = true;
+ continue_ = false;
+ } else if (expr == nullptr || sub_expr == nullptr) {
+ continue_ = false;
+ } else if (expr->kind() != sub_expr->kind()) {
+ continue_ = false;
+ }
+}
+
+template <typename T>
+void FindAnySubExprVisitor::compareWithSubExprs(T* expr) {
+ for (Expression* sub : subExprs_) {
+ continue_ = true;
+ checkExprKind(expr, sub);
+ if (found_) return;
+ if (!continue_) continue;
+ if (*static_cast<T*>(sub) == *expr) {
+ found_ = true;
+ return;
+ }
+ }
+}
+
+} // namespace graph
+} // namespace nebula
diff --git a/src/visitor/FindAnySubExprVisitor.h b/src/visitor/FindAnySubExprVisitor.h
new file mode 100644
index 00000000..c9e33489
--- /dev/null
+++ b/src/visitor/FindAnySubExprVisitor.h
@@ -0,0 +1,83 @@
+/* Copyright (c) 2020 vesoft inc. All rights reserved.
+ *
+ * This source code is licensed under Apache 2.0 License,
+ * attached with Common Clause Condition 1.0, found in the LICENSES directory.
+ */
+
+#ifndef VISITOR_FINDANYSUBEXPRVISITOR_H_
+#define VISITOR_FINDANYSUBEXPRVISITOR_H_
+
+#include <unordered_set>
+
+#include "common/expression/Expression.h"
+#include "visitor/ExprVisitorImpl.h"
+
+namespace nebula {
+namespace graph {
+
+class FindAnySubExprVisitor final : public ExprVisitorImpl {
+public:
+ explicit FindAnySubExprVisitor(std::unordered_set<Expression*> &subExprs,
+ bool needRecursiveSearch);
+
+ bool ok() const override {
+ return !found_;
+ }
+
+ bool found() const {
+ return found_;
+ }
+
+ const Expression* expr() const {
+ return expr_;
+ }
+
+private:
+ using ExprVisitorImpl::visit;
+
+ void visit(TypeCastingExpression* expr) override;
+ void visit(UnaryExpression* expr) override;
+ void visit(FunctionCallExpression* expr) override;
+ void visit(AggregateExpression* expr) override;
+ void visit(ListExpression* expr) override;
+ void visit(SetExpression* expr) override;
+ void visit(MapExpression* expr) override;
+ void visit(CaseExpression* expr) override;
+
+ void visit(ConstantExpression* expr) override;
+ void visit(EdgePropertyExpression* expr) override;
+ void visit(TagPropertyExpression* expr) override;
+ void visit(InputPropertyExpression* expr) override;
+ void visit(VariablePropertyExpression* expr) override;
+ void visit(SourcePropertyExpression* expr) override;
+ void visit(DestPropertyExpression* expr) override;
+ void visit(EdgeSrcIdExpression* expr) override;
+ void visit(EdgeTypeExpression* expr) override;
+ void visit(EdgeRankExpression* expr) override;
+ void visit(EdgeDstIdExpression* expr) override;
+ void visit(UUIDExpression* expr) override;
+ void visit(VariableExpression* expr) override;
+ void visit(VersionedVariableExpression* expr) override;
+ void visit(LabelExpression* expr) override;
+ void visit(VertexExpression* expr) override;
+ void visit(EdgeExpression* expr) override;
+ void visit(ColumnExpression* expr) override;
+ void visitBinaryExpr(BinaryExpression* expr) override;
+
+ void checkExprKind(const Expression*, const Expression*);
+
+ template <typename T>
+ void compareWithSubExprs(T* expr);
+
+ bool found_{false};
+ // need continue search
+ bool continue_{true};
+ const Expression* expr_{nullptr};
+ const std::unordered_set<Expression*> subExprs_;
+ bool needRecursiveSearch_{false};
+};
+
+} // namespace graph
+} // namespace nebula
+
+#endif // VISITOR_FINDANYSUBEXPRVISITOR_H_
diff --git a/src/visitor/FoldConstantExprVisitor.cpp b/src/visitor/FoldConstantExprVisitor.cpp
index 82184be9..a8abe6ac 100644
--- a/src/visitor/FoldConstantExprVisitor.cpp
+++ b/src/visitor/FoldConstantExprVisitor.cpp
@@ -115,6 +115,16 @@ void FoldConstantExprVisitor::visit(FunctionCallExpression *expr) {
canBeFolded_ = canBeFolded;
}
+void FoldConstantExprVisitor::visit(AggregateExpression *expr) {
+ // TODO : impl AggExpr foldConstantExprVisitor
+ if (!isConstant(expr->arg())) {
+ expr->arg()->accept(this);
+ if (canBeFolded_) {
+ expr->setArg(fold(expr->arg()));
+ }
+ }
+}
+
void FoldConstantExprVisitor::visit(UUIDExpression *expr) {
UNUSED(expr);
canBeFolded_ = false;
diff --git a/src/visitor/FoldConstantExprVisitor.h b/src/visitor/FoldConstantExprVisitor.h
index a111122b..18995604 100644
--- a/src/visitor/FoldConstantExprVisitor.h
+++ b/src/visitor/FoldConstantExprVisitor.h
@@ -35,6 +35,7 @@ public:
void visit(LogicalExpression *expr) override;
// function call
void visit(FunctionCallExpression *expr) override;
+ void visit(AggregateExpression *expr) override;
void visit(UUIDExpression *expr) override;
// variable expression
void visit(VariableExpression *expr) override;
diff --git a/src/visitor/RewriteAggExprVisitor.cpp b/src/visitor/RewriteAggExprVisitor.cpp
new file mode 100644
index 00000000..223c42c3
--- /dev/null
+++ b/src/visitor/RewriteAggExprVisitor.cpp
@@ -0,0 +1,71 @@
+/* Copyright (c) 2020 vesoft inc. All rights reserved.
+ *
+ * This source code is licensed under Apache 2.0 License,
+ * attached with Common Clause Condition 1.0, found in the LICENSES directory.
+ */
+
+#include "visitor/RewriteAggExprVisitor.h"
+#include "util/ExpressionUtils.h"
+
+#include "common/base/Logging.h"
+
+namespace nebula {
+namespace graph {
+
+RewriteAggExprVisitor::RewriteAggExprVisitor(std::string* var,
+ std::string* prop) {
+ var_.reset(var);
+ prop_.reset(prop);
+}
+
+void RewriteAggExprVisitor::visit(TypeCastingExpression* expr) {
+ if (isAggExpr(expr->operand())) {
+ expr->setOperand(ExpressionUtils::newVarPropExpr(prop_->c_str(), var_->c_str()));
+ } else {
+ expr->operand()->accept(this);
+ }
+}
+
+// rewrite AggregateExpression to VariablePropertyExpression
+void RewriteAggExprVisitor::visit(FunctionCallExpression* expr) {
+ for (auto& arg : expr->args()->args()) {
+ if (isAggExpr(arg.get())) {
+ arg.reset(new VariablePropertyExpression(var_.release(),
+ prop_.release()));
+ } else {
+ arg->accept(this);
+ }
+ }
+}
+
+void RewriteAggExprVisitor::visit(ArithmeticExpression *expr) {
+ visitBinaryExpr(static_cast<BinaryExpression*>(expr));
+}
+
+void RewriteAggExprVisitor::visitBinaryExpr(BinaryExpression *expr) {
+ DCHECK(ok());
+ auto* lhs = expr->left();
+ if (isAggExpr(lhs)) {
+ expr->setLeft(new VariablePropertyExpression(std::move(var_).release(),
+ std::move(prop_).release()));
+ // only support rewrite single agg expr
+ return;
+ } else {
+ lhs->accept(this);
+ }
+ auto* rhs = expr->right();
+ if (isAggExpr(rhs)) {
+ expr->setRight(new VariablePropertyExpression(std::move(var_).release(),
+ std::move(prop_).release()));
+ return;
+ } else {
+ rhs->accept(this);
+ }
+}
+bool RewriteAggExprVisitor::isAggExpr(const Expression* expr) {
+ return expr->kind() == Expression::Kind::kAggregate;
+}
+
+
+} // namespace graph
+} // namespace nebula
diff --git a/src/visitor/RewriteAggExprVisitor.h b/src/visitor/RewriteAggExprVisitor.h
new file mode 100644
index 00000000..598366d5
--- /dev/null
+++ b/src/visitor/RewriteAggExprVisitor.h
@@ -0,0 +1,70 @@
+/* Copyright (c) 2020 vesoft inc. All rights reserved.
+ *
+ * This source code is licensed under Apache 2.0 License,
+ * attached with Common Clause Condition 1.0, found in the LICENSES directory.
+ */
+
+#ifndef VISITOR_REWRITEAGGEXPRVISITOR_H_
+#define VISITOR_REWRITEAGGEXPRVISITOR_H_
+
+#include <memory>
+#include <vector>
+
+#include "visitor/ExprVisitorImpl.h"
+
+namespace nebula {
+namespace graph {
+
+class RewriteAggExprVisitor final : public ExprVisitorImpl {
+public:
+ explicit RewriteAggExprVisitor(std::string* var,
+ std::string* prop);
+
+ bool ok() const override {
+ return true;
+ }
+
+private:
+ using ExprVisitorImpl::visit;
+
+ void visit(TypeCastingExpression *expr) override;
+ void visit(FunctionCallExpression *expr) override;
+ void visit(ArithmeticExpression *expr) override;
+
+ void visit(UnaryExpression *) override {}
+ void visit(ListExpression *) override {}
+ void visit(SetExpression *) override {}
+ void visit(MapExpression *) override {}
+ void visit(CaseExpression *) override {}
+ void visit(ConstantExpression *) override {}
+ void visit(LabelExpression *) override {}
+ void visit(UUIDExpression *) override {}
+ void visit(LabelAttributeExpression *) override {}
+ void visit(VariableExpression *) override {}
+ void visit(VersionedVariableExpression *) override {}
+ void visit(TagPropertyExpression *) override {}
+ void visit(EdgePropertyExpression *) override {}
+ void visit(InputPropertyExpression *) override {}
+ void visit(VariablePropertyExpression *) override {}
+ void visit(DestPropertyExpression *) override {}
+ void visit(SourcePropertyExpression *) override {}
+ void visit(EdgeSrcIdExpression *) override {}
+ void visit(EdgeTypeExpression *) override {}
+ void visit(EdgeRankExpression *) override {}
+ void visit(EdgeDstIdExpression *) override {}
+ void visit(VertexExpression *) override {}
+ void visit(EdgeExpression *) override {}
+ void visit(ColumnExpression *) override {}
+ void visitBinaryExpr(BinaryExpression *) override;
+
+ static bool isAggExpr(const Expression* expr);
+
+private:
+ std::unique_ptr<std::string> var_;
+ std::unique_ptr<std::string> prop_;
+};
+
+} // namespace graph
+} // namespace nebula
+
+#endif // VISITOR_REWRITEAGGEXPRVISITOR_H_
diff --git a/src/visitor/RewriteInputPropVisitor.cpp b/src/visitor/RewriteInputPropVisitor.cpp
index bd5db3d2..b93635ca 100644
--- a/src/visitor/RewriteInputPropVisitor.cpp
+++ b/src/visitor/RewriteInputPropVisitor.cpp
@@ -171,6 +171,14 @@ void RewriteInputPropVisitor::visit(FunctionCallExpression* expr) {
}
}
+void RewriteInputPropVisitor::visit(AggregateExpression* expr) {
+ auto* arg = expr->arg();
+ arg->accept(this);
+ if (ok()) {
+ expr->setArg(std::move(result_).release());
+ }
+}
+
void RewriteInputPropVisitor::visit(TypeCastingExpression* expr) {
expr->operand()->accept(this);
if (ok()) {
diff --git a/src/visitor/RewriteInputPropVisitor.h b/src/visitor/RewriteInputPropVisitor.h
index b14d3639..a15a9479 100644
--- a/src/visitor/RewriteInputPropVisitor.h
+++ b/src/visitor/RewriteInputPropVisitor.h
@@ -47,6 +47,7 @@ private:
void visit(LogicalExpression *) override;
// function call
void visit(FunctionCallExpression *) override;
+ void visit(AggregateExpression *) override;
void visit(UUIDExpression *) override;
// variable expression
void visit(VariableExpression *) override;
diff --git a/src/visitor/RewriteSymExprVisitor.cpp b/src/visitor/RewriteSymExprVisitor.cpp
index 403627ae..892c1fd5 100644
--- a/src/visitor/RewriteSymExprVisitor.cpp
+++ b/src/visitor/RewriteSymExprVisitor.cpp
@@ -93,6 +93,14 @@ void RewriteSymExprVisitor::visit(FunctionCallExpression *expr) {
}
}
+void RewriteSymExprVisitor::visit(AggregateExpression *expr) {
+ auto* arg = expr->arg();
+ arg->accept(this);
+ if (expr_) {
+ expr->setArg(std::move(expr_).release());
+ }
+}
+
void RewriteSymExprVisitor::visit(UUIDExpression *expr) {
UNUSED(expr);
hasWrongType_ = true;
diff --git a/src/visitor/RewriteSymExprVisitor.h b/src/visitor/RewriteSymExprVisitor.h
index b37fa3c3..6e14c56f 100644
--- a/src/visitor/RewriteSymExprVisitor.h
+++ b/src/visitor/RewriteSymExprVisitor.h
@@ -42,6 +42,7 @@ public:
void visit(LogicalExpression *expr) override;
// function call
void visit(FunctionCallExpression *expr) override;
+ void visit(AggregateExpression *expr) override;
void visit(UUIDExpression *expr) override;
// variable expression
void visit(VariableExpression *expr) override;
diff --git a/src/visitor/test/CMakeLists.txt b/src/visitor/test/CMakeLists.txt
index 583e5a4d..231eb9b8 100644
--- a/src/visitor/test/CMakeLists.txt
+++ b/src/visitor/test/CMakeLists.txt
@@ -42,7 +42,6 @@ nebula_add_test(
$<TARGET_OBJECTS:common_meta_client_obj>
$<TARGET_OBJECTS:common_file_based_cluster_id_man_obj>
$<TARGET_OBJECTS:common_function_manager_obj>
- $<TARGET_OBJECTS:common_agg_function_obj>
$<TARGET_OBJECTS:common_conf_obj>
$<TARGET_OBJECTS:common_encryption_obj>
$<TARGET_OBJECTS:common_http_client_obj>
diff --git a/tests/query/v1/test_yield.py b/tests/query/v1/test_yield.py
index 83842f07..31c414a5 100644
--- a/tests/query/v1/test_yield.py
+++ b/tests/query/v1/test_yield.py
@@ -404,13 +404,13 @@ class TestYield(NebulaTestSuite):
resp = self.execute(query)
self.check_resp_failed(resp, ttypes.ErrorCode.E_SEMANTIC_ERROR)
- # query = '''YIELD 1+COUNT(*), 1+1'''
- # resp = self.execute(query)
- # self.check_resp_succeeded(resp)
- # columns = ["(1+count(*))", "(1+1)"]
- # self.check_column_names(resp, columns)
- # expect_result = [[2, 2]]
- # self.check_result(resp, expect_result)
+ query = '''YIELD 1+COUNT(*), 1+1'''
+ resp = self.execute(query)
+ self.check_resp_succeeded(resp)
+ columns = ["(1+COUNT(*))", "(1+1)"]
+ self.check_column_names(resp, columns)
+ expect_result = [[2, 2]]
+ self.check_result(resp, expect_result)
query = '''YIELD COUNT(*), 1+1'''
resp = self.execute(query)
@@ -423,7 +423,7 @@ class TestYield(NebulaTestSuite):
query = '''GO FROM "Carmelo Anthony" OVER like YIELD $$.player.age AS age, like.likeness AS like \
| YIELD COUNT(*), $-.age'''
resp = self.execute(query)
- self.check_resp_failed(resp, ttypes.ErrorCode.E_SEMANTIC_ERROR)
+ self.check_resp_succeeded(resp)
# Test input
query = '''GO FROM "Carmelo Anthony" OVER like YIELD $$.player.age AS age, like.likeness AS like \
diff --git a/tests/tck/features/agg/Agg.feature b/tests/tck/features/agg/Agg.feature
new file mode 100644
index 00000000..d9998f9c
--- /dev/null
+++ b/tests/tck/features/agg/Agg.feature
@@ -0,0 +1,559 @@
+Feature: Basic Aggregate and GroupBy
+
+ Background:
+ Given a graph with space named "nba"
+
+ Scenario: Basic Aggregate
+ When executing query:
+ """
+ YIELD COUNT(*), 1+1
+ """
+ Then the result should be, in any order, with relax comparison:
+ | COUNT(*) | (1+1) |
+ | 1 | 2 |
+ When executing query:
+ """
+ YIELD count(*)+1 ,1+2 ,(INT)abs(count(2))
+ """
+ Then the result should be, in any order, with relax comparison:
+ | (COUNT(*)+1) | (1+2) | (INT)abs(COUNT(2)) |
+ | 2 | 3 | 1 |
+
+ Scenario: Basic GroupBy
+ When executing query:
+ """
+ GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age
+ | YIELD count(*) AS count
+ """
+ Then the result should be, in any order, with relax comparison:
+ | count |
+ | 2 |
+ When executing query:
+ """
+ GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age
+ | YIELD DISTINCT count(*) AS count where $-.age > 40
+ """
+ Then the result should be, in any order, with relax comparison:
+ | count |
+ | 1 |
+ When executing query:
+ """
+ $var=GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age;
+ YIELD count($var.dst) AS count
+ """
+ Then the result should be, in any order, with relax comparison:
+ | count |
+ | 2 |
+ When executing query:
+ """
+ $var=GO FROM "Tim Duncan" OVER like YIELD DISTINCT like._dst AS dst, $$.player.age AS age;
+ YIELD count($var.dst) AS count where $var.age > 40
+ """
+ Then the result should be, in any order, with relax comparison:
+ | count |
+ | 1 |
+ When executing query:
+ """
+ GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age
+ | GROUP BY $-.dst YIELD $-.dst AS dst, avg(distinct $-.age) AS age
+ """
+ Then the result should be, in any order, with relax comparison:
+ | dst | age |
+ | "Tony Parker" | 36.0 |
+ | "Manu Ginobili" | 41.0 |
+ When executing query:
+ """
+ $var=GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age
+ | GROUP BY $-.dst YIELD $-.dst AS dst, avg(distinct $-.age) AS age
+ """
+ Then the result should be, in any order, with relax comparison:
+ | dst | age |
+ | "Tony Parker" | 36.0 |
+ | "Manu Ginobili" | 41.0 |
+ When executing query:
+ """
+ GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age
+ | GROUP BY $-.dst YIELD $-.dst AS dst, avg(distinct $-.age) AS age
+ """
+ Then the result should be, in any order, with relax comparison:
+ | dst | age |
+ | "Tony Parker" | 36.0 |
+ | "Manu Ginobili" | 41.0 |
+ When executing query:
+ """
+ $var=GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age;
+ YIELD DISTINCT $var.dst AS dst, avg(distinct $var.age) AS age
+ """
+ Then the result should be, in any order, with relax comparison:
+ | dst | age |
+ | "Tony Parker" | 36.0 |
+ | "Manu Ginobili" | 41.0 |
+ When executing query:
+ """
+ GO FROM 'Aron Baynes', 'Tracy McGrady' OVER serve
+ YIELD $$.team.name AS name,
+ serve._dst AS id,
+ serve.start_year AS start_year,
+ serve.end_year AS end_year
+ | GROUP BY $-.name, $-.start_year
+ YIELD $-.name AS teamName,
+ $-.start_year AS start_year,
+ MAX($-.start_year),
+ MIN($-.end_year),
+ AVG($-.end_year) AS avg_end_year,
+ STD($-.end_year) AS std_end_year,
+ COUNT($-.id)
+ """
+ Then the result should be, in any order, with relax comparison:
+ | teamName | start_year | MAX($-.start_year) | MIN($-.end_year) | avg_end_year | std_end_year | COUNT($-.id) |
+ | "Celtics" | 2017 | 2017 | 2019 | 2019.0 | 0.0 | 1 |
+ | "Magic" | 2000 | 2000 | 2004 | 2004.0 | 0.0 | 1 |
+ | "Pistons" | 2015 | 2015 | 2017 | 2017.0 | 0.0 | 1 |
+ | "Raptors" | 1997 | 1997 | 2000 | 2000.0 | 0.0 | 1 |
+ | "Rockets" | 2004 | 2004 | 2010 | 2010.0 | 0.0 | 1 |
+ | "Spurs" | 2013 | 2013 | 2013 | 2014.0 | 1.0 | 2 |
+ When executing query:
+ """
+ GO FROM "Marco Belinelli" OVER serve
+ YIELD $$.team.name AS name,
+ serve._dst AS id,
+ serve.start_year AS start_year,
+ serve.end_year AS end_year
+ | GROUP BY $-.start_year
+ YIELD COUNT($-.id),
+ $-.start_year AS start_year,
+ AVG($-.end_year) as avg
+ """
+ Then the result should be, in any order, with relax comparison:
+ | COUNT($-.id) | start_year | avg |
+ | 2 | 2018 | 2018.5 |
+ | 1 | 2017 | 2018.0 |
+ | 1 | 2016 | 2017.0 |
+ | 1 | 2009 | 2010.0 |
+ | 1 | 2007 | 2009.0 |
+ | 1 | 2012 | 2013.0 |
+ | 1 | 2013 | 2015.0 |
+ | 1 | 2015 | 2016.0 |
+ | 1 | 2010 | 2012.0 |
+ When executing query:
+ """
+ GO FROM 'Carmelo Anthony', 'Dwyane Wade' OVER like
+ YIELD $$.player.name AS name,
+ $$.player.age AS dst_age,
+ $$.player.age AS src_age,
+ like.likeness AS likeness
+ | GROUP BY $-.name
+ YIELD $-.name AS name,
+ SUM($-.dst_age) AS sum_dst_age,
+ AVG($-.dst_age) AS avg_dst_age,
+ MAX($-.src_age) AS max_src_age,
+ MIN($-.src_age) AS min_src_age,
+ BIT_AND(1) AS bit_and,
+ BIT_OR(2) AS bit_or,
+ BIT_XOR(3) AS bit_xor,
+ COUNT($-.likeness),
+ COUNT(DISTINCT $-.likeness)
+ """
+ Then the result should be, in any order, with relax comparison:
+ | name | sum_dst_age | avg_dst_age | max_src_age | min_src_age | bit_and | bit_or | bit_xor | COUNT($-.likeness) | COUNT(distinct $-.likeness) |
+ | "LeBron James" | 68 | 34.0 | 34 | 34 | 1 | 2 | 0 | 2 | 1 |
+ | "Chris Paul" | 66 | 33.0 | 33 | 33 | 1 | 2 | 0 | 2 | 1 |
+ | "Dwyane Wade" | 37 | 37.0 | 37 | 37 | 1 | 2 | 3 | 1 | 1 |
+ | "Carmelo Anthony" | 34 | 34.0 | 34 | 34 | 1 | 2 | 3 | 1 | 1 |
+ When executing query:
+ """
+ GO FROM 'Carmelo Anthony', 'Dwyane Wade' OVER like
+ YIELD $$.player.name AS name,
+ $$.player.age AS dst_age,
+ $$.player.age AS src_age,
+ like.likeness AS likeness
+ | GROUP BY $-.name
+ YIELD $-.name AS name,
+ SUM($-.dst_age) AS sum_dst_age,
+ AVG($-.dst_age) AS avg_dst_age,
+ MAX($-.src_age) AS max_src_age,
+ MIN($-.src_age) AS min_src_age,
+ BIT_AND(1) AS bit_and,
+ BIT_OR(2) AS bit_or,
+ BIT_XOR(3) AS bit_xor,
+ COUNT($-.likeness)
+ """
+ Then the result should be, in any order, with relax comparison:
+ | name | sum_dst_age | avg_dst_age | max_src_age | min_src_age | bit_and | bit_or | bit_xor | COUNT($-.likeness) |
+ | "LeBron James" | 68 | 34.0 | 34 | 34 | 1 | 2 | 0 | 2 |
+ | "Chris Paul" | 66 | 33.0 | 33 | 33 | 1 | 2 | 0 | 2 |
+ | "Dwyane Wade" | 37 | 37.0 | 37 | 37 | 1 | 2 | 3 | 1 |
+ | "Carmelo Anthony" | 34 | 34.0 | 34 | 34 | 1 | 2 | 3 | 1 |
+ When executing query:
+ """
+ GO FROM 'Tim Duncan' OVER like YIELD like._dst as dst
+ | GO FROM $-.dst over like YIELD $-.dst as dst, like._dst == 'Tim Duncan' as following
+ | GROUP BY $-.dst
+ YIELD $-.dst AS dst, BIT_OR($-.following) AS following
+ """
+ Then the result should be, in any order, with relax comparison:
+ | dst | following |
+ | "Tony Parker" | BAD_TYPE |
+ | "Manu Ginobili" | BAD_TYPE |
+ When executing query:
+ """
+ GO FROM 'Tim Duncan' OVER like YIELD like._dst as dst
+ | GO FROM $-.dst over like YIELD $-.dst as dst, like._dst == 'Tim Duncan' as following
+ | GROUP BY $-.dst
+ YIELD $-.dst AS dst,
+ BIT_OR(case when $-.following==true then 1 else 0 end) AS following
+ """
+ Then the result should be, in any order, with relax comparison:
+ | dst | following |
+ | "Tony Parker" | 1 |
+ | "Manu Ginobili" | 1 |
+ When executing query:
+ """
+ GO FROM 'Tim Duncan' OVER like YIELD like._dst as dst
+ | GO FROM $-.dst over like YIELD $-.dst as dst, like._dst == 'Tim Duncan' as following
+ | GROUP BY $-.dst
+ YIELD $-.dst AS dst,
+ BIT_AND($-.following) AS following
+ """
+ Then the result should be, in any order, with relax comparison:
+ | dst | following |
+ | "Tony Parker" | BAD_TYPE |
+ | "Manu Ginobili" | BAD_TYPE |
+ When executing query:
+ """
+ GO FROM 'Tim Duncan' OVER like YIELD like._dst as dst
+ | GO FROM $-.dst over like YIELD $-.dst as dst, like._dst == 'Tim Duncan' as following
+ | GROUP BY $-.dst
+ YIELD $-.dst AS dst,
+ BIT_AND(case when $-.following==true then 1 else 0 end) AS following
+ """
+ Then the result should be, in any order, with relax comparison:
+ | dst | following |
+ | "Tony Parker" | 0 |
+ | "Manu Ginobili" | 1 |
+ When executing query:
+ """
+ GO FROM 'Carmelo Anthony', 'Dwyane Wade' OVER like
+ YIELD $$.player.name AS name
+ | GROUP BY $-.name
+ YIELD $-.name AS name,
+ SUM(1.5) AS sum,
+ COUNT(*) AS count,
+ 1+1 AS cal
+ """
+ Then the result should be, in any order, with relax comparison:
+ | name | sum | count | cal |
+ | "LeBron James" | 3.0 | 2 | 2 |
+ | "Chris Paul" | 3.0 | 2 | 2 |
+ | "Dwyane Wade" | 1.5 | 1 | 2 |
+ | "Carmelo Anthony" | 1.5 | 1 | 2 |
+ When executing query:
+ """
+ GO FROM 'Paul Gasol' OVER like
+ YIELD $$.player.age AS age,
+ like._dst AS id
+ | GROUP BY $-.id
+ YIELD $-.id AS id,
+ SUM($-.age) AS age
+ | GO FROM $-.id OVER serve
+ YIELD $$.team.name AS name,
+ $-.age AS sumAge
+ """
+ Then the result should be, in any order, with relax comparison:
+ | name | sumAge |
+ | "Grizzlies" | 34 |
+ | "Raptors" | 34 |
+ | "Lakers" | 40 |
+
+ Scenario: Implicit GroupBy
+ When executing query:
+ """
+ GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age
+ | YIELD $-.dst AS dst, 1+avg(distinct $-.age) AS age, abs(5) as abs
+ """
+ Then the result should be, in any order, with relax comparison:
+ | dst | age | abs |
+ | "Tony Parker" | 37.0 | 5 |
+ | "Manu Ginobili" | 42.0 | 5 |
+ When executing query:
+ """
+ GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age
+ | YIELD $-.dst AS dst, 1+avg(distinct $-.age) AS age where $-.age > 40
+ """
+ Then the result should be, in any order, with relax comparison:
+ | dst | age |
+ | "Manu Ginobili" | 42.0 |
+ When executing query:
+ """
+ GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age
+ | GROUP BY $-.age+1 YIELD (INT)($-.age+1) AS age, 1+count(distinct $-.dst) AS count
+ """
+ Then the result should be, in any order, with relax comparison:
+ | age | count |
+ | 37 | 2 |
+ | 42 | 2 |
+ When executing query:
+ """
+ $var=GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age;
+ YIELD $var.dst AS dst, (INT)abs(1+avg(distinct $var.age)) AS age
+ """
+ Then the result should be, in any order, with relax comparison:
+ | dst | age |
+ | "Tony Parker" | 37 |
+ | "Manu Ginobili" | 42 |
+ When executing query:
+ """
+ $var1=GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst;
+ $var2=GO FROM "Tim Duncan" OVER serve YIELD serve._dst AS dst;
+ YIELD $var1.dst AS dst, count($var1.dst) AS count
+ """
+ Then the result should be, in any order, with relax comparison:
+ | dst | count |
+ | "Tony Parker" | 1 |
+ | "Manu Ginobili" | 1 |
+
+ Scenario: Empty input
+ When executing query:
+ """
+ GO FROM 'noexist' OVER like
+ YIELD $$.player.name AS name
+ | GROUP BY $-.name
+ YIELD $-.name AS name,
+ SUM(1.5) AS sum,
+ COUNT(*) AS count
+ | ORDER BY $-.sum
+ | LIMIT 2
+ """
+ Then the result should be, in order, with relax comparison:
+ | name | sum | count |
+ When executing query:
+ """
+ GO FROM 'noexist' OVER serve
+ YIELD $^.player.name as name,
+ serve.start_year as start,
+ $$.team.name as team
+ | YIELD $-.name as name
+ WHERE $-.start > 20000
+ | GROUP BY $-.name
+ YIELD $-.name AS name
+ """
+ Then the result should be, in order, with relax comparison:
+ | name |
+ When executing query:
+ """
+ GO FROM 'noexist' OVER serve
+ YIELD $^.player.name as name,
+ serve.start_year as start,
+ $$.team.name as team
+ | YIELD $-.name as name
+ WHERE $-.start > 20000
+ | Limit 1
+ """
+ Then the result should be, in any order, with relax comparison:
+ | name |
+
+ Scenario: Duplicate column
+ When executing query:
+ """
+ GO FROM "Marco Belinelli" OVER serve
+ YIELD $$.team.name AS name,
+ serve._dst AS id,
+ serve.start_year AS start_year,
+ serve.end_year AS start_year
+ | GROUP BY $-.start_year
+ YIELD COUNT($-.id),
+ $-.start_year AS start_year,
+ AVG($-.end_year) as avg
+ """
+ Then a SemanticError should be raised at runtime:
+ When executing query:
+ """
+ GO FROM 'noexist' OVER serve
+ YIELD $^.player.name as name,
+ serve.start_year as start,
+ $$.team.name as name
+ | GROUP BY $-.name
+ YIELD $-.name AS name
+ """
+ Then a SemanticError should be raised at runtime:
+
+ Scenario: order by and limit
+ When executing query:
+ """
+ GO FROM 'Carmelo Anthony', 'Dwyane Wade' OVER like
+ YIELD $$.player.name AS name
+ | GROUP BY $-.name
+ YIELD $-.name AS name,
+ SUM(1.5) AS sum,
+ COUNT(*) AS count
+ | ORDER BY $-.sum, $-.name
+ """
+ Then the result should be, in any order, with relax comparison:
+ | name | sum | count |
+ | "Carmelo Anthony" | 1.5 | 1 |
+ | "Dwyane Wade" | 1.5 | 1 |
+ | "Chris Paul" | 3.0 | 2 |
+ | "LeBron James" | 3.0 | 2 |
+ When executing query:
+ """
+ GO FROM 'Carmelo Anthony', 'Dwyane Wade' OVER like
+ YIELD $$.player.name AS name
+ | GROUP BY $-.name
+ YIELD $-.name AS name,
+ SUM(1.5) AS sum,
+ COUNT(*) AS count
+ | ORDER BY $-.sum, $-.name DESC
+ | LIMIT 2
+ """
+ Then the result should be, in any order, with relax comparison:
+ | name | sum | count |
+ | "Carmelo Anthony" | 1.5 | 1 |
+ | "Dwyane Wade" | 1.5 | 1 |
+
+ Scenario: Error Check
+ When executing query:
+ """
+ YIELD avg(*)+1 ,1+2 ,(INT)abs(min(2))
+ """
+ Then a SemanticError should be raised at runtime: Could not apply aggregation function `AVG(*)' on `*`
+ When executing query:
+ """
+ GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age
+ | GROUP BY $-.dst,$-.x YIELD avg(distinct $-.age) AS age
+ """
+ Then a SemanticError should be raised at runtime: `$-.x', not exist prop `x'
+ When executing query:
+ """
+ GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age
+ | GROUP BY $-.age+1 YIELD $-.age+1,age,avg(distinct $-.age) AS age
+ """
+ Then a SemanticError should be raised at runtime: Not supported expression `age' for props deduction.
+ When executing query:
+ """
+ GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age
+ | GROUP BY $-.age+1 YIELD $-.age,avg(distinct $-.age) AS age
+ """
+ Then a SemanticError should be raised at runtime: Yield non-agg expression `$-.age' must be functionally dependent on items in GROUP BY clause
+ When executing query:
+ """
+ GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age
+ | GROUP BY $-.age+1 YIELD $-.age+1,abs(avg(distinct count($-.age))) AS age
+ """
+ Then a SemanticError should be raised at runtime: Aggregate function nesting is not allowed: `abs(AVG(distinct COUNT($-.age)))'
+ When executing query:
+ """
+ GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age
+ | GROUP BY $-.age+1 YIELD $-.age+1,avg(distinct count($-.age+1)) AS age
+ """
+ Then a SemanticError should be raised at runtime: Aggregate function nesting is not allowed: `AVG(distinct COUNT(($-.age+1)))'
+ When executing query:
+ """
+ GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age
+ | GROUP BY avg($-.age+1)+1 YIELD $-.age,avg(distinct $-.age) AS age
+ """
+ Then a SemanticError should be raised at runtime: Group `(AVG(($-.age+1))+1)' invalid
+ When executing query:
+ """
+ $var=GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age;
+ GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst, $$.player.age AS age
+ | YIELD $var.dst AS dst, avg(distinct $-.age) AS age
+ """
+ Then a SemanticError should be raised at runtime: Not support both input and variable in GroupBy sentence.
+ When executing query:
+ """
+ $var1=GO FROM "Tim Duncan" OVER like YIELD like._dst AS dst;
+ $var2=GO FROM "Tim Duncan" OVER serve YIELD serve._dst AS dst;
+ YIELD count($var1.dst),$var2.dst AS count
+ """
+ Then a SemanticError should be raised at runtime: Only one variable allowed to use.
+ When executing query:
+ """
+ GO FROM "Tim Duncan" OVER like YIELD count(*)
+ """
+ Then a SemanticError should be raised at runtime: `COUNT(*)', not support aggregate function in go sentence.
+ When executing query:
+ """
+ GO FROM "Tim Duncan" OVER like where count(*) > 2
+ """
+ Then a SemanticError should be raised at runtime: `(COUNT(*)>2)', not support aggregate function in where sentence.
+ When executing query:
+ """
+ GO FROM "Marco Belinelli" OVER serve
+ YIELD $$.team.name AS name,
+ serve.end_year AS end_year
+ | GROUP BY $-.end_year
+ YIELD COUNT($$.team.name)
+ """
+ Then a SemanticError should be raised at runtime: Only support input and variable in GroupBy sentence.
+ When executing query:
+ """
+ GO FROM "Marco Belinelli" OVER serve
+ YIELD $$.team.name AS name,
+ serve._dst AS id
+ | GROUP BY $-.start_year
+ YIELD COUNT($-.id)
+ """
+ Then a SemanticError should be raised at runtime: `$-.start_year', not exist prop `start_year'
+ When executing query:
+ """
+ GO FROM "Marco Belinelli" OVER serve
+ YIELD $$.team.name AS name,
+ serve._dst AS id
+ | GROUP BY team
+ YIELD COUNT($-.id),
+ $-.name AS teamName
+ """
+ Then a SemanticError should be raised at runtime: Group `team' invalid
+ When executing query:
+ """
+ GO FROM "Marco Belinelli" OVER serve
+ YIELD $$.team.name AS name,
+ serve._dst AS id
+ | GROUP BY $-.name
+ YIELD COUNT($-.start_year)
+ """
+ Then a SemanticError should be raised at runtime: `$-.start_year', not exist prop `start_year'
+ When executing query:
+ """
+ GO FROM "Marco Belinelli" OVER serve
+ YIELD $$.team.name AS name,
+ serve._dst AS id
+ | GROUP BY $-.name
+ YIELD SUM(*)
+ """
+ Then a SemanticError should be raised at runtime: Could not apply aggregation function `SUM(*)' on `*`
+ When executing query:
+ """
+ GO FROM "Marco Belinelli" OVER serve
+ YIELD $$.team.name AS name,
+ serve._dst AS id
+ | GROUP BY $-.name
+ YIELD COUNT($-.name, $-.id)
+ """
+ Then a SyntaxError should be raised at runtime: syntax error near `, $-.id)'
+ When executing query:
+ """
+ GO FROM "Marco Belinelli" OVER serve
+ YIELD $$.team.name AS name,
+ serve._dst AS id
+ | GROUP BY $-.name, SUM($-.id)
+ YIELD $-.name, SUM($-.id)
+ """
+ Then a SemanticError should be raised at runtime: Group `SUM($-.id)' invalid
+ When executing query:
+ """
+ GO FROM "Marco Belinelli" OVER serve
+ YIELD $$.team.name AS name,
+ COUNT(serve._dst) AS id
+ """
+ Then a SemanticError should be raised at runtime: `COUNT(serve._dst) AS id', not support aggregate function in go sentence.
+
+# When executing query:
+# """
+# GO FROM "Marco Belinelli" OVER serve
+# YIELD $$.team.name AS name,
+# serve.end_year AS end_year
+# | GROUP BY $-.end_year
+# YIELD COUNT($var)
+# """
+# Then a SemanticError should be raised at runtime:
diff --git a/tests/tck/features/go/GO.feature b/tests/tck/features/go/GO.feature
index e8b4ae4c..ab7754b5 100644
--- a/tests/tck/features/go/GO.feature
+++ b/tests/tck/features/go/GO.feature
@@ -1610,7 +1610,8 @@ Feature: Go Sentence
When executing query:
"""
GO 1 TO 2 STEPS FROM "Tim Duncan" OVER like WHERE like._dst != "YAO MING" YIELD like._dst AS vid
- | GROUP BY $-.vid YIELD 1 AS id | GROUP BY $-.id YIELD COUNT($-.id);
+ | GROUP BY $-.vid YIELD 1 AS id
+ | GROUP BY $-.id YIELD COUNT($-.id);
"""
Then the result should be, in any order, with relax comparison:
| COUNT($-.id) |
diff --git a/tests/tck/features/go/GroupbyLimit.IntVid.feature b/tests/tck/features/go/GroupbyLimit.IntVid.feature
index e919b036..ce2ad2ae 100644
--- a/tests/tck/features/go/GroupbyLimit.IntVid.feature
+++ b/tests/tck/features/go/GroupbyLimit.IntVid.feature
@@ -178,7 +178,7 @@ Feature: Groupby & limit Sentence
Then a SemanticError should be raised at runtime:
When executing query:
"""
- GO FROM hash("Carmelo Anthony"),hash("Dwyane Wade") OVER like YIELD $$.player.name AS name | GROUP BY $-.name, abs(5)
+ GO FROM hash("Carmelo Anthony"),hash("Dwyane Wade") OVER like YIELD $$.player.name AS name | GROUP BY $-.name
YIELD $-.name AS name, SUM(1.5) AS sum, COUNT(*) AS count, 1+1 AS cal
"""
Then the result should be, in any order, with relax comparison:
@@ -275,7 +275,7 @@ Feature: Groupby & limit Sentence
Scenario: Groupby works with orderby or limit test
When executing query:
"""
- GO FROM hash("Carmelo Anthony"),hash("Dwyane Wade") OVER like YIELD $$.player.name AS name | GROUP BY $-.name, abs(5)
+ GO FROM hash("Carmelo Anthony"),hash("Dwyane Wade") OVER like YIELD $$.player.name AS name | GROUP BY $-.name
YIELD $-.name AS name, SUM(1.5) AS sum, COUNT(*) AS count | ORDER BY $-.sum, $-.name
"""
Then the result should be, in order, with relax comparison:
@@ -286,7 +286,7 @@ Feature: Groupby & limit Sentence
| "LeBron James" | 3.0 | 2 |
When executing query:
"""
- GO FROM hash("Carmelo Anthony"),hash("Dwyane Wade") OVER like YIELD $$.player.name AS name | GROUP BY $-.name, abs(5)
+ GO FROM hash("Carmelo Anthony"),hash("Dwyane Wade") OVER like YIELD $$.player.name AS name | GROUP BY $-.name
YIELD $-.name AS name, SUM(1.5) AS sum, COUNT(*) AS count | ORDER BY $-.sum, $-.name DESC | LIMIT 2
"""
Then the result should be, in order, with relax comparison:
@@ -303,7 +303,7 @@ Feature: Groupby & limit Sentence
Then a SemanticError should be raised at runtime:
When executing query:
"""
- GO FROM hash("NON EXIST VERTEX ID") OVER like YIELD $$.player.name AS name | GROUP BY $-.name, abs(5)
+ GO FROM hash("NON EXIST VERTEX ID") OVER like YIELD $$.player.name AS name | GROUP BY $-.name
YIELD $-.name AS name, SUM(1.5) AS sum, COUNT(*) AS count | ORDER BY $-.sum | LIMIT 2
"""
Then the result should be, in order, with relax comparison:
diff --git a/tests/tck/features/go/GroupbyLimit.feature b/tests/tck/features/go/GroupbyLimit.feature
index 6445ce97..fb2a65cb 100644
--- a/tests/tck/features/go/GroupbyLimit.feature
+++ b/tests/tck/features/go/GroupbyLimit.feature
@@ -193,7 +193,7 @@ Feature: Groupby & limit Sentence
Then a SemanticError should be raised at runtime:
When executing query:
"""
- GO FROM "Carmelo Anthony","Dwyane Wade" OVER like YIELD $$.player.name AS name | GROUP BY $-.name, abs(5)
+ GO FROM "Carmelo Anthony","Dwyane Wade" OVER like YIELD $$.player.name AS name | GROUP BY $-.name
YIELD $-.name AS name, SUM(1.5) AS sum, COUNT(*) AS count, 1+1 AS cal
"""
Then the result should be, in any order, with relax comparison:
@@ -290,7 +290,7 @@ Feature: Groupby & limit Sentence
Scenario: Groupby works with orderby or limit test
When executing query:
"""
- GO FROM "Carmelo Anthony","Dwyane Wade" OVER like YIELD $$.player.name AS name | GROUP BY $-.name, abs(5)
+ GO FROM "Carmelo Anthony","Dwyane Wade" OVER like YIELD $$.player.name AS name | GROUP BY $-.name
YIELD $-.name AS name, SUM(1.5) AS sum, COUNT(*) AS count | ORDER BY $-.sum, $-.name
"""
Then the result should be, in order, with relax comparison:
@@ -301,7 +301,7 @@ Feature: Groupby & limit Sentence
| "LeBron James" | 3.0 | 2 |
When executing query:
"""
- GO FROM "Carmelo Anthony","Dwyane Wade" OVER like YIELD $$.player.name AS name | GROUP BY $-.name, abs(5)
+ GO FROM "Carmelo Anthony","Dwyane Wade" OVER like YIELD $$.player.name AS name | GROUP BY $-.name
YIELD $-.name AS name, SUM(1.5) AS sum, COUNT(*) AS count | ORDER BY $-.sum, $-.name DESC | LIMIT 2
"""
Then the result should be, in order, with relax comparison:
@@ -318,7 +318,7 @@ Feature: Groupby & limit Sentence
Then a SemanticError should be raised at runtime:
When executing query:
"""
- GO FROM "NON EXIST VERTEX ID" OVER like YIELD $$.player.name AS name | GROUP BY $-.name, abs(5)
+ GO FROM "NON EXIST VERTEX ID" OVER like YIELD $$.player.name AS name | GROUP BY $-.name
YIELD $-.name AS name, SUM(1.5) AS sum, COUNT(*) AS count | ORDER BY $-.sum | LIMIT 2
"""
Then the result should be, in order, with relax comparison:
--
GitLab