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

fix filter degradation #1240

Merged
merged 5 commits into from
Jul 13, 2021
Merged

fix filter degradation #1240

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9995caf
fix filter degradation
czpmango Jul 12, 2021
1600e23
Merge branch 'master' into fix-filter-degradation
czpmango Jul 12, 2021
033faa3
Merge branch 'master' into fix-filter-degradation
Aiee Jul 13, 2021
cf658ad
Merge branch 'master' into fix-filter-degradation
czpmango Jul 13, 2021
b630bfd
Merge branch 'master' into fix-filter-degradation
yixinglu Jul 13, 2021
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/planner/match/WhereClausePlanner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ StatusOr<SubPlan> WhereClausePlanner::transform(CypherClauseContextBase* ctx) {
if (wctx->filter) {
SubPlan wherePlan;
auto* newFilter = MatchSolver::doRewrite(wctx->qctx, *wctx->aliasesUsed, wctx->filter);
wherePlan.root = Filter::make(wctx->qctx, nullptr, newFilter, true);
wherePlan.root = Filter::make(wctx->qctx, nullptr, newFilter, needStableFilter_);
wherePlan.tail = wherePlan.root;

return wherePlan;
Expand Down
7 changes: 6 additions & 1 deletion src/planner/match/WhereClausePlanner.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,14 @@ namespace graph {
*/
class WhereClausePlanner final : public CypherClausePlanner {
public:
WhereClausePlanner() = default;
explicit WhereClausePlanner(bool needStableFilter = false)
: needStableFilter_(needStableFilter) {}

StatusOr<SubPlan> transform(CypherClauseContextBase* clauseCtx) override;

private:
// `needStableFilter_=true` only if there is orderBy in withClause(to avoid unstableErase)
bool needStableFilter_{false};
};
} // namespace graph
} // namespace nebula
Expand Down
4 changes: 3 additions & 1 deletion src/planner/match/WithClausePlanner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ Status WithClausePlanner::buildWith(WithClauseContext* wctx, SubPlan& subPlan) {
}

if (wctx->where != nullptr) {
auto wherePlan = std::make_unique<WhereClausePlanner>()->transform(wctx->where.get());
czpmango marked this conversation as resolved.
Show resolved Hide resolved
bool needStableFilter = wctx->order != nullptr;
auto wherePlan =
std::make_unique<WhereClausePlanner>(needStableFilter)->transform(wctx->where.get());
NG_RETURN_IF_ERROR(wherePlan);
auto plan = std::move(wherePlan).value();
SegmentsConnector::addInput(plan.tail, subPlan.root, true);
Expand Down
1 change: 1 addition & 0 deletions src/planner/plan/Query.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ Filter::Filter(QueryContext* qctx, PlanNode* input, Expression* condition, bool
std::unique_ptr<PlanNodeDescription> Filter::explain() const {
auto desc = SingleInputNode::explain();
addDescription("condition", condition_ ? condition_->toString() : "", desc.get());
addDescription("isStable", needStableFilter_ ? "true" : "false", desc.get());
return desc;
}

Expand Down
81 changes: 46 additions & 35 deletions tests/tck/features/match/With.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ Feature: With clause
RETURN count(names)
"""
Then the result should be, in any order, with relax comparison:
| COUNT(names) |
| count(names) |
| 191 |
When executing query:
"""
Expand All @@ -82,8 +82,52 @@ Feature: With clause
RETURN collect(names)
"""
Then the result should be, in any order, with relax comparison:
| COLLECT(names) |
| collect(names) |
| ["Tony Parker", "Tiago Splitter", "Spurs", "Shaquile O'Neal", "Marco Belinelli"] |
When profiling query:
"""
MATCH (v:player)
WITH v.age AS age, v AS v, v.name AS name
ORDER BY age DESCENDING, name ASCENDING
LIMIT 20
WHERE age > 30
RETURN v, age
"""
Then the result should be, in order, with relax comparison:
| v | age |
| ("Shaquile O'Neal" :player{age: 47, name: "Shaquile O'Neal"}) | 47 |
| ("Grant Hill" :player{age: 46, name: "Grant Hill"}) | 46 |
| ("Jason Kidd" :player{age: 45, name: "Jason Kidd"}) | 45 |
| ("Steve Nash" :player{age: 45, name: "Steve Nash"}) | 45 |
| ("Ray Allen" :player{age: 43, name: "Ray Allen"}) | 43 |
| ("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"}) | 42 |
| ("Vince Carter" :player{age: 42, name: "Vince Carter"}) | 42 |
| ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) | 41 |
| ("Dirk Nowitzki" :player{age: 40, name: "Dirk Nowitzki"}) | 40 |
| ("Kobe Bryant" :player{age: 40, name: "Kobe Bryant"}) | 40 |
| ("Tracy McGrady" :player{age: 39, name: "Tracy McGrady"}) | 39 |
| ("David West" :player{age: 38, name: "David West"}) | 38 |
| ("Paul Gasol" :player{age: 38, name: "Paul Gasol"}) | 38 |
| ("Yao Ming" :player{age: 38, name: "Yao Ming"}) | 38 |
| ("Dwyane Wade" :player{age: 37, name: "Dwyane Wade"}) | 37 |
| ("Amar'e Stoudemire" :player{age: 36, name: "Amar'e Stoudemire"}) | 36 |
| ("Boris Diaw" :player{age: 36, name: "Boris Diaw"}) | 36 |
| ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | 36 |
| ("Carmelo Anthony" :player{age: 34, name: "Carmelo Anthony"}) | 34 |
| ("LeBron James" :player{age: 34, name: "LeBron James"}) | 34 |
And the execution plan should be:
| id | name | dependencies | operator info |
| 13 | Project | 12 | |
| 12 | Filter | 17 | {"isStable": "true"} |
| 17 | TopN | 9 | |
| 9 | Project | 8 | |
| 8 | Filter | 7 | {"isStable": "false"} |
| 7 | Project | 6 | |
| 6 | Project | 5 | |
| 5 | Filter | 16 | {"isStable": "false"} |
| 16 | GetVertices | 1 | |
| 1 | IndexScan | 0 | |
| 0 | Start | | |

@skip
Scenario: with match return
Expand Down Expand Up @@ -122,39 +166,6 @@ Feature: With clause
| "hello" | 2 | 2 |
| "hello" | 3 | 3 |

Scenario: match with return
When executing query:
"""
MATCH (v:player)
WITH v.age AS age, v AS v, v.name AS name
ORDER BY age DESCENDING, name ASCENDING
LIMIT 20
WHERE age > 30
RETURN v, age
"""
Then the result should be, in order, with relax comparison:
| v | age |
| ("Shaquile O'Neal" :player{age: 47, name: "Shaquile O'Neal"}) | 47 |
| ("Grant Hill" :player{age: 46, name: "Grant Hill"}) | 46 |
| ("Jason Kidd" :player{age: 45, name: "Jason Kidd"}) | 45 |
| ("Steve Nash" :player{age: 45, name: "Steve Nash"}) | 45 |
| ("Ray Allen" :player{age: 43, name: "Ray Allen"}) | 43 |
| ("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"}) | 42 |
| ("Vince Carter" :player{age: 42, name: "Vince Carter"}) | 42 |
| ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) | 41 |
| ("Dirk Nowitzki" :player{age: 40, name: "Dirk Nowitzki"}) | 40 |
| ("Kobe Bryant" :player{age: 40, name: "Kobe Bryant"}) | 40 |
| ("Tracy McGrady" :player{age: 39, name: "Tracy McGrady"}) | 39 |
| ("David West" :player{age: 38, name: "David West"}) | 38 |
| ("Paul Gasol" :player{age: 38, name: "Paul Gasol"}) | 38 |
| ("Yao Ming" :player{age: 38, name: "Yao Ming"}) | 38 |
| ("Dwyane Wade" :player{age: 37, name: "Dwyane Wade"}) | 37 |
| ("Amar'e Stoudemire" :player{age: 36, name: "Amar'e Stoudemire"}) | 36 |
| ("Boris Diaw" :player{age: 36, name: "Boris Diaw"}) | 36 |
| ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | 36 |
| ("Carmelo Anthony" :player{age: 34, name: "Carmelo Anthony"}) | 34 |
| ("LeBron James" :player{age: 34, name: "LeBron James"}) | 34 |

Scenario: with exists
When executing query:
"""
Expand Down