diff --git a/src/parser/parser.yy b/src/parser/parser.yy index 718b53634..36d98cdb5 100644 --- a/src/parser/parser.yy +++ b/src/parser/parser.yy @@ -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 { @@ -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 { diff --git a/src/util/ExpressionUtils.cpp b/src/util/ExpressionUtils.cpp index 7cd518501..e71e9b926 100644 --- a/src/util/ExpressionUtils.cpp +++ b/src/util/ExpressionUtils.cpp @@ -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(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(aggArg); + if (*propExpr->prop() == "*") { + return Status::SemanticError("Could not apply aggregation function `%s' on `%s'", + aggExpr->toString().c_str(), + propExpr->toString().c_str()); } } diff --git a/src/validator/GroupByValidator.cpp b/src/validator/GroupByValidator.cpp index 2873c75f9..71ca619ae 100644 --- a/src/validator/GroupByValidator.cpp +++ b/src/validator/GroupByValidator.cpp @@ -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(colExpr); - NG_RETURN_IF_ERROR(checkAggExpr(aggExpr)); + NG_RETURN_IF_ERROR(ExpressionUtils::checkAggExpr(aggExpr)); } else { yieldCols_.emplace_back(colExpr); } @@ -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(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 baca02e15..f522f0694 100644 --- a/src/validator/GroupByValidator.h +++ b/src/validator/GroupByValidator.h @@ -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; @@ -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 yieldCols_; + std::vector yieldCols_; // key: alias, value: input name - std::unordered_map aliases_; + std::unordered_map aliases_; - bool needGenProject_{false}; - std::vector outputColumnNames_; - std::vector projOutputColumnNames_; + bool needGenProject_{false}; + std::vector outputColumnNames_; + std::vector projOutputColumnNames_; // used to generate Project node when there is an internally nested aggregateExpression - YieldColumns* projCols_; + YieldColumns *projCols_; - std::vector groupKeys_; - std::vector groupItems_; + std::vector groupKeys_; + std::vector groupItems_; }; - -} // namespace graph -} // namespace nebula +} // namespace graph +} // namespace nebula #endif diff --git a/src/validator/test/GroupByValidatorTest.cpp b/src/validator/test/GroupByValidatorTest.cpp index b39b83934..a0dfac6dc 100644 --- a/src/validator/test/GroupByValidatorTest.cpp +++ b/src/validator/test/GroupByValidatorTest.cpp @@ -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 @@ -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."); } { diff --git a/tests/tck/features/aggregate/Agg.feature b/tests/tck/features/aggregate/Agg.feature index f3c2bdf7c..c94ead78e 100644 --- a/tests/tck/features/aggregate/Agg.feature +++ b/tests/tck/features/aggregate/Agg.feature @@ -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 diff --git a/tests/tck/features/bugfix/MatchUsedInPipe.feature b/tests/tck/features/bugfix/MatchUsedInPipe.feature index 436360bbb..1661247dd 100644 --- a/tests/tck/features/bugfix/MatchUsedInPipe.feature +++ b/tests/tck/features/bugfix/MatchUsedInPipe.feature @@ -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 | diff --git a/tests/tck/features/yield/yield.feature b/tests/tck/features/yield/yield.feature index 82a68b5ce..ec5986310 100644 --- a/tests/tck/features/yield/yield.feature +++ b/tests/tck/features/yield/yield.feature @@ -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 |