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

Fix the lookup limit result. #3171

Merged
merged 4 commits into from
Oct 21, 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
1 change: 0 additions & 1 deletion src/graph/context/ast/QueryAstContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> idxReturnCols;
Expand Down
10 changes: 2 additions & 8 deletions src/graph/planner/ngql/LookupPlanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,8 @@ StatusOr<SubPlan> 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,
Expand All @@ -51,15 +49,11 @@ StatusOr<SubPlan> 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);
}
Expand Down
13 changes: 0 additions & 13 deletions src/graph/validator/LookupValidator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down Expand Up @@ -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<Expression*> LookupValidator::handleLogicalExprOperands(LogicalExpression* lExpr) {
auto& operands = lExpr->operands();
for (auto i = 0u; i < operands.size(); i++) {
Expand Down
1 change: 0 additions & 1 deletion src/graph/validator/LookupValidator.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ class LookupValidator final : public Validator {
Status validateFilter();
Status validateYieldTag();
Status validateYieldEdge();
Status validateLimit();

StatusOr<Expression*> checkFilter(Expression* expr);
Status checkRelExpr(RelationalExpression* expr);
Expand Down
16 changes: 0 additions & 16 deletions src/parser/Clauses.h
Original file line number Diff line number Diff line change
Expand Up @@ -382,21 +382,5 @@ class NameLabelList {
std::vector<std::unique_ptr<std::string>> 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_
12 changes: 2 additions & 10 deletions src/parser/TraverseSentences.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -63,10 +59,6 @@ std::string LookupSentence::toString() const {
buf += " ";
buf += yieldClause_->toString();
}
if (limitClause_ != nullptr) {
buf += " ";
buf += limitClause_->toString();
}
return buf;
}

Expand Down
5 changes: 1 addition & 4 deletions src/parser/TraverseSentences.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,23 +61,20 @@ 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_; }

const WhereClause* whereClause() const { return whereClause_.get(); }

const YieldClause* yieldClause() const { return yieldClause_.get(); }

const LimitClause* limitClause() const { return limitClause_.get(); }

std::string toString() const override;

private:
std::unique_ptr<std::string> from_;
std::unique_ptr<WhereClause> whereClause_;
std::unique_ptr<YieldClause> yieldClause_;
std::unique_ptr<LimitClause> limitClause_;
};

class UseSentence final : public Sentence {
Expand Down
12 changes: 2 additions & 10 deletions src/parser/parser.yy
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -343,8 +342,6 @@ static constexpr size_t kCommentLengthLimit = 256;

%type <query_unique_identifier> query_unique_identifier

%type <limit_clause> limit_clause

%type <sentence> maintain_sentence
%type <sentence> create_space_sentence describe_space_sentence drop_space_sentence
%type <sentence> create_tag_sentence create_edge_sentence
Expand Down Expand Up @@ -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);
}
;

Expand Down
14 changes: 0 additions & 14 deletions src/parser/test/ParserTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading