From efcacb6a6080fc858409ec0d1ec88fa5f4d1e18c Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Wed, 7 Dec 2022 16:14:19 +0800 Subject: [PATCH 1/3] fix push efilter down --- .../optimizer/rule/PushEFilterDownRule.cpp | 2 +- .../optimizer/PushEFilterDownRule.feature | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/graph/optimizer/rule/PushEFilterDownRule.cpp b/src/graph/optimizer/rule/PushEFilterDownRule.cpp index 3b5df8904d2..b49ccfb5d1d 100644 --- a/src/graph/optimizer/rule/PushEFilterDownRule.cpp +++ b/src/graph/optimizer/rule/PushEFilterDownRule.cpp @@ -144,7 +144,7 @@ std::string PushEFilterDownRule::toString() const { const storage::cpp2::EdgeProp &edge, meta::SchemaManager *schemaMng, ObjectPool *pool) { - auto edgeNameResult = schemaMng->toEdgeName(spaceId, edge.get_type()); + auto edgeNameResult = schemaMng->toEdgeName(spaceId, std::abs(edge.get_type())); if (!edgeNameResult.ok()) { return nullptr; } diff --git a/tests/tck/features/optimizer/PushEFilterDownRule.feature b/tests/tck/features/optimizer/PushEFilterDownRule.feature index 96637782021..747e95fa6b8 100644 --- a/tests/tck/features/optimizer/PushEFilterDownRule.feature +++ b/tests/tck/features/optimizer/PushEFilterDownRule.feature @@ -25,6 +25,34 @@ Feature: Push EFilter down rule | 8 | Traverse | 7 | {"edge filter": "", "filter": "(like.likeness==95)"} | | 7 | IndexScan | 0 | | | 0 | Start | | | + When profiling query: + """ + MATCH (v:player{name: 'Tim Duncan'})<-[e:like{likeness: 95}]-() return v.player.name AS name + """ + Then the result should be, in any order: + | name | + | "Tim Duncan" | + | "Tim Duncan" | + And the execution plan should be: + | id | name | dependencies | operator info | + | 5 | Project | 8 | | + | 8 | Traverse | 7 | {"edge filter": "", "filter": "(like.likeness==95)"} | + | 7 | IndexScan | 0 | | + | 0 | Start | | | + When profiling query: + """ + MATCH (v:player{name: 'Tim Duncan'})-[e:like{likeness: 95}]-() return v.player.name AS name + """ + Then the result should be, in any order: + | name | + | "Tim Duncan" | + | "Tim Duncan" | + And the execution plan should be: + | id | name | dependencies | operator info | + | 5 | Project | 8 | | + | 8 | Traverse | 7 | {"edge filter": "", "filter": "(like.likeness==95)"} | + | 7 | IndexScan | 0 | | + | 0 | Start | | | When profiling query: """ MATCH (v:player{name: 'Tim Duncan'})-[e:like*1..2{likeness: 95}]->() return v.player.name AS name From 11b6a55c146b367194c3cf06603a434fb37c0755 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Wed, 7 Dec 2022 16:24:47 +0800 Subject: [PATCH 2/3] fix to edge type --- src/graph/optimizer/rule/PushEFilterDownRule.cpp | 14 +++++++++++--- src/graph/optimizer/rule/PushEFilterDownRule.h | 1 + .../features/optimizer/PushEFilterDownRule.feature | 14 +++++++------- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/graph/optimizer/rule/PushEFilterDownRule.cpp b/src/graph/optimizer/rule/PushEFilterDownRule.cpp index b49ccfb5d1d..1d212e7102e 100644 --- a/src/graph/optimizer/rule/PushEFilterDownRule.cpp +++ b/src/graph/optimizer/rule/PushEFilterDownRule.cpp @@ -53,8 +53,12 @@ StatusOr PushEFilterDownRule::transform( auto pool = qctx->objPool(); auto eFilter = traverse->eFilter()->clone(); - eFilter = rewriteStarEdge( - eFilter, traverse->space(), *DCHECK_NOTNULL(traverse->edgeProps()), qctx->schemaMng(), pool); + eFilter = rewriteStarEdge(eFilter, + traverse->space(), + *DCHECK_NOTNULL(traverse->edgeProps()), + qctx->schemaMng(), + traverse->edgeDirection() == storage::cpp2::EdgeDirection::BOTH, + pool); if (eFilter == nullptr) { return TransformResult::noTransform(); } @@ -93,6 +97,7 @@ std::string PushEFilterDownRule::toString() const { GraphSpaceID spaceId, const std::vector &edges, meta::SchemaManager *schemaMng, + bool isBothDirection, ObjectPool *pool) { graph::RewriteVisitor::Matcher matcher = [](const Expression *exp) -> bool { switch (exp->kind()) { @@ -113,7 +118,7 @@ std::string PushEFilterDownRule::toString() const { } }; graph::RewriteVisitor::Rewriter rewriter = - [spaceId, &edges, schemaMng, pool](const Expression *exp) -> Expression * { + [spaceId, &edges, schemaMng, isBothDirection, pool](const Expression *exp) -> Expression * { auto *propertyExpr = static_cast(exp); DCHECK_EQ(propertyExpr->sym(), "*"); DCHECK(!edges.empty()); @@ -126,6 +131,9 @@ std::string PushEFilterDownRule::toString() const { } else { auto args = ArgumentList::make(pool); for (auto &edge : edges) { + if (isBothDirection && edge.get_type() < 0) { + continue; + } auto reEdgeExp = rewriteStarEdge(propertyExpr, spaceId, edge, schemaMng, pool); if (reEdgeExp == nullptr) { return nullptr; diff --git a/src/graph/optimizer/rule/PushEFilterDownRule.h b/src/graph/optimizer/rule/PushEFilterDownRule.h index cb2b415d738..4add56e8ef3 100644 --- a/src/graph/optimizer/rule/PushEFilterDownRule.h +++ b/src/graph/optimizer/rule/PushEFilterDownRule.h @@ -26,6 +26,7 @@ class PushEFilterDownRule final : public OptRule { GraphSpaceID spaceId, const std::vector &edges, meta::SchemaManager *schemaMng, + bool isBothDirection, ObjectPool *pool); static Expression *rewriteStarEdge(const PropertyExpression *exp, GraphSpaceID spaceId, diff --git a/tests/tck/features/optimizer/PushEFilterDownRule.feature b/tests/tck/features/optimizer/PushEFilterDownRule.feature index 747e95fa6b8..7366594663c 100644 --- a/tests/tck/features/optimizer/PushEFilterDownRule.feature +++ b/tests/tck/features/optimizer/PushEFilterDownRule.feature @@ -32,7 +32,6 @@ Feature: Push EFilter down rule Then the result should be, in any order: | name | | "Tim Duncan" | - | "Tim Duncan" | And the execution plan should be: | id | name | dependencies | operator info | | 5 | Project | 8 | | @@ -41,18 +40,19 @@ Feature: Push EFilter down rule | 0 | Start | | | When profiling query: """ - MATCH (v:player{name: 'Tim Duncan'})-[e:like{likeness: 95}]-() return v.player.name AS name + MATCH (v:player{name: 'Tim Duncan'})-[e:like|serve{likeness: 95}]-() return v.player.name AS name """ Then the result should be, in any order: | name | | "Tim Duncan" | | "Tim Duncan" | + | "Tim Duncan" | And the execution plan should be: - | id | name | dependencies | operator info | - | 5 | Project | 8 | | - | 8 | Traverse | 7 | {"edge filter": "", "filter": "(like.likeness==95)"} | - | 7 | IndexScan | 0 | | - | 0 | Start | | | + | id | name | dependencies | operator info | + | 5 | Project | 8 | | + | 8 | Traverse | 7 | {"edge filter": "", "filter": "(_any(like.likeness,serve.likeness)==95)"} | + | 7 | IndexScan | 0 | | + | 0 | Start | | | When profiling query: """ MATCH (v:player{name: 'Tim Duncan'})-[e:like*1..2{likeness: 95}]->() return v.player.name AS name From 80eb0d1bae378727b2d849041ac732c11fb08e53 Mon Sep 17 00:00:00 2001 From: jievince <38901892+jievince@users.noreply.github.com> Date: Wed, 7 Dec 2022 17:33:16 +0800 Subject: [PATCH 3/3] update tck --- .../optimizer/rule/PushEFilterDownRule.cpp | 2 +- .../PushFilterDownHashInnerJoinRule.feature | 28 +++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/graph/optimizer/rule/PushEFilterDownRule.cpp b/src/graph/optimizer/rule/PushEFilterDownRule.cpp index 1d212e7102e..0af06f4a4b3 100644 --- a/src/graph/optimizer/rule/PushEFilterDownRule.cpp +++ b/src/graph/optimizer/rule/PushEFilterDownRule.cpp @@ -123,7 +123,7 @@ std::string PushEFilterDownRule::toString() const { DCHECK_EQ(propertyExpr->sym(), "*"); DCHECK(!edges.empty()); Expression *ret = nullptr; - if (edges.size() == 1) { + if (edges.size() == 1 || (isBothDirection && edges.size() == 2)) { ret = rewriteStarEdge(propertyExpr, spaceId, edges.front(), schemaMng, pool); if (ret == nullptr) { return nullptr; diff --git a/tests/tck/features/optimizer/PushFilterDownHashInnerJoinRule.feature b/tests/tck/features/optimizer/PushFilterDownHashInnerJoinRule.feature index e7c09c3b2d8..0fcdd16ade2 100644 --- a/tests/tck/features/optimizer/PushFilterDownHashInnerJoinRule.feature +++ b/tests/tck/features/optimizer/PushFilterDownHashInnerJoinRule.feature @@ -32,19 +32,19 @@ Feature: Push Filter down HashInnerJoin rule | [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | | [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | And the execution plan should be: - | id | name | dependencies | operator info | - | 30 | Sort | 14 | | - | 14 | Project | 19 | | - | 19 | HashInnerJoin | 6,22 | | - | 6 | Project | 20 | | - | 20 | AppendVertices | 2 | | - | 2 | Dedup | 1 | | - | 1 | PassThrough | 3 | | - | 3 | Start | | | - | 22 | Project | 10 | | - | 10 | AppendVertices | 9 | | - | 9 | Traverse | 7 | {"edge filter": "(*.likeness>0)"} | - | 7 | Argument | | | + | id | name | dependencies | operator info | + | 30 | Sort | 14 | | + | 14 | Project | 19 | | + | 19 | HashInnerJoin | 6,22 | | + | 6 | Project | 20 | | + | 20 | AppendVertices | 2 | | + | 2 | Dedup | 1 | | + | 1 | PassThrough | 3 | | + | 3 | Start | | | + | 22 | Project | 10 | | + | 10 | AppendVertices | 9 | | + | 9 | Traverse | 7 | {"filter": "(like.likeness>0)"} | + | 7 | Argument | | | When profiling query: """ MATCH (v:player) @@ -122,7 +122,7 @@ Feature: Push Filter down HashInnerJoin rule | 3 | Start | | | | 25 | Project | 10 | | | 10 | AppendVertices | 9 | | - | 9 | Traverse | 7 | {"edge filter": "(*.likeness>0)"} | + | 9 | Traverse | 7 | {"filter": "(like.likeness>0)"} | | 7 | Argument | | | When profiling query: """