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

[cherry-pick #1222]fix the wrong usage of limit sentence #1314

Merged
merged 4 commits into from
Aug 11, 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
2 changes: 1 addition & 1 deletion src/parser/parser.yy
Original file line number Diff line number Diff line change
Expand Up @@ -2537,7 +2537,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 @@ -2549,6 +2548,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
55 changes: 40 additions & 15 deletions src/validator/OrderByValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,47 @@ 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 @@ -46,6 +67,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/validator/OrderByValidator.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,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/validator/SequentialValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,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 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
5 changes: 5 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
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 @@ -528,7 +528,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 @@ -545,7 +545,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 @@ -528,7 +528,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 @@ -545,7 +545,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