Skip to content

Commit

Permalink
disable agg function in ngql's yield clause & where clause (#3597)
Browse files Browse the repository at this point in the history
* disable agg function in ngql's yield clause

* fix error

* fix ci test error
  • Loading branch information
nevermore3 committed Dec 31, 2021
1 parent 0ab305d commit 7c5e431
Show file tree
Hide file tree
Showing 15 changed files with 58 additions and 49 deletions.
3 changes: 1 addition & 2 deletions src/graph/validator/FetchEdgesValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,7 @@ Status FetchEdgesValidator::validateYield(const YieldClause *yield) {
auto pool = qctx_->objPool();
auto *newCols = pool->add(new YieldColumns());
for (auto col : yield->columns()) {
if (ExpressionUtils::hasAny(col->expr(),
{Expression::Kind::kVertex, Expression::Kind::kPathBuild})) {
if (ExpressionUtils::hasAny(col->expr(), {Expression::Kind::kVertex})) {
return Status::SemanticError("illegal yield clauses `%s'", col->toString().c_str());
}
col->setExpr(ExpressionUtils::rewriteLabelAttr2EdgeProp(col->expr()));
Expand Down
3 changes: 1 addition & 2 deletions src/graph/validator/FetchVerticesValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ Status FetchVerticesValidator::validateYield(YieldClause *yield) {

auto &exprProps = fetchCtx_->exprProps;
for (auto col : yield->columns()) {
if (ExpressionUtils::hasAny(col->expr(),
{Expression::Kind::kEdge, Expression::Kind::kPathBuild})) {
if (ExpressionUtils::hasAny(col->expr(), {Expression::Kind::kEdge})) {
return Status::SemanticError("illegal yield clauses `%s'", col->toString().c_str());
}
col->setExpr(ExpressionUtils::rewriteLabelAttr2TagProp(col->expr()));
Expand Down
3 changes: 1 addition & 2 deletions src/graph/validator/FindPathValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ Status FindPathValidator::validateWhere(WhereClause* where) {
// Not Support $-、$var、$$.tag.prop、$^.tag.prop、agg
auto expr = where->filter();
if (ExpressionUtils::findAny(expr,
{Expression::Kind::kAggregate,
Expression::Kind::kSrcProperty,
{Expression::Kind::kSrcProperty,
Expression::Kind::kDstProperty,
Expression::Kind::kVarProperty,
Expression::Kind::kInputProperty})) {
Expand Down
9 changes: 0 additions & 9 deletions src/graph/validator/GoValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,6 @@ Status GoValidator::validateWhere(WhereClause* where) {
}

auto expr = where->filter();
if (graph::ExpressionUtils::findAny(expr, {Expression::Kind::kAggregate})) {
return Status::SemanticError("`%s', not support aggregate function in where sentence.",
expr->toString().c_str());
}
where->setFilter(ExpressionUtils::rewriteLabelAttr2EdgeProp(expr));
auto foldRes = ExpressionUtils::foldConstantExpr(where->filter());
NG_RETURN_IF_ERROR(foldRes);
Expand Down Expand Up @@ -131,11 +127,6 @@ Status GoValidator::validateYield(YieldClause* yield) {
auto& exprProps = goCtx_->exprProps;

for (auto col : yield->columns()) {
if (ExpressionUtils::hasAny(col->expr(),
{Expression::Kind::kAggregate, Expression::Kind::kPathBuild})) {
return Status::SemanticError("`%s' is not support in go sentence.", col->toString().c_str());
}

auto vertexExpr = ExpressionUtils::findAny(col->expr(), {Expression::Kind::kVertex});
if (vertexExpr != nullptr &&
static_cast<const VertexExpression*>(vertexExpr)->name() == "VERTEX") {
Expand Down
6 changes: 2 additions & 4 deletions src/graph/validator/LookupValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,7 @@ Status LookupValidator::validateYieldEdge() {
auto yield = sentence()->yieldClause();
auto yieldExpr = lookupCtx_->yieldExpr;
for (auto col : yield->columns()) {
if (ExpressionUtils::hasAny(col->expr(),
{Expression::Kind::kAggregate, Expression::Kind::kVertex})) {
if (ExpressionUtils::hasAny(col->expr(), {Expression::Kind::kVertex})) {
return Status::SemanticError("illegal yield clauses `%s'", col->toString().c_str());
}
if (col->expr()->kind() == Expression::Kind::kLabelAttribute) {
Expand All @@ -124,8 +123,7 @@ Status LookupValidator::validateYieldTag() {
auto yield = sentence()->yieldClause();
auto yieldExpr = lookupCtx_->yieldExpr;
for (auto col : yield->columns()) {
if (ExpressionUtils::hasAny(col->expr(),
{Expression::Kind::kAggregate, Expression::Kind::kEdge})) {
if (ExpressionUtils::hasAny(col->expr(), {Expression::Kind::kEdge})) {
return Status::SemanticError("illegal yield clauses `%s'", col->toString().c_str());
}
if (col->expr()->kind() == Expression::Kind::kLabelAttribute) {
Expand Down
15 changes: 7 additions & 8 deletions src/graph/validator/test/GroupByValidatorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -302,33 +302,32 @@ TEST_F(GroupByValidatorTest, InvalidTest) {
EXPECT_EQ(std::string(result.message()), "SemanticError: Group `SUM($-.age)' invalid");
}
{
// yield without group by
std::string query =
"GO FROM \"1\" OVER like YIELD $^.person.age AS age, "
"COUNT(like._dst) AS id ";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
"SemanticError: `COUNT(like._dst) AS id' is not support in go sentence.");
"SyntaxError: Invalid use of aggregating function in yield clause. near "
"`$^.person.age AS age, COUNT(like._dst) AS id'");
}
{
// 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' is not support in go sentence.");
"SyntaxError: Invalid use of aggregating function in yield clause. near "
"`$^.person.age AS age, COUNT(like._dst)+1 AS id'");
}
{
// 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.");
EXPECT_EQ(
std::string(result.message()),
"SyntaxError: Invalid use of aggregating function in where clause. near `count(*) + 1 >3'");
}
{
// yield col not in group output
Expand Down
3 changes: 2 additions & 1 deletion src/graph/validator/test/LookupValidatorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ TEST_F(LookupValidatorTest, wrongYield) {
{
std::string query = "LOOKUP ON person YIELD count(*)";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()), "SemanticError: illegal yield clauses `count(*)'");
EXPECT_EQ(std::string(result.message()),
"SyntaxError: Invalid use of aggregating function in yield clause. near `count(*)'");
}
{
std::string query = "LOOKUP ON person YIELD vertex as node, edge";
Expand Down
5 changes: 3 additions & 2 deletions src/graph/validator/test/QueryValidatorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -908,8 +908,9 @@ TEST_F(QueryValidatorTest, GoInvalid) {
// yield agg without groupBy is not supported
std::string query = "GO FROM \"2\" OVER like YIELD COUNT(123);";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
"SemanticError: `COUNT(123)' is not support in go sentence.");
EXPECT_EQ(
std::string(result.message()),
"SyntaxError: Invalid use of aggregating function in yield clause. near `COUNT(123)'");
}
{
std::string query =
Expand Down
33 changes: 28 additions & 5 deletions src/parser/parser.yy
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ static constexpr size_t kCommentLengthLimit = 256;
nebula::WhereClause *lookup_where_clause;
nebula::WhenClause *when_clause;
nebula::YieldClause *yield_clause;
nebula::YieldClause *group_by_yield_clause;
nebula::YieldColumns *yield_columns;
nebula::YieldColumn *yield_column;
nebula::TruncateClause *truncate_clause;
Expand Down Expand Up @@ -262,7 +263,7 @@ static constexpr size_t kCommentLengthLimit = 256;
%type <lookup_where_clause> lookup_where_clause
%type <when_clause> when_clause
%type <truncate_clause> truncate_clause
%type <yield_clause> yield_clause
%type <yield_clause> yield_clause group_by_yield_clause
%type <yield_columns> yield_columns
%type <yield_column> yield_column
%type <vertex_tag_list> vertex_tag_list
Expand Down Expand Up @@ -1472,7 +1473,12 @@ over_clause

where_clause
: %empty { $$ = nullptr; }
| KW_WHERE expression { $$ = new WhereClause($2); }
| KW_WHERE expression {
if (graph::ExpressionUtils::findAny($2, {Expression::Kind::kAggregate})) {
throw nebula::GraphParser::syntax_error(@2, "Invalid use of aggregating function in where clause.");
}
$$ = new WhereClause($2);
}
;

when_clause
Expand All @@ -1482,8 +1488,20 @@ when_clause

yield_clause
: %empty { $$ = nullptr; }
| KW_YIELD yield_columns { $$ = new YieldClause($2); }
| KW_YIELD KW_DISTINCT yield_columns { $$ = new YieldClause($3, true); }
| KW_YIELD yield_columns {
if ($2->hasAgg()) {
delete($2);
throw nebula::GraphParser::syntax_error(@2, "Invalid use of aggregating function in yield clause.");
}
$$ = new YieldClause($2);
}
| KW_YIELD KW_DISTINCT yield_columns {
if ($3->hasAgg()) {
delete($3);
throw nebula::GraphParser::syntax_error(@3, "Invalid use of aggregating function in yield clause.");
}
$$ = new YieldClause($3, true);
}
;

yield_columns
Expand Down Expand Up @@ -2304,8 +2322,13 @@ limit_sentence
}
;

group_by_yield_clause
: KW_YIELD yield_columns { $$ = new YieldClause($2); }
| KW_YIELD KW_DISTINCT yield_columns { $$ = new YieldClause($3, true); }
;

group_by_sentence
: KW_GROUP KW_BY group_clause yield_clause {
: KW_GROUP KW_BY group_clause group_by_yield_clause {
auto group = new GroupBySentence();
group->setGroupClause($3);
group->setYieldClause($4);
Expand Down
3 changes: 1 addition & 2 deletions src/parser/test/ParserTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2721,8 +2721,7 @@ TEST_F(ParserTest, MatchErrorCheck) {
auto result = parse(query);
ASSERT_FALSE(result.ok());
auto error =
"SyntaxError: Invalid use of aggregating "
"function in this context. near `WHERE count(v)>1'";
"SyntaxError: Invalid use of aggregating function in where clause. near `count(v)>1'";
ASSERT_EQ(error, result.status().toString());
}
}
Expand Down
12 changes: 6 additions & 6 deletions tests/tck/features/aggregate/Agg.feature
Original file line number Diff line number Diff line change
Expand Up @@ -691,12 +691,12 @@ Feature: Basic Aggregate and GroupBy
"""
GO FROM "Tim Duncan" OVER like YIELD count(*)
"""
Then a SemanticError should be raised at runtime: `count(*)' is not support in go sentence.
Then a SyntaxError should be raised at runtime: Invalid use of aggregating function in yield clause. near `count(*)'
When executing query:
"""
GO FROM "Tim Duncan" OVER like where COUNT(*) > 2 YIELD like._dst
"""
Then a SemanticError should be raised at runtime: `(COUNT(*)>2)', not support aggregate function in where sentence.
Then a SyntaxError should be raised at runtime: Invalid use of aggregating function in where clause. near `COUNT(*) > 2'
When executing query:
"""
GO FROM "Marco Belinelli" OVER serve
Expand Down Expand Up @@ -767,30 +767,30 @@ Feature: Basic Aggregate and GroupBy
YIELD $$.team.name AS name,
COUNT(serve._dst) AS id
"""
Then a SemanticError should be raised at runtime: `COUNT(serve._dst) AS id' is not support in go sentence.
Then a SyntaxError should be raised at runtime: Invalid use of aggregating function in yield clause. near `$$.team.name AS name, COUNT(serve._dst) AS id'
When executing query:
"""
MATCH (v:player)
WHERE avg(v.player.age) > 1
RETURN v.player.age
"""
Then a SyntaxError should be raised at runtime: Invalid use of aggregating function in this context. near `WHERE avg(v.player.age) > 1'
Then a SyntaxError should be raised at runtime: Invalid use of aggregating function in where clause. near `avg(v.player.age) > 1'
When executing query:
"""
MATCH (v:player)
WITH v
WHERE avg(v.player.age) > 1
RETURN v.player.age
"""
Then a SyntaxError should be raised at runtime: Invalid use of aggregating function in this context. near `WHERE avg(v.player.age) > 1'
Then a SyntaxError should be raised at runtime: Invalid use of aggregating function in where clause. near `avg(v.player.age) > 1'
When executing query:
"""
MATCH (v:player)
WITH DISTINCT v
WHERE avg(v.player.age) > 1
RETURN v.player.age
"""
Then a SyntaxError should be raised at runtime: Invalid use of aggregating function in this context. near `WHERE avg(v.player.age) > 1'
Then a SyntaxError should be raised at runtime: Invalid use of aggregating function in where clause. near `avg(v.player.age) > 1'
# When executing query:
# """
Expand Down
2 changes: 1 addition & 1 deletion tests/tck/features/go/GroupbyLimit.IntVid.feature
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ Feature: Groupby & limit Sentence
"""
GO FROM hash("Marco Belinelli") OVER serve YIELD $$.team.name AS name, COUNT(serve._dst) AS id
"""
Then a SemanticError should be raised at runtime:
Then a SyntaxError should be raised at runtime: Invalid use of aggregating function in yield clause. near `$$.team.name AS name, COUNT(serve._dst) AS id'

Scenario: Limit test
When executing query:
Expand Down
2 changes: 1 addition & 1 deletion tests/tck/features/go/GroupbyLimit.feature
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Feature: Groupby & limit Sentence
"""
GO FROM "Marco Belinelli" OVER serve YIELD $$.team.name AS name, COUNT(serve._dst) AS id
"""
Then a SemanticError should be raised at runtime:
Then a SyntaxError should be raised at runtime: Invalid use of aggregating function in yield clause. near `$$.team.name AS name, COUNT(serve._dst) AS id'

Scenario: Limit test
When executing query:
Expand Down
4 changes: 2 additions & 2 deletions tests/tck/features/yield/yield.IntVid.feature
Original file line number Diff line number Diff line change
Expand Up @@ -441,12 +441,12 @@ Feature: Yield Sentence
"""
GO FROM hash("Tim Duncan") OVER like YIELD like._dst as id, $$ as dst | YIELD $-.dst where count($-.id) > 2
"""
Then a SemanticError should be raised at runtime: `(count($-.id)>2)', not support aggregate function in where sentence.
Then a SyntaxError should be raised at runtime: Invalid use of aggregating function in where clause. near `count($-.id) > 2'
When executing query:
"""
$var = go from hash("Tim Duncan") over like yield like._dst as id, $$ as dst; yield $var.dst where count($var.id) > 2
"""
Then a SemanticError should be raised at runtime: `(count($var.id)>2)', not support aggregate function in where sentence.
Then a SyntaxError should be raised at runtime: Invalid use of aggregating function in where clause. near `count($var.id) > 2'

Scenario: EmptyInput
When executing query:
Expand Down
4 changes: 2 additions & 2 deletions tests/tck/features/yield/yield.feature
Original file line number Diff line number Diff line change
Expand Up @@ -471,12 +471,12 @@ Feature: Yield Sentence
"""
GO FROM "Tim Duncan" OVER like YIELD like._dst as id, $$ as dst | YIELD $-.dst where count($-.id) > 2
"""
Then a SemanticError should be raised at runtime: `(count($-.id)>2)', not support aggregate function in where sentence.
Then a SyntaxError should be raised at runtime: Invalid use of aggregating function in where clause. near `count($-.id) > 2'
When executing query:
"""
$var = go from "Tim Duncan" over like yield like._dst as id, $$ as dst; yield $var.dst where count($var.id) > 2
"""
Then a SemanticError should be raised at runtime: `(count($var.id)>2)', not support aggregate function in where sentence.
Then a SyntaxError should be raised at runtime: Invalid use of aggregating function in where clause. near `count($var.id) > 2'

Scenario: DuplicateColumn
When executing query:
Expand Down

0 comments on commit 7c5e431

Please sign in to comment.