Skip to content
This repository has been archived by the owner on Dec 1, 2022. It is now read-only.

Commit

Permalink
fix function case
Browse files Browse the repository at this point in the history 
add tck

fix tck

fix ut
  • Loading branch information
@czpmango
czpmango committed Apr 7, 2021
1 parent 8063659 commit 9c36f74
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 72 deletions.
10 changes: 6 additions & 4 deletions src/parser/parser.yy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -896,8 +896,9 @@ function_call_expression
}
}
| LABEL L_PAREN STAR R_PAREN {
std::transform($1->begin(), $1->end(), $1->begin(), ::toupper);
if (!$1->compare("COUNT")) {
auto func = *$1;
std::transform(func.begin(), func.end(), func.begin(), ::toupper);
if (!func.compare("COUNT")) {
auto star = new ConstantExpression(std::string("*"));
$$ = new AggregateExpression($1, star, false);
} else {
Expand All @@ -906,8 +907,9 @@ function_call_expression
}
}
| LABEL L_PAREN KW_DISTINCT STAR R_PAREN {
std::transform($1->begin(), $1->end(), $1->begin(), ::toupper);
if (!$1->compare("COUNT")) {
auto func = *$1;
std::transform(func.begin(), func.end(), func.begin(), ::toupper);
if (!func.compare("COUNT")) {
auto star = new ConstantExpression(std::string("*"));
$$ = new AggregateExpression($1, star, true);
} else {
Expand Down
26 changes: 10 additions & 16 deletions src/util/ExpressionUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,26 +217,20 @@ Status ExpressionUtils::checkAggExpr(const AggregateExpression* aggExpr) {

NG_RETURN_IF_ERROR(AggFunctionManager::find(func));

auto* aggArg = aggExpr->arg();
if (graph::ExpressionUtils::findAny(aggArg,
{Expression::Kind::kAggregate})) {
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 (func.compare("COUNT")) {
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<const PropertyExpression*>(aggArg);
if (*propExpr->prop() == "*") {
return Status::SemanticError(
"Could not apply aggregation function `%s' on `%s'",
aggExpr->toString().c_str(), propExpr->toString().c_str());
}
// check : $-.* or $var.* can only be applied on `COUNT`
if (func.compare("COUNT") && (aggArg->kind() == Expression::Kind::kInputProperty ||
aggArg->kind() == Expression::Kind::kVarProperty)) {
auto propExpr = static_cast<const PropertyExpression *>(aggArg);
if (*propExpr->prop() == "*") {
return Status::SemanticError("Could not apply aggregation function `%s' on `%s'",
aggExpr->toString().c_str(),
propExpr->toString().c_str());
}
}

Expand Down
32 changes: 1 addition & 31 deletions src/validator/GroupByValidator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ Status GroupByValidator::validateYield(const YieldClause* yieldClause) {
// collect exprs for check later
if (colExpr->kind() == Expression::Kind::kAggregate) {
auto* aggExpr = static_cast<AggregateExpression*>(colExpr);
NG_RETURN_IF_ERROR(checkAggExpr(aggExpr));
NG_RETURN_IF_ERROR(ExpressionUtils::checkAggExpr(aggExpr));
} else {
yieldCols_.emplace_back(colExpr);
}
Expand Down Expand Up @@ -187,35 +187,5 @@ Status GroupByValidator::rewriteInnerAggExpr(YieldColumn* col, bool& rewrited) {
return Status::OK();
}

Status GroupByValidator::checkAggExpr(AggregateExpression* aggExpr) {
auto func = *aggExpr->name();
std::transform(func.begin(), func.end(), func.begin(), ::toupper);
NG_RETURN_IF_ERROR(AggFunctionManager::find(func));

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 (func.compare("COUNT")) {
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
30 changes: 13 additions & 17 deletions src/validator/GroupByValidator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,15 @@
#define VALIDATOR_GROUPBY_VALIDATOR_H_

#include "common/base/Base.h"
#include "validator/Validator.h"
#include "planner/Query.h"

#include "validator/Validator.h"

namespace nebula {
namespace graph {

class GroupByValidator final : public Validator {
public:
GroupByValidator(Sentence *sentence, QueryContext *context)
: Validator(sentence, context) {}
GroupByValidator(Sentence *sentence, QueryContext *context) : Validator(sentence, context) {}

Status validateImpl() override;

Expand All @@ -30,27 +28,25 @@ class GroupByValidator final : public Validator {
Status validateYield(const YieldClause *yieldClause);

Status groupClauseSemanticCheck();
Status rewriteInnerAggExpr(YieldColumn* col, bool& rewrited);
Status checkAggExpr(AggregateExpression* aggExpr);
Status rewriteInnerAggExpr(YieldColumn *col, bool &rewrited);

private:
std::vector<Expression*> yieldCols_;
std::vector<Expression *> yieldCols_;

// key: alias, value: input name
std::unordered_map<std::string, YieldColumn*> aliases_;
std::unordered_map<std::string, YieldColumn *> aliases_;

bool needGenProject_{false};
std::vector<std::string> outputColumnNames_;
std::vector<std::string> projOutputColumnNames_;
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_;
YieldColumns *projCols_;

std::vector<Expression*> groupKeys_;
std::vector<Expression*> groupItems_;
std::vector<Expression *> groupKeys_;
std::vector<Expression *> groupItems_;
};


} // namespace graph
} // namespace nebula
} // namespace graph
} // namespace nebula
#endif
4 changes: 2 additions & 2 deletions src/validator/test/GroupByValidatorTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ TEST_F(GroupByValidatorTest, InvalidTest) {
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");
EXPECT_EQ(std::string(result.message()), "SemanticError: Group `(count(*)+1)' invalid");
}
{
// use groupby without input
Expand Down Expand Up @@ -338,7 +338,7 @@ TEST_F(GroupByValidatorTest, InvalidTest) {
"COUNT(like._dst)+1 AS id ";
auto result = checkResult(query);
EXPECT_EQ(std::string(result.message()),
"SemanticError: `((COUNT(*)+1)>3)', "
"SemanticError: `((count(*)+1)>3)', "
"not support aggregate function in where sentence.");
}
{
Expand Down
2 changes: 1 addition & 1 deletion tests/tck/features/aggregate/Agg.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ Feature: Basic Aggregate and GroupBy
"""
GO FROM "Tim Duncan" OVER like YIELD count(*)
"""
Then a SemanticError should be raised at runtime: `COUNT(*)', not support aggregate function in go sentence.
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
Expand Down
2 changes: 1 addition & 1 deletion tests/tck/features/bugfix/MatchUsedInPipe.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Feature: Test match used in pipe
MATCH (n:player{name:"Tim Duncan"})-[]-(m) RETURN n,m | GROUP BY $-.n, $-.m YIELD $-.n, $-.m, count(*);
"""
Then the result should be, in any order, with relax comparison:
| $-.n | $-.m | COUNT(*) |
| $-.n | $-.m | count(*) |
| ("Tim Duncan") | ("Spurs") | 1 |
| ("Tim Duncan") | ("Shaquile O'Neal") | 1 |
| ("Tim Duncan") | ("Tiago Splitter") | 1 |
Expand Down
16 changes: 16 additions & 0 deletions tests/tck/features/yield/yield.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -524,3 +524,19 @@ Feature: Yield Sentence
Then the result should be, in any order, with relax comparison:
| c |
| [123,456,789] |

Scenario: function name case test
When executing query:
"""
yield [aBs(-3), tofloat(3), bit_Or(1, 2)] AS function_case_test
"""
Then the result should be, in any order, with relax comparison:
| function_case_test |
| [3, 3.0, 3] |
When executing query:
"""
yield counT(*), aVg(3), bit_Or(1)
"""
Then the result should be, in any order, with relax comparison:
| counT(*) | aVg(3) | bit_Or(1) |
| 1 | 3.0 | 1 |

0 comments on commit 9c36f74

Please sign in to comment.