diff --git a/src/graph/context/ast/QueryAstContext.h b/src/graph/context/ast/QueryAstContext.h index b4fb5596fbc..df6f7049529 100644 --- a/src/graph/context/ast/QueryAstContext.h +++ b/src/graph/context/ast/QueryAstContext.h @@ -113,7 +113,6 @@ struct LookupContext final : public AstContext { bool dedup{false}; bool isEmptyResultSet{false}; int32_t schemaId{-1}; - int64_t limit{-1}; Expression* filter{nullptr}; YieldColumns* yieldExpr{nullptr}; std::vector idxReturnCols; diff --git a/src/graph/planner/ngql/LookupPlanner.cpp b/src/graph/planner/ngql/LookupPlanner.cpp index 551945a4097..3a51e6eb570 100644 --- a/src/graph/planner/ngql/LookupPlanner.cpp +++ b/src/graph/planner/ngql/LookupPlanner.cpp @@ -38,10 +38,8 @@ StatusOr LookupPlanner::transform(AstContext* astCtx) { lookupCtx->idxReturnCols, lookupCtx->schemaId, lookupCtx->isEmptyResultSet); - if (lookupCtx->limit >= 0) { - edgeIndexFullScan->setLimit(lookupCtx->limit); - } plan.tail = edgeIndexFullScan; + plan.root = edgeIndexFullScan; } else { auto* tagIndexFullScan = TagIndexFullScan::make(qctx, nullptr, @@ -51,15 +49,11 @@ StatusOr LookupPlanner::transform(AstContext* astCtx) { lookupCtx->idxReturnCols, lookupCtx->schemaId, lookupCtx->isEmptyResultSet); - if (lookupCtx->limit >= 0) { - tagIndexFullScan->setLimit(lookupCtx->limit); - } plan.tail = tagIndexFullScan; + plan.root = tagIndexFullScan; } plan.tail->setColNames(lookupCtx->idxColNames); - plan.root = plan.tail; - if (lookupCtx->filter) { plan.root = Filter::make(qctx, plan.root, lookupCtx->filter); } diff --git a/src/graph/validator/LookupValidator.cpp b/src/graph/validator/LookupValidator.cpp index 45ebd91330d..a17f83a9807 100644 --- a/src/graph/validator/LookupValidator.cpp +++ b/src/graph/validator/LookupValidator.cpp @@ -44,7 +44,6 @@ Status LookupValidator::validateImpl() { NG_RETURN_IF_ERROR(validateFrom()); NG_RETURN_IF_ERROR(validateFilter()); NG_RETURN_IF_ERROR(validateYield()); - NG_RETURN_IF_ERROR(validateLimit()); return Status::OK(); } @@ -230,18 +229,6 @@ Status LookupValidator::validateFilter() { return Status::OK(); } -Status LookupValidator::validateLimit() { - auto* limitClause = sentence()->limitClause(); - if (limitClause == nullptr) { - return Status::OK(); - } - if (limitClause->limit() < 0) { - return Status::SemanticError("Invalid negative limit number %ld.", limitClause->limit()); - } - lookupCtx_->limit = limitClause->limit(); - return Status::OK(); -} - StatusOr LookupValidator::handleLogicalExprOperands(LogicalExpression* lExpr) { auto& operands = lExpr->operands(); for (auto i = 0u; i < operands.size(); i++) { diff --git a/src/graph/validator/LookupValidator.h b/src/graph/validator/LookupValidator.h index 5a382b7b05e..9d0178ad35c 100644 --- a/src/graph/validator/LookupValidator.h +++ b/src/graph/validator/LookupValidator.h @@ -33,7 +33,6 @@ class LookupValidator final : public Validator { Status validateFilter(); Status validateYieldTag(); Status validateYieldEdge(); - Status validateLimit(); StatusOr checkFilter(Expression* expr); Status checkRelExpr(RelationalExpression* expr); diff --git a/src/parser/Clauses.h b/src/parser/Clauses.h index b769619f00f..23e0350b3ef 100644 --- a/src/parser/Clauses.h +++ b/src/parser/Clauses.h @@ -382,21 +382,5 @@ class NameLabelList { std::vector> labels_; }; -class LimitClause { - public: - explicit LimitClause(std::size_t limit) : limit_(limit) {} - - int64_t limit() const { return limit_; } - - std::string toString() const { - std::stringstream ss; - ss << "LIMIT " << limit_; - return ss.str(); - } - - private: - int64_t limit_{0}; -}; - } // namespace nebula #endif // PARSER_CLAUSES_H_ diff --git a/src/parser/TraverseSentences.cpp b/src/parser/TraverseSentences.cpp index 034e94e2cfa..0862d7ae54f 100644 --- a/src/parser/TraverseSentences.cpp +++ b/src/parser/TraverseSentences.cpp @@ -40,15 +40,11 @@ std::string GoSentence::toString() const { return buf; } -LookupSentence::LookupSentence(std::string *from, - WhereClause *where, - YieldClause *yield, - LimitClause *limit) +LookupSentence::LookupSentence(std::string *from, WhereClause *where, YieldClause *yield) : Sentence(Kind::kLookup), from_(DCHECK_NOTNULL(from)), whereClause_(where), - yieldClause_(yield), - limitClause_(limit) {} + yieldClause_(yield) {} std::string LookupSentence::toString() const { std::string buf; @@ -63,10 +59,6 @@ std::string LookupSentence::toString() const { buf += " "; buf += yieldClause_->toString(); } - if (limitClause_ != nullptr) { - buf += " "; - buf += limitClause_->toString(); - } return buf; } diff --git a/src/parser/TraverseSentences.h b/src/parser/TraverseSentences.h index 3156f16e4d3..2829d82def7 100644 --- a/src/parser/TraverseSentences.h +++ b/src/parser/TraverseSentences.h @@ -61,7 +61,7 @@ class GoSentence final : public Sentence { class LookupSentence final : public Sentence { public: - LookupSentence(std::string* from, WhereClause* where, YieldClause* yield, LimitClause* limit); + LookupSentence(std::string* from, WhereClause* where, YieldClause* yield); const std::string& from() const { return *from_; } @@ -69,15 +69,12 @@ class LookupSentence final : public Sentence { const YieldClause* yieldClause() const { return yieldClause_.get(); } - const LimitClause* limitClause() const { return limitClause_.get(); } - std::string toString() const override; private: std::unique_ptr from_; std::unique_ptr whereClause_; std::unique_ptr yieldClause_; - std::unique_ptr limitClause_; }; class UseSentence final : public Sentence { diff --git a/src/parser/parser.yy b/src/parser/parser.yy index 8c56bec6a34..6588ed35041 100644 --- a/src/parser/parser.yy +++ b/src/parser/parser.yy @@ -148,7 +148,6 @@ static constexpr size_t kCommentLengthLimit = 256; nebula::meta::cpp2::FTClient *text_search_client_item; nebula::TSClientList *text_search_client_list; nebula::QueryUniqueIdentifier *query_unique_identifier; - nebula::LimitClause *limit_clause; } /* destructors */ @@ -343,8 +342,6 @@ static constexpr size_t kCommentLengthLimit = 256; %type query_unique_identifier -%type limit_clause - %type maintain_sentence %type create_space_sentence describe_space_sentence drop_space_sentence %type create_tag_sentence create_edge_sentence @@ -1943,14 +1940,9 @@ lookup_where_clause | KW_WHERE expression { $$ = new WhereClause($2); } ; -limit_clause - : %empty { $$ = nullptr; } - | KW_LIMIT legal_integer { $$ = new LimitClause($2); } - ; - lookup_sentence - : KW_LOOKUP KW_ON name_label lookup_where_clause yield_clause limit_clause { - $$ = new LookupSentence($3, $4, $5, $6); + : KW_LOOKUP KW_ON name_label lookup_where_clause yield_clause { + $$ = new LookupSentence($3, $4, $5); } ; diff --git a/src/parser/test/ParserTest.cpp b/src/parser/test/ParserTest.cpp index 4d647db9960..87b2bcd7240 100644 --- a/src/parser/test/ParserTest.cpp +++ b/src/parser/test/ParserTest.cpp @@ -1370,20 +1370,6 @@ TEST_F(ParserTest, Lookup) { auto result = parse(query); ASSERT_TRUE(result.ok()) << result.status(); } - { - std::string query = - "LOOKUP ON transfer WHERE transfer.amount > 1000 YIELD transfer.amount," - " transfer.test LIMIT 1"; - auto result = parse(query); - EXPECT_TRUE(result.ok()) << result.status(); - } - { - std::string query = - "LOOKUP ON transfer WHERE transfer.amount > 1000 YIELD transfer.amount," - " transfer.test LIMIT -1"; - auto result = parse(query); - EXPECT_FALSE(result.ok()); - } } TEST_F(ParserTest, subgraph) { diff --git a/tests/tck/features/lookup/LookUpLimit.feature b/tests/tck/features/lookup/LookUpLimit.feature index 49771cd5657..d22f0a55285 100644 --- a/tests/tck/features/lookup/LookUpLimit.feature +++ b/tests/tck/features/lookup/LookUpLimit.feature @@ -10,166 +10,135 @@ Feature: Push Limit down IndexScan Rule Scenario: push limit down to IndexScan When profiling query: """ - LOOKUP ON player Limit 2 | ORDER BY $-.VertexID + LOOKUP ON player | Limit 2 | ORDER BY $-.VertexID """ Then the result should be, in any order: - | VertexID | - | "Amar'e Stoudemire" | - | "Aron Baynes" | - | "Ben Simmons" | - | "Blake Griffin" | - | "Boris Diaw" | - | "Carmelo Anthony" | - | "Chris Paul" | - | "Cory Joseph" | - | "Damian Lillard" | - | "DeAndre Jordan" | - | "Dwyane Wade" | - | "JaVale McGee" | - | "Klay Thompson" | - | "Luka Doncic" | + | VertexID | + | /[a-zA-Z ']+/ | + | /[a-zA-Z ']+/ | And the execution plan should be: | id | name | dependencies | operator info | | 4 | DataCollect | 5 | | | 5 | Sort | 6 | | | 6 | Project | 7 | | - | 7 | TagIndexFullScan | 0 | {"limit": "2"} | + | 7 | Limit | 8 | {"count": "2"} | + | 8 | TagIndexFullScan | 0 | {"limit": "2"} | | 0 | Start | | | When profiling query: """ - LOOKUP ON like Limit 2 | ORDER BY $-.SrcVID + LOOKUP ON like | Limit 2 | ORDER BY $-.SrcVID """ Then the result should be, in any order: - | SrcVID | DstVID | Ranking | - | "Ben Simmons" | "Joel Embiid" | 0 | - | "Blake Griffin" | "Chris Paul" | 0 | - | "Damian Lillard" | "LaMarcus Aldridge" | 0 | - | "Dirk Nowitzki" | "Dwyane Wade" | 0 | - | "Jason Kidd" | "Vince Carter" | 0 | - | "Klay Thompson" | "Stephen Curry" | 0 | - | "Kyrie Irving" | "LeBron James" | 0 | - | "LaMarcus Aldridge" | "Tim Duncan" | 0 | - | "LaMarcus Aldridge" | "Tony Parker" | 0 | - | "Marco Belinelli" | "Tim Duncan" | 0 | - | "Marco Belinelli" | "Tony Parker" | 0 | - | "Rajon Rondo" | "Ray Allen" | 0 | - | "Ray Allen" | "Rajon Rondo" | 0 | - | "Rudy Gay" | "LaMarcus Aldridge" | 0 | + | SrcVID | DstVID | Ranking | + | /[a-zA-Z ']+/ | /[a-zA-Z ']+/ | /\d+/ | + | /[a-zA-Z ']+/ | /[a-zA-Z ']+/ | /\d+/ | And the execution plan should be: | id | name | dependencies | operator info | | 4 | DataCollect | 5 | | | 5 | Sort | 6 | | | 6 | Project | 7 | | - | 7 | EdgeIndexFullScan | 0 | {"limit": "2"} | + | 7 | Limit | 8 | {"count": "2"} | + | 8 | EdgeIndexFullScan | 0 | {"limit": "2"} | | 0 | Start | | | When profiling query: """ - LOOKUP ON player WHERE player.age == 33 Limit 2 | ORDER BY $-.VertexID + LOOKUP ON player WHERE player.age == 33 | Limit 2 | ORDER BY $-.VertexID """ Then the result should be, in any order: - | VertexID | - | "Chris Paul" | - | "Dwight Howard" | - | "LaMarcus Aldridge" | - | "Rajon Rondo" | + | VertexID | + | /[a-zA-Z ']+/ | + | /[a-zA-Z ']+/ | And the execution plan should be: | id | name | dependencies | operator info | | 4 | DataCollect | 5 | | | 5 | Sort | 6 | | | 6 | Project | 7 | | - | 7 | TagIndexPrefixScan | 0 | {"limit": "2"} | + | 7 | Limit | 8 | {"count": "2"} | + | 8 | TagIndexPrefixScan | 0 | {"limit": "2"} | | 0 | Start | | | When profiling query: """ - LOOKUP ON like WHERE like.likeness == 90 Limit 2 | ORDER BY $-.SrcVID + LOOKUP ON like WHERE like.likeness == 90 | Limit 2 | ORDER BY $-.SrcVID """ Then the result should be, in any order: - | SrcVID | DstVID | Ranking | - | "Amar'e Stoudemire" | "Steve Nash" | 0 | - | "Carmelo Anthony" | "Chris Paul" | 0 | - | "Chris Paul" | "Carmelo Anthony" | 0 | - | "Chris Paul" | "Dwyane Wade" | 0 | - | "Dwyane Wade" | "Carmelo Anthony" | 0 | - | "Dwyane Wade" | "Chris Paul" | 0 | - | "Jason Kidd" | "Steve Nash" | 0 | - | "Klay Thompson" | "Stephen Curry" | 0 | - | "Luka Doncic" | "Dirk Nowitzki" | 0 | - | "Luka Doncic" | "Kristaps Porzingis" | 0 | - | "Paul Gasol" | "Kobe Bryant" | 0 | + | SrcVID | DstVID | Ranking | + | /[a-zA-Z ']+/ | /[a-zA-Z ']+/ | /\d+/ | + | /[a-zA-Z ']+/ | /[a-zA-Z ']+/ | /\d+/ | And the execution plan should be: | id | name | dependencies | operator info | | 4 | DataCollect | 5 | | | 5 | Sort | 6 | | | 6 | Project | 7 | | - | 7 | EdgeIndexPrefixScan | 0 | {"limit": "2"} | + | 7 | Limit | 8 | {"count": "2"} | + | 8 | EdgeIndexPrefixScan | 0 | {"limit": "2"} | | 0 | Start | | | Scenario: push limit down to IndexScan with limit When profiling query: """ - LOOKUP ON player Limit 3 | LIMIT 3 | ORDER BY $-.VertexID + LOOKUP ON player | LIMIT 3 | ORDER BY $-.VertexID """ Then the result should be, in any order: - | VertexID | - | "Amar'e Stoudemire" | - | "Aron Baynes" | - | "Ben Simmons" | + | VertexID | + | /[a-zA-Z ']+/ | + | /[a-zA-Z ']+/ | + | /[a-zA-Z ']+/ | And the execution plan should be: | id | name | dependencies | operator info | | 3 | DataCollect | 4 | | | 4 | Sort | 5 | | - | 5 | Project | 6 | | - | 6 | Limit | 7 | | - | 7 | TagIndexFullScan | 8 | {"limit": "3"} | - | 8 | Start | | | + | 5 | Project | 7 | | + | 7 | Limit | 8 | | + | 8 | TagIndexFullScan | 9 | {"limit": "3"} | + | 9 | Start | | | When profiling query: """ - LOOKUP ON like Limit 3 | LIMIT 3 | ORDER BY $-.SrcVID + LOOKUP ON like | LIMIT 3 | ORDER BY $-.SrcVID """ Then the result should be, in any order: - | SrcVID | DstVID | Ranking | - | "Aron Baynes" | "Tim Duncan" | 0 | - | "Ben Simmons" | "Joel Embiid" | 0 | - | "Blake Griffin" | "Chris Paul" | 0 | + | SrcVID | DstVID | Ranking | + | /[a-zA-Z ']+/ | /[a-zA-Z ']+/ | /\d+/ | + | /[a-zA-Z ']+/ | /[a-zA-Z ']+/ | /\d+/ | + | /[a-zA-Z ']+/ | /[a-zA-Z ']+/ | /\d+/ | And the execution plan should be: | id | name | dependencies | operator info | | 3 | DataCollect | 4 | | | 4 | Sort | 5 | | - | 5 | Project | 6 | | - | 6 | Limit | 7 | | - | 7 | EdgeIndexFullScan | 8 | {"limit": "3"} | - | 8 | Start | | | + | 5 | Project | 7 | | + | 7 | Limit | 8 | | + | 8 | EdgeIndexFullScan | 9 | {"limit": "3"} | + | 9 | Start | | | When profiling query: """ - LOOKUP ON player WHERE player.age == 33 Limit 3 | LIMIT 3 | ORDER BY $-.VertexID + LOOKUP ON player WHERE player.age == 33 | LIMIT 3 | ORDER BY $-.VertexID """ Then the result should be, in any order: - | VertexID | - | "Chris Paul" | - | "Dwight Howard" | - | "LaMarcus Aldridge" | + | VertexID | + | /[a-zA-Z ']+/ | + | /[a-zA-Z ']+/ | + | /[a-zA-Z ']+/ | And the execution plan should be: | id | name | dependencies | operator info | | 3 | DataCollect | 4 | | | 4 | Sort | 5 | | - | 5 | Project | 6 | | - | 6 | Limit | 7 | | - | 7 | TagIndexPrefixScan | 8 | {"limit": "3"} | - | 8 | Start | | | + | 5 | Project | 7 | | + | 7 | Limit | 8 | | + | 8 | TagIndexPrefixScan | 9 | {"limit": "3"} | + | 9 | Start | | | When profiling query: """ - LOOKUP ON like WHERE like.likeness == 90 Limit 3 | LIMIT 3 | ORDER BY $-.SrcVID + LOOKUP ON like WHERE like.likeness == 90 | LIMIT 3 | ORDER BY $-.SrcVID """ Then the result should be, in any order: - | SrcVID | DstVID | Ranking | - | "Amar'e Stoudemire" | "Steve Nash" | 0 | - | "Carmelo Anthony" | "Chris Paul" | 0 | - | "Carmelo Anthony" | "Dwyane Wade" | 0 | + | SrcVID | DstVID | Ranking | + | /[a-zA-Z ']+/ | /[a-zA-Z ']+/ | /\d+/ | + | /[a-zA-Z ']+/ | /[a-zA-Z ']+/ | /\d+/ | + | /[a-zA-Z ']+/ | /[a-zA-Z ']+/ | /\d+/ | And the execution plan should be: | id | name | dependencies | operator info | | 3 | DataCollect | 4 | | | 4 | Sort | 5 | | - | 5 | Project | 6 | | - | 6 | Limit | 7 | | - | 7 | EdgeIndexPrefixScan | 8 | {"limit": "3"} | - | 8 | Start | | | + | 5 | Project | 7 | | + | 7 | Limit | 8 | | + | 8 | EdgeIndexPrefixScan | 9 | {"limit": "3"} | + | 9 | Start | | |