Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[migrate #1314]fix the wrong usage of limit sentence #2570

Merged
merged 1 commit into from
Aug 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 39 additions & 16 deletions src/graph/validator/OrderByValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,53 @@

#include "graph/validator/OrderByValidator.h"

#include "common/expression/LabelExpression.h"
#include "graph/planner/plan/Query.h"
#include "parser/TraverseSentences.h"

namespace nebula {
namespace graph {
Status OrderByValidator::validateImpl() {
auto sentence = static_cast<OrderBySentence *>(sentence_);
outputs_ = inputCols();
auto &factors = sentence->factors();
auto *pool = qctx_->objPool();
for (auto &factor : factors) {
if (factor->expr()->kind() == Expression::Kind::kLabel) {
auto *label = static_cast<const LabelExpression *>(factor->expr());
auto *expr = InputPropertyExpression::make(pool, label->name());
factor->setExpr(expr);
}
if (factor->expr()->kind() != Expression::Kind::kInputProperty) {
if (factor->expr()->kind() == Expression::Kind::kInputProperty) {
auto expr = static_cast<InputPropertyExpression *>(factor->expr());
NG_RETURN_IF_ERROR(deduceExprType(expr));
NG_RETURN_IF_ERROR(deduceProps(expr, exprProps_));
const auto &cols = inputCols();
auto &name = expr->prop();
auto eq = [&](const ColDef &col) { return col.name == name; };
auto iter = std::find_if(cols.cbegin(), cols.cend(), eq);
size_t colIdx = std::distance(cols.cbegin(), iter);
colOrderTypes_.emplace_back(std::make_pair(colIdx, factor->orderType()));
} else if (factor->expr()->kind() == Expression::Kind::kVarProperty) {
auto expr = static_cast<VariablePropertyExpression *>(factor->expr());
NG_RETURN_IF_ERROR(deduceExprType(expr));
NG_RETURN_IF_ERROR(deduceProps(expr, exprProps_));
const auto &cols = vctx_->getVar(expr->sym());
auto &name = expr->prop();
auto eq = [&](const ColDef &col) { return col.name == name; };
auto iter = std::find_if(cols.cbegin(), cols.cend(), eq);
size_t colIdx = std::distance(cols.cbegin(), iter);
colOrderTypes_.emplace_back(std::make_pair(colIdx, factor->orderType()));
} else {
return Status::SemanticError("Order by with invalid expression `%s'",
factor->expr()->toString().c_str());
}
auto expr = static_cast<InputPropertyExpression *>(factor->expr());
NG_RETURN_IF_ERROR(deduceExprType(expr));
auto &name = expr->prop();
auto eq = [&](const ColDef &col) { return col.name == name; };
auto iter = std::find_if(outputs_.cbegin(), outputs_.cend(), eq);
size_t colIdx = std::distance(outputs_.cbegin(), iter);
colOrderTypes_.emplace_back(std::make_pair(colIdx, factor->orderType()));
}

if (!exprProps_.inputProps().empty() && !exprProps_.varProps().empty()) {
return Status::SemanticError("Not support both input and variable.");
} else if (!exprProps_.inputProps().empty()) {
outputs_ = inputCols();
} else if (!exprProps_.varProps().empty()) {
if (!userDefinedVarNameList_.empty()) {
if (userDefinedVarNameList_.size() != 1) {
return Status::SemanticError("Multiple user defined vars are not supported yet.");
}
userDefinedVarName_ = *userDefinedVarNameList_.begin();
outputs_ = vctx_->getVar(userDefinedVarName_);
}
}

return Status::OK();
Expand All @@ -47,6 +66,10 @@ Status OrderByValidator::toPlan() {
colNames.emplace_back(col.name);
}
sortNode->setColNames(std::move(colNames));
if (!userDefinedVarName_.empty()) {
sortNode->setInputVar(userDefinedVarName_);
}

root_ = sortNode;
tail_ = root_;
return Status::OK();
Expand Down
1 change: 1 addition & 0 deletions src/graph/validator/OrderByValidator.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class OrderByValidator final : public Validator {

private:
std::vector<std::pair<size_t, OrderFactor::OrderType>> colOrderTypes_;
std::string userDefinedVarName_;
};
} // namespace graph
} // namespace nebula
Expand Down
10 changes: 0 additions & 10 deletions src/graph/validator/SequentialValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,6 @@ Status SequentialValidator::validateImpl() {
}

DCHECK(!sentences.empty());
auto firstSentence = getFirstSentence(sentences.front());
switch (firstSentence->kind()) {
case Sentence::Kind::kLimit:
case Sentence::Kind::kOrderBy:
case Sentence::Kind::kGroupBy:
return Status::SyntaxError("Could not start with the statement: %s",
firstSentence->toString().c_str());
default:
break;
}

seqAstCtx_->startNode = StartNode::make(seqAstCtx_->qctx);
for (auto* sentence : sentences) {
Expand Down
2 changes: 1 addition & 1 deletion src/parser/parser.yy
Original file line number Diff line number Diff line change
Expand Up @@ -2544,7 +2544,6 @@ traverse_sentence
| order_by_sentence { $$ = $1; }
| fetch_sentence { $$ = $1; }
| find_path_sentence { $$ = $1; }
| limit_sentence { $$ = $1; }
| yield_sentence { $$ = $1; }
| get_subgraph_sentence { $$ = $1; }
| delete_vertex_sentence { $$ = $1; }
Expand All @@ -2557,6 +2556,7 @@ traverse_sentence
piped_sentence
: traverse_sentence { $$ = $1; }
| piped_sentence PIPE traverse_sentence { $$ = new PipedSentence($1, $3); }
| piped_sentence PIPE limit_sentence { $$ = new PipedSentence($1, $3); }
;

set_sentence
Expand Down
2 changes: 1 addition & 1 deletion tests/tck/features/fetch/FetchVertices.intVid.feature
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Feature: Fetch Int Vid Vertices
"""
$var = GO FROM hash('Boris Diaw') over like YIELD like._dst as id;
FETCH PROP ON player $var.id YIELD player.name as name, player.age |
ORDER BY name
ORDER BY $-.name
"""
Then the result should be, in order, and the columns 0 should be hashed:
| VertexID | name | player.age |
Expand Down
2 changes: 1 addition & 1 deletion tests/tck/features/fetch/FetchVertices.strVid.feature
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ Feature: Fetch String Vertices
"""
$var = GO FROM 'Boris Diaw' over like YIELD like._dst as id;
FETCH PROP ON player $var.id YIELD player.name as name, player.age |
ORDER BY name
ORDER BY $-.name
"""
Then the result should be, in order:
| VertexID | name | player.age |
Expand Down
12 changes: 12 additions & 0 deletions tests/tck/features/go/GroupbyLimit.feature
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,11 @@ Feature: Groupby & limit Sentence
| "Grizzlies" | 34 |
| "Raptors" | 34 |
| "Lakers" | 40 |
When executing query:
"""
GROUP BY 1 YIELD 1
"""
Then a SemanticError should be raised at runtime:

Scenario: Groupby with all agg functions
When executing query:
Expand Down Expand Up @@ -365,3 +370,10 @@ Feature: Groupby & limit Sentence
| GROUP BY $-.name YIELD $-.name AS name
"""
Then a SemanticError should be raised at runtime:

Scenario: Dangle limit
When executing query:
"""
YIELD 1; LIMIT 1;
"""
Then a SyntaxError should be raised at runtime:
44 changes: 42 additions & 2 deletions tests/tck/features/go/Orderby.feature
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Feature: Orderby Sentence
Background: Prepare space
Given a graph with space named "nba"

Scenario: Syntax Error
Scenario: Syntax Error or Semantic Error
When executing query:
"""
ORDER BY
Expand All @@ -22,7 +22,12 @@ Feature: Orderby Sentence
"""
ORDER BY $-.xx
"""
Then a SyntaxError should be raised at runtime:
Then a SemanticError should be raised at runtime: `$-.xx', not exist prop `xx'
When executing query:
"""
ORDER BY 1
"""
Then a SemanticError should be raised at runtime: Order by with invalid expression `1'

Scenario: Empty Input
When executing query:
Expand Down Expand Up @@ -165,6 +170,41 @@ Feature: Orderby Sentence
| "Spurs" |
| "Hornets" |

Scenario: Order by with Variable
When executing query:
"""
$var = GO FROM "Tony Parker" OVER like YIELD like._dst AS dst; ORDER BY $var.dst DESC
"""
Then the result should be, in order, with relax comparison:
| dst |
| "Tim Duncan" |
| "Manu Ginobili" |
| "LaMarcus Aldridge" |
When executing query:
"""
$var = GO FROM "Tony Parker" OVER like YIELD like._dst AS dst, like.likeness AS likeness; ORDER BY $var.dst DESC, $var.likeness
"""
Then the result should be, in order, with relax comparison:
| dst | likeness |
| "Tim Duncan" | 95 |
| "Manu Ginobili" | 95 |
| "LaMarcus Aldridge" | 90 |
When executing query:
"""
$var = GO FROM "Tony Parker" OVER like YIELD like._dst AS dst; ORDER BY $var.dst DESC | FETCH PROP ON * $-.dst
"""
Then the result should be, in order, with relax comparison:
| vertices_ |
| ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) |
| ("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"}) |
| ("LaMarcus Aldridge" :player{age: 33, name: "LaMarcus Aldridge"}) |
When executing query:
"""
$var = GO FROM "Tony Parker" OVER like YIELD like._dst AS dst;
GO FROM $var.dst OVER like YIELD like._dst AS id | ORDER BY $var.dst, $-.id;
"""
Then a SemanticError should be raised at runtime: Not support both input and variable.

Scenario: Duplicate columns
When executing query:
"""
Expand Down
4 changes: 2 additions & 2 deletions tests/tck/features/lookup/ByIndex.feature
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ Feature: Lookup by index itself
When executing query:
"""
LOOKUP ON player WHERE player.age < 40
YIELD player.age AS Age, player.name AS Name | order by Age DESC, Name| limit 10
YIELD player.age AS Age, player.name AS Name | order by $-.Age DESC, $-.Name| limit 10
"""
Then the result should be, in order, with relax comparison:
| VertexID | Age | Name |
Expand All @@ -535,7 +535,7 @@ Feature: Lookup by index itself
When executing query:
"""
LOOKUP ON player WHERE player.age <= 40
YIELD player.age AS Age, player.name AS Name | order by Age DESC, Name| limit 10
YIELD player.age AS Age, player.name AS Name | order by $-.Age DESC, $-.Name| limit 10
"""
Then the result should be, in order, with relax comparison:
| VertexID | Age | Name |
Expand Down
4 changes: 2 additions & 2 deletions tests/tck/features/lookup/ByIndex.intVid.feature
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ Feature: Lookup by index itself in integer vid
When executing query:
"""
LOOKUP ON player WHERE player.age < 40
YIELD player.age AS Age, player.name AS Name | order by Age DESC, Name| limit 10
YIELD player.age AS Age, player.name AS Name | order by $-.Age DESC, $-.Name| limit 10
"""
Then the result should be, in order, with relax comparison, and the columns 0 should be hashed:
| VertexID | Age | Name |
Expand All @@ -535,7 +535,7 @@ Feature: Lookup by index itself in integer vid
When executing query:
"""
LOOKUP ON player WHERE player.age <= 40
YIELD player.age AS Age, player.name AS Name | order by Age DESC, Name| limit 10
YIELD player.age AS Age, player.name AS Name | order by $-.Age DESC, $-.Name| limit 10
"""
Then the result should be, in order, with relax comparison, and the columns 0 should be hashed:
| VertexID | Age | Name |
Expand Down
8 changes: 4 additions & 4 deletions tests/tck/features/optimizer/TopNRule.feature
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Feature: TopN rule
"""
GO 1 STEPS FROM "Marco Belinelli" OVER like
YIELD like.likeness AS likeness |
ORDER BY likeness |
ORDER BY $-.likeness |
LIMIT 2
"""
Then the result should be, in order:
Expand All @@ -32,7 +32,7 @@ Feature: TopN rule
"""
GO 1 STEPS FROM "Marco Belinelli" OVER like REVERSELY
YIELD like.likeness AS likeness |
ORDER BY likeness |
ORDER BY $-.likeness |
LIMIT 1
"""
Then the result should be, in order:
Expand All @@ -51,7 +51,7 @@ Feature: TopN rule
"""
GO 1 STEPS FROM "Marco Belinelli" OVER like
YIELD like.likeness as likeness |
ORDER BY likeness |
ORDER BY $-.likeness |
LIMIT 2, 3
"""
Then the result should be, in order:
Expand All @@ -71,7 +71,7 @@ Feature: TopN rule
"""
GO 1 STEPS FROM "Marco Belinelli" OVER like
YIELD like.likeness AS likeness |
ORDER BY likeness
ORDER BY $-.likeness
"""
Then the result should be, in any order:
| likeness |
Expand Down