From b73d9506a2c9b889be3f49e4376198696030cbca Mon Sep 17 00:00:00 2001 From: l30059514 Date: Thu, 29 Aug 2024 15:14:44 +0800 Subject: [PATCH 01/10] Optimized filter push-down in certain scenarios. --- src/graph/planner/match/MatchSolver.cpp | 18 +++++++++++------- src/graph/util/OptimizerUtils.cpp | 10 +++++++--- src/graph/util/OptimizerUtils.h | 2 +- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/graph/planner/match/MatchSolver.cpp b/src/graph/planner/match/MatchSolver.cpp index cb1ab8ddfd4..3cd9005f12b 100644 --- a/src/graph/planner/match/MatchSolver.cpp +++ b/src/graph/planner/match/MatchSolver.cpp @@ -136,23 +136,27 @@ Expression* MatchSolver::makeIndexFilter(const std::string& label, Expression::Kind::kRelGE, }; - std::vector ands; + std::vector opnds; + auto optr = LogicalExpression::makeAnd; auto kind = filter->kind(); if (kinds.count(kind) == 1) { - ands.emplace_back(filter); + opnds.emplace_back(filter); } else if (kind == Expression::Kind::kLogicalAnd) { auto* logic = static_cast(filter); ExpressionUtils::pullAnds(logic); - for (auto& operand : logic->operands()) { - ands.emplace_back(operand); - } + opnds = logic->operands(); + } else if (kind == Expression::Kind::kLogicalOr) { + auto* logic = static_cast(filter); + ExpressionUtils::pullOrs(logic); + opnds = logic->operands(); + optr = LogicalExpression::makeOr; } else { return nullptr; } auto* pool = qctx->objPool(); std::vector relationals; - for (auto* item : ands) { + for (auto item : opnds) { if (kinds.count(item->kind()) != 1) { continue; } @@ -218,7 +222,7 @@ Expression* MatchSolver::makeIndexFilter(const std::string& label, auto* root = relationals[0]; for (auto i = 1u; i < relationals.size(); i++) { - root = LogicalExpression::makeAnd(qctx->objPool(), root, relationals[i]); + root = optr(qctx->objPool(), root, relationals[i]); } return root; diff --git a/src/graph/util/OptimizerUtils.cpp b/src/graph/util/OptimizerUtils.cpp index 9b824c3c538..83b98835de4 100644 --- a/src/graph/util/OptimizerUtils.cpp +++ b/src/graph/util/OptimizerUtils.cpp @@ -801,10 +801,11 @@ Status OptimizerUtils::appendColHint(std::vector& hints, } Status OptimizerUtils::appendIQCtx(const std::shared_ptr& index, - std::vector& iqctx) { + std::vector& iqctx, + const Expression *filter) { IndexQueryContext ctx; ctx.index_id_ref() = index->get_index_id(); - ctx.filter_ref() = ""; + ctx.filter_ref() = (filter) ? Expression::encode(*filter) : ""; iqctx.emplace_back(std::move(ctx)); return Status::OK(); } @@ -940,7 +941,10 @@ Status OptimizerUtils::createIndexQueryCtx(std::vector& iqctx if (index == nullptr) { return Status::IndexNotFound("No valid index found"); } - return appendIQCtx(index, iqctx); + auto in = static_cast(node); + auto* filter = Expression::decode(qctx->objPool(), in->queryContext().begin()->get_filter()); + auto* newFilter = ExpressionUtils::rewriteParameter(filter, qctx); + return appendIQCtx(index, iqctx, newFilter); } } // namespace graph diff --git a/src/graph/util/OptimizerUtils.h b/src/graph/util/OptimizerUtils.h index 6db12b73690..cda78d3a8db 100644 --- a/src/graph/util/OptimizerUtils.h +++ b/src/graph/util/OptimizerUtils.h @@ -148,7 +148,7 @@ class OptimizerUtils { const std::vector &items, IndexQueryContextList &iqctx, const Expression *filter = nullptr); - static Status appendIQCtx(const IndexItemPtr &index, IndexQueryContextList &iqctx); + static Status appendIQCtx(const IndexItemPtr &index, IndexQueryContextList &iqctx,const Expression *filter = nullptr); static Status appendColHint(std::vector &hints, const std::vector &items, const meta::cpp2::ColumnDef &col); From 883ea7687d3bd85dc61ea247e8a6f9f525c49714 Mon Sep 17 00:00:00 2001 From: l30059514 Date: Sat, 31 Aug 2024 14:51:10 +0800 Subject: [PATCH 02/10] Fixed the issue where the execution plan could not correctly handle dictionaries with only one level of nesting. --- tests/common/plan_differ.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/common/plan_differ.py b/tests/common/plan_differ.py index 061f8a2d31e..e383b28a121 100644 --- a/tests/common/plan_differ.py +++ b/tests/common/plan_differ.py @@ -183,10 +183,6 @@ def _is_subdict_nested(self, expect, resp): # The inner map cannot be empty if len(extracted_expected_dict) == 0: return None - # Unnested dict, push the first key into list - if extracted_expected_dict == expect: - key_list.append(list(expect.keys())[0]) - def _try_convert_json(j): try: res = json.loads(j) @@ -202,7 +198,7 @@ def _try_convert_json(j): return j extracted_resp_dict = {} - if len(key_list) == 1: + if not key_list: for k in resp: extracted_resp_dict[k] = _try_convert_json(resp[k]) else: From f2fbd505fa80f47ebe15c85e06016f3dda46b3be Mon Sep 17 00:00:00 2001 From: l30059514 Date: Sat, 31 Aug 2024 16:58:45 +0800 Subject: [PATCH 03/10] Add TCK test cases --- .../tck/features/match/PushFilterDown.feature | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 tests/tck/features/match/PushFilterDown.feature diff --git a/tests/tck/features/match/PushFilterDown.feature b/tests/tck/features/match/PushFilterDown.feature new file mode 100644 index 00000000000..aac566e970e --- /dev/null +++ b/tests/tck/features/match/PushFilterDown.feature @@ -0,0 +1,78 @@ +Feature: Push filter down + + Background: Prepare a new space + Given an empty graph + And create a space with following options: + | partition_num | 1 | + | replica_factor | 1 | + | vid_type | FIXED_STRING(30) | + | charset | utf8 | + | collate | utf8_bin | + And having executed: + """ + CREATE tag player(name string, age int, score int, gender bool); + CREATE EDGE IF NOT EXISTS follow(); + """ + And having executed: + """ + INSERT VERTEX player(name, age, score, gender) VALUES "Tim Duncan":("Tim Duncan", 42, 28, true),"Yao Ming":("Yao Ming", 38, 23, true),"Nneka Ogwumike":("Nneka Ogwumike", 35, 13, false); + INSERT EDGE follow () VALUES "Tim Duncan"->"Yao Ming":(); + """ + And having executed: + """ + create tag index player_index on player(); + create tag index player_name_index on player(name(8)); + rebuild tag index + """ + And wait all indexes ready + + Scenario: Single vertex + When profiling query: + """ + MATCH (v:player) where v.player.name == "Tim Duncan" and v.player.age > 20 RETURN v + """ + Then the execution plan should be: + | id | name | dependencies | operator info | + | 5 | Project | 4 | | + | 4 | Filter | 3 | {"condition": "((v.player.name==\"Tim Duncan\") AND (v.player.age>20))"} | + | 3 | AppendVertices | 2 | | + | 2 | IndexScan | 1 | {"indexCtx": {"filter":"((player.name==\"Tim Duncan\") AND (player.age>20))"}} | + | 1 | Start | | | + + When profiling query: + """ + MATCH (v:player) where v.player.age > 20 RETURN v + """ + Then the execution plan should be: + | id | name | dependencies | operator info | + | 5 | Project | 4 | | + | 4 | Filter | 3 | {"condition": "(v.player.age>20)"} | + | 3 | AppendVertices | 2 | | + | 2 | IndexScan | 1 | {"indexCtx": {"filter":"(player.age>20)"}} | + | 1 | Start | | | + + When profiling query: + """ + MATCH (v:player) where v.player.age < 3 or v.player.age > 20 RETURN v + """ + Then the execution plan should be: + | id | name | dependencies | operator info | + | 5 | Project | 4 | | + | 4 | Filter | 3 | {"condition": "((v.player.age<3) OR (v.player.age>20))"} | + | 3 | AppendVertices | 2 | | + | 2 | IndexScan | 1 | {"indexCtx": {"filter":"((player.age<3) OR (player.age>20))"}} | + | 1 | Start | | | + + Scenario: Vertex and edge + When profiling query: + """ + MATCH (v:player)-[e]-() where v.player.age > 20 RETURN v + """ + Then the execution plan should be: + | id | name | dependencies | operator info | + | 6 | Project | 5 | | + | 5 | Filter | 4 | | + | 4 | AppendVertices | 3 | | + | 3 | Traverse | 2 | | + | 2 | IndexScan | 1 | {"indexCtx": {"filter":"(player.age>20)"}} | + | 1 | Start | | | From 1f299a178f432171b1ea45f46d1055bbfce6f59b Mon Sep 17 00:00:00 2001 From: l30059514 Date: Tue, 3 Sep 2024 10:27:32 +0800 Subject: [PATCH 04/10] Fixed the issue with error down-pushing conditions in certain situations. --- src/graph/planner/match/MatchSolver.cpp | 10 +++++-- src/graph/util/OptimizerUtils.h | 4 ++- .../tck/features/match/PushFilterDown.feature | 26 ++++++++++++++++++- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/graph/planner/match/MatchSolver.cpp b/src/graph/planner/match/MatchSolver.cpp index 3cd9005f12b..effd052381a 100644 --- a/src/graph/planner/match/MatchSolver.cpp +++ b/src/graph/planner/match/MatchSolver.cpp @@ -188,13 +188,19 @@ Expression* MatchSolver::makeIndexFilter(const std::string& label, if (left->kind() == Expression::Kind::kLabelTagProperty && right->kind() == Expression::Kind::kConstant) { if (!extractTagPropName(left, alias, label, &propName)) { - continue; + if (optr == LogicalExpression::makeAnd) { + continue; + } + return nullptr; } constant = static_cast(right); } else if (right->kind() == Expression::Kind::kLabelTagProperty && left->kind() == Expression::Kind::kConstant) { if (!extractTagPropName(right, alias, label, &propName)) { - continue; + if (optr == LogicalExpression::makeAnd) { + continue; + } + return nullptr; } constant = static_cast(left); } else { diff --git a/src/graph/util/OptimizerUtils.h b/src/graph/util/OptimizerUtils.h index cda78d3a8db..0cf3c649be2 100644 --- a/src/graph/util/OptimizerUtils.h +++ b/src/graph/util/OptimizerUtils.h @@ -148,7 +148,9 @@ class OptimizerUtils { const std::vector &items, IndexQueryContextList &iqctx, const Expression *filter = nullptr); - static Status appendIQCtx(const IndexItemPtr &index, IndexQueryContextList &iqctx,const Expression *filter = nullptr); + static Status appendIQCtx(const IndexItemPtr &index, + IndexQueryContextList &iqctx, + const Expression *filter = nullptr); static Status appendColHint(std::vector &hints, const std::vector &items, const meta::cpp2::ColumnDef &col); diff --git a/tests/tck/features/match/PushFilterDown.feature b/tests/tck/features/match/PushFilterDown.feature index aac566e970e..2e592dc34a7 100644 --- a/tests/tck/features/match/PushFilterDown.feature +++ b/tests/tck/features/match/PushFilterDown.feature @@ -66,7 +66,31 @@ Feature: Push filter down Scenario: Vertex and edge When profiling query: """ - MATCH (v:player)-[e]-() where v.player.age > 20 RETURN v + MATCH (v:player)-[]-() where v.player.age > 20 RETURN v + """ + Then the execution plan should be: + | id | name | dependencies | operator info | + | 6 | Project | 5 | | + | 5 | Filter | 4 | | + | 4 | AppendVertices | 3 | | + | 3 | Traverse | 2 | | + | 2 | IndexScan | 1 | {"indexCtx": {"filter":"(player.age>20)"}} | + | 1 | Start | | | + When profiling query: + """ + MATCH (v:player)-[]-(o:player) where v.player.age > 20 or o.player.name == "Yao Ming" RETURN v + """ + Then the execution plan should be: + | id | name | dependencies | operator info | + | 6 | Project | 5 | | + | 5 | Filter | 4 | | + | 4 | AppendVertices | 3 | | + | 3 | Traverse | 2 | | + | 2 | IndexScan | 1 | {"indexCtx": {"filter":""}} | + | 1 | Start | | | + When profiling query: + """ + MATCH (v:player)-[]-(o:player) where v.player.age > 20 and o.player.name == "Yao Ming" RETURN v """ Then the execution plan should be: | id | name | dependencies | operator info | From 3d58cbc4028cc5d26324738a9d6045348ddfaf7a Mon Sep 17 00:00:00 2001 From: l30059514 Date: Tue, 3 Sep 2024 11:36:19 +0800 Subject: [PATCH 05/10] chore: Refactor OptimizerUtils.cpp for better readability --- src/graph/util/OptimizerUtils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/graph/util/OptimizerUtils.cpp b/src/graph/util/OptimizerUtils.cpp index 83b98835de4..d6f6c90b829 100644 --- a/src/graph/util/OptimizerUtils.cpp +++ b/src/graph/util/OptimizerUtils.cpp @@ -802,7 +802,7 @@ Status OptimizerUtils::appendColHint(std::vector& hints, Status OptimizerUtils::appendIQCtx(const std::shared_ptr& index, std::vector& iqctx, - const Expression *filter) { + const Expression* filter) { IndexQueryContext ctx; ctx.index_id_ref() = index->get_index_id(); ctx.filter_ref() = (filter) ? Expression::encode(*filter) : ""; From a9b02217c83053ebb64d5c8c5b4e8195ff6bf5a0 Mon Sep 17 00:00:00 2001 From: l30059514 Date: Tue, 3 Sep 2024 14:26:57 +0800 Subject: [PATCH 06/10] chore: Refactor PushFilterDown.feature for better readability --- .../tck/features/match/PushFilterDown.feature | 80 +++++++++---------- 1 file changed, 39 insertions(+), 41 deletions(-) diff --git a/tests/tck/features/match/PushFilterDown.feature b/tests/tck/features/match/PushFilterDown.feature index 2e592dc34a7..ed2bbb1bb61 100644 --- a/tests/tck/features/match/PushFilterDown.feature +++ b/tests/tck/features/match/PushFilterDown.feature @@ -32,36 +32,34 @@ Feature: Push filter down MATCH (v:player) where v.player.name == "Tim Duncan" and v.player.age > 20 RETURN v """ Then the execution plan should be: - | id | name | dependencies | operator info | - | 5 | Project | 4 | | - | 4 | Filter | 3 | {"condition": "((v.player.name==\"Tim Duncan\") AND (v.player.age>20))"} | - | 3 | AppendVertices | 2 | | - | 2 | IndexScan | 1 | {"indexCtx": {"filter":"((player.name==\"Tim Duncan\") AND (player.age>20))"}} | - | 1 | Start | | | - + | id | name | dependencies | operator info | + | 5 | Project | 4 | | + | 4 | Filter | 3 | {"condition": "((v.player.name==\\"Tim Duncan\\") AND (v.player.age>20))"} | + | 3 | AppendVertices | 2 | | + | 2 | IndexScan | 1 | {"indexCtx": {"filter":"((player.name==\\"Tim Duncan\\") AND (player.age>20))"}} | + | 1 | Start | | | When profiling query: """ MATCH (v:player) where v.player.age > 20 RETURN v """ Then the execution plan should be: - | id | name | dependencies | operator info | - | 5 | Project | 4 | | - | 4 | Filter | 3 | {"condition": "(v.player.age>20)"} | - | 3 | AppendVertices | 2 | | - | 2 | IndexScan | 1 | {"indexCtx": {"filter":"(player.age>20)"}} | - | 1 | Start | | | - + | id | name | dependencies | operator info | + | 5 | Project | 4 | | + | 4 | Filter | 3 | {"condition": "(v.player.age>20)"} | + | 3 | AppendVertices | 2 | | + | 2 | IndexScan | 1 | {"indexCtx": {"filter":"(player.age>20)"}} | + | 1 | Start | | | When profiling query: """ MATCH (v:player) where v.player.age < 3 or v.player.age > 20 RETURN v """ Then the execution plan should be: - | id | name | dependencies | operator info | - | 5 | Project | 4 | | - | 4 | Filter | 3 | {"condition": "((v.player.age<3) OR (v.player.age>20))"} | - | 3 | AppendVertices | 2 | | - | 2 | IndexScan | 1 | {"indexCtx": {"filter":"((player.age<3) OR (player.age>20))"}} | - | 1 | Start | | | + | id | name | dependencies | operator info | + | 5 | Project | 4 | | + | 4 | Filter | 3 | {"condition": "((v.player.age<3) OR (v.player.age>20))"} | + | 3 | AppendVertices | 2 | | + | 2 | IndexScan | 1 | {"indexCtx": {"filter":"((player.age<3) OR (player.age>20))"}} | + | 1 | Start | | | Scenario: Vertex and edge When profiling query: @@ -69,34 +67,34 @@ Feature: Push filter down MATCH (v:player)-[]-() where v.player.age > 20 RETURN v """ Then the execution plan should be: - | id | name | dependencies | operator info | - | 6 | Project | 5 | | - | 5 | Filter | 4 | | - | 4 | AppendVertices | 3 | | - | 3 | Traverse | 2 | | - | 2 | IndexScan | 1 | {"indexCtx": {"filter":"(player.age>20)"}} | - | 1 | Start | | | + | id | name | dependencies | operator info | + | 6 | Project | 5 | | + | 5 | Filter | 4 | | + | 4 | AppendVertices | 3 | | + | 3 | Traverse | 2 | | + | 2 | IndexScan | 1 | {"indexCtx": {"filter":"(player.age>20)"}} | + | 1 | Start | | | When profiling query: """ MATCH (v:player)-[]-(o:player) where v.player.age > 20 or o.player.name == "Yao Ming" RETURN v """ Then the execution plan should be: - | id | name | dependencies | operator info | - | 6 | Project | 5 | | - | 5 | Filter | 4 | | - | 4 | AppendVertices | 3 | | - | 3 | Traverse | 2 | | - | 2 | IndexScan | 1 | {"indexCtx": {"filter":""}} | - | 1 | Start | | | + | id | name | dependencies | operator info | + | 6 | Project | 5 | | + | 5 | Filter | 4 | | + | 4 | AppendVertices | 3 | | + | 3 | Traverse | 2 | | + | 2 | IndexScan | 1 | {"indexCtx": {"filter":""}} | + | 1 | Start | | | When profiling query: """ MATCH (v:player)-[]-(o:player) where v.player.age > 20 and o.player.name == "Yao Ming" RETURN v """ Then the execution plan should be: - | id | name | dependencies | operator info | - | 6 | Project | 5 | | - | 5 | Filter | 4 | | - | 4 | AppendVertices | 3 | | - | 3 | Traverse | 2 | | - | 2 | IndexScan | 1 | {"indexCtx": {"filter":"(player.age>20)"}} | - | 1 | Start | | | + | id | name | dependencies | operator info | + | 6 | Project | 5 | | + | 5 | Filter | 4 | | + | 4 | AppendVertices | 3 | | + | 3 | Traverse | 2 | | + | 2 | IndexScan | 1 | {"indexCtx": {"filter":"(player.age>20)"}} | + | 1 | Start | | | From 83803636cae2df2550db3f448792842754cb41f4 Mon Sep 17 00:00:00 2001 From: l30059514 Date: Tue, 3 Sep 2024 19:27:24 +0800 Subject: [PATCH 07/10] chore: Refactor PushFilterDown.feature for better readability --- tests/tck/features/match/PushFilterDown.feature | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/tck/features/match/PushFilterDown.feature b/tests/tck/features/match/PushFilterDown.feature index ed2bbb1bb61..385882905c1 100644 --- a/tests/tck/features/match/PushFilterDown.feature +++ b/tests/tck/features/match/PushFilterDown.feature @@ -26,18 +26,19 @@ Feature: Push filter down """ And wait all indexes ready + @ccc Scenario: Single vertex When profiling query: """ MATCH (v:player) where v.player.name == "Tim Duncan" and v.player.age > 20 RETURN v """ Then the execution plan should be: - | id | name | dependencies | operator info | - | 5 | Project | 4 | | - | 4 | Filter | 3 | {"condition": "((v.player.name==\\"Tim Duncan\\") AND (v.player.age>20))"} | - | 3 | AppendVertices | 2 | | - | 2 | IndexScan | 1 | {"indexCtx": {"filter":"((player.name==\\"Tim Duncan\\") AND (player.age>20))"}} | - | 1 | Start | | | + | id | name | dependencies | operator info | + | 5 | Project | 4 | | + | 4 | Filter | 3 | {"condition": "((v.player.name==\"Tim Duncan\") AND (v.player.age>20))"} | + | 3 | AppendVertices | 2 | | + | 2 | IndexScan | 1 | {"indexCtx": {"filter":"((player.name==\"Tim Duncan\") AND (player.age>20))"}} | + | 1 | Start | | | When profiling query: """ MATCH (v:player) where v.player.age > 20 RETURN v From f1573899f1050e4af55ae381497f3bef542786c5 Mon Sep 17 00:00:00 2001 From: l30059514 Date: Thu, 5 Sep 2024 10:29:15 +0800 Subject: [PATCH 08/10] Fixed the issue with error down-pushing conditions in certain situations. --- src/graph/planner/match/MatchSolver.cpp | 20 +++++++++++++++---- .../tck/features/match/PushFilterDown.feature | 2 +- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/graph/planner/match/MatchSolver.cpp b/src/graph/planner/match/MatchSolver.cpp index effd052381a..b4de7138054 100644 --- a/src/graph/planner/match/MatchSolver.cpp +++ b/src/graph/planner/match/MatchSolver.cpp @@ -158,7 +158,10 @@ Expression* MatchSolver::makeIndexFilter(const std::string& label, std::vector relationals; for (auto item : opnds) { if (kinds.count(item->kind()) != 1) { - continue; + if (optr == LogicalExpression::makeAnd) { + continue; + } + return nullptr; } auto* binary = static_cast(item); @@ -178,10 +181,16 @@ Expression* MatchSolver::makeIndexFilter(const std::string& label, la = static_cast(right); constant = static_cast(left); } else { - continue; + if (optr == LogicalExpression::makeAnd) { + continue; + } + return nullptr; } if (la->left()->name() != alias) { - continue; + if (optr == LogicalExpression::makeAnd) { + continue; + } + return nullptr; } propName = la->right()->value().getStr(); } else { @@ -204,7 +213,10 @@ Expression* MatchSolver::makeIndexFilter(const std::string& label, } constant = static_cast(left); } else { - continue; + if (optr == LogicalExpression::makeAnd) { + continue; + } + return nullptr; } } diff --git a/tests/tck/features/match/PushFilterDown.feature b/tests/tck/features/match/PushFilterDown.feature index 385882905c1..e6f7796bdab 100644 --- a/tests/tck/features/match/PushFilterDown.feature +++ b/tests/tck/features/match/PushFilterDown.feature @@ -26,7 +26,7 @@ Feature: Push filter down """ And wait all indexes ready - @ccc + Scenario: Single vertex When profiling query: """ From d9f0eadce67187f6475d957d228fbbf675afded8 Mon Sep 17 00:00:00 2001 From: l30059514 Date: Mon, 9 Sep 2024 17:08:15 +0800 Subject: [PATCH 09/10] Refactor MatchSolver.cpp for better readability and maintainability --- src/graph/planner/match/MatchSolver.cpp | 90 +++++++++---------- src/graph/planner/match/MatchSolver.h | 18 ++++ .../tck/features/match/PushFilterDown.feature | 1 - tests/tck/features/yield/parameter.feature | 1 + 4 files changed, 62 insertions(+), 48 deletions(-) diff --git a/src/graph/planner/match/MatchSolver.cpp b/src/graph/planner/match/MatchSolver.cpp index b4de7138054..69d11923d70 100644 --- a/src/graph/planner/match/MatchSolver.cpp +++ b/src/graph/planner/match/MatchSolver.cpp @@ -123,6 +123,43 @@ bool MatchSolver::extractTagPropName(const Expression* expr, return true; } +bool MatchSolver::extractTagPropName(const Expression* expr, + const std::string& alias, + std::string* propName) { + if (expr->kind() != Expression::Kind::kLabelAttribute) return false; + auto laExpr = static_cast(expr); + if (laExpr->left()->name() != alias) return false; + *propName = laExpr->right()->value().getStr(); + return true; +} + +bool MatchSolver::extract(const Expression* left, + const Expression* right, + const std::string& label, + const std::string& alias, + Expression::Kind labelKind, + const ConstantExpression*& constant, + std::string& propName) { + if (left->kind() != labelKind || right->kind() != Expression::Kind::kConstant) { + return false; + } + constant = static_cast(right); + + return extractTagPropName(left, alias, label, &propName) || + extractTagPropName(left, alias, &propName); +} + +bool MatchSolver::extractLabelAndConstant(const Expression* left, + const Expression* right, + const std::string& label, + const std::string& alias, + Expression::Kind labelKind, + const ConstantExpression*& constant, + std::string& propName) { + return extract(left, right, label, alias, labelKind, constant, propName) || + extract(right, left, label, alias, labelKind, constant, propName); +} + Expression* MatchSolver::makeIndexFilter(const std::string& label, const std::string& alias, Expression* filter, @@ -170,54 +207,13 @@ Expression* MatchSolver::makeIndexFilter(const std::string& label, const ConstantExpression* constant = nullptr; std::string propName; // TODO(aiee) extract the logic that applies to both match and lookup - if (isEdgeProperties) { - const LabelAttributeExpression* la = nullptr; - if (left->kind() == Expression::Kind::kLabelAttribute && - right->kind() == Expression::Kind::kConstant) { - la = static_cast(left); - constant = static_cast(right); - } else if (right->kind() == Expression::Kind::kLabelAttribute && - left->kind() == Expression::Kind::kConstant) { - la = static_cast(right); - constant = static_cast(left); - } else { - if (optr == LogicalExpression::makeAnd) { - continue; - } - return nullptr; - } - if (la->left()->name() != alias) { - if (optr == LogicalExpression::makeAnd) { - continue; - } - return nullptr; - } - propName = la->right()->value().getStr(); - } else { - if (left->kind() == Expression::Kind::kLabelTagProperty && - right->kind() == Expression::Kind::kConstant) { - if (!extractTagPropName(left, alias, label, &propName)) { - if (optr == LogicalExpression::makeAnd) { - continue; - } - return nullptr; - } - constant = static_cast(right); - } else if (right->kind() == Expression::Kind::kLabelTagProperty && - left->kind() == Expression::Kind::kConstant) { - if (!extractTagPropName(right, alias, label, &propName)) { - if (optr == LogicalExpression::makeAnd) { - continue; - } - return nullptr; - } - constant = static_cast(left); - } else { - if (optr == LogicalExpression::makeAnd) { - continue; - } - return nullptr; + auto labelkind = (isEdgeProperties) ? Expression::Kind::kLabelAttribute + : Expression::Kind::kLabelTagProperty; + if (!extractLabelAndConstant(left, right, label, alias, labelkind, constant, propName)) { + if (optr == LogicalExpression::makeAnd) { + continue; } + return nullptr; } auto* tpExpr = diff --git a/src/graph/planner/match/MatchSolver.h b/src/graph/planner/match/MatchSolver.h index b09bce5d1e0..8cd1d998391 100644 --- a/src/graph/planner/match/MatchSolver.h +++ b/src/graph/planner/match/MatchSolver.h @@ -38,6 +38,8 @@ class MatchSolver final { QueryContext* qctx, bool isEdgeProperties = false); + static bool extractTagPropName(const Expression* expr, const string& alias, string* propName); + static bool extractTagPropName(const Expression* expr, const std::string& alias, const std::string& label, @@ -55,6 +57,22 @@ class MatchSolver final { // Build yield columns for match & shortestPath statement static void buildProjectColumns(QueryContext* qctx, const Path& path, SubPlan& plan); + + static bool extractLabelAndConstant(const Expression* left, + const Expression* right, + const string& label, + const string& alias, + Expression::Kind labelKind, + const ConstantExpression*& constant, + string& propName); + + static bool extract(const Expression* left, + const Expression* right, + const string& label, + const string& alias, + Expression::Kind labelKind, + const ConstantExpression*& constant, + string& propName); }; } // namespace graph diff --git a/tests/tck/features/match/PushFilterDown.feature b/tests/tck/features/match/PushFilterDown.feature index e6f7796bdab..095a63d1d2a 100644 --- a/tests/tck/features/match/PushFilterDown.feature +++ b/tests/tck/features/match/PushFilterDown.feature @@ -26,7 +26,6 @@ Feature: Push filter down """ And wait all indexes ready - Scenario: Single vertex When profiling query: """ diff --git a/tests/tck/features/yield/parameter.feature b/tests/tck/features/yield/parameter.feature index df32e3b31c1..178164e86b8 100644 --- a/tests/tck/features/yield/parameter.feature +++ b/tests/tck/features/yield/parameter.feature @@ -48,6 +48,7 @@ Feature: Parameter | v | | {a: 3, b: false, c: "Tim Duncan"} | + @skip #bug to fix: https://github.com/vesoft-inc/nebula/issues/5939 Scenario: [param-test-004] cypher with parameters # where clause When executing query: From 32da07c73e5d6a58761b96f65a5748bb5fd62edc Mon Sep 17 00:00:00 2001 From: l30059514 Date: Mon, 9 Sep 2024 19:02:06 +0800 Subject: [PATCH 10/10] chore: Refactor MatchSolver.h for better readability and maintainability --- src/graph/planner/match/MatchSolver.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/graph/planner/match/MatchSolver.h b/src/graph/planner/match/MatchSolver.h index 8cd1d998391..e3d44ab42ba 100644 --- a/src/graph/planner/match/MatchSolver.h +++ b/src/graph/planner/match/MatchSolver.h @@ -39,7 +39,7 @@ class MatchSolver final { bool isEdgeProperties = false); static bool extractTagPropName(const Expression* expr, const string& alias, string* propName); - + static bool extractTagPropName(const Expression* expr, const std::string& alias, const std::string& label,