From 22a05321873e4386f4782632bff464304ae398eb Mon Sep 17 00:00:00 2001 From: Shylock Hg <33566796+Shylock-Hg@users.noreply.github.com> Date: Wed, 16 Dec 2020 11:22:51 +0800 Subject: [PATCH 1/7] Match by tag index. --- src/planner/CMakeLists.txt | 1 + src/planner/PlannersRegister.cpp | 7 +- src/planner/match/LabelIndexSeek.cpp | 90 ++++++++++++++++++ src/planner/match/LabelIndexSeek.h | 51 +++++++++++ src/planner/match/TagIndexSeek.h | 19 ---- src/validator/test/CMakeLists.txt | 2 + src/validator/test/MatchValidatorTest.cpp | 53 +++++++++++ src/validator/test/MockIndexManager.cpp | 52 +++++++++++ src/validator/test/MockIndexManager.h | 87 ++++++++++++++++++ src/validator/test/MockSchemaManager.cpp | 9 ++ src/validator/test/ValidatorTestBase.h | 4 + tests/data/nba/schema.ngql | 1 + tests/query/v2/match/test_match_base.py | 6 +- tests/tck/features/match/SeekByTag.feature | 102 +++++++++++++++++++++ 14 files changed, 460 insertions(+), 24 deletions(-) create mode 100644 src/planner/match/LabelIndexSeek.cpp create mode 100644 src/planner/match/LabelIndexSeek.h delete mode 100644 src/planner/match/TagIndexSeek.h create mode 100644 src/validator/test/MatchValidatorTest.cpp create mode 100644 src/validator/test/MockIndexManager.cpp create mode 100644 src/validator/test/MockIndexManager.h create mode 100644 tests/tck/features/match/SeekByTag.feature diff --git a/src/planner/CMakeLists.txt b/src/planner/CMakeLists.txt index c9dad2af7..bcd88cbd5 100644 --- a/src/planner/CMakeLists.txt +++ b/src/planner/CMakeLists.txt @@ -31,4 +31,5 @@ nebula_add_library( match/PropIndexSeek.cpp match/VertexIdSeek.cpp match/Expand.cpp + match/LabelIndexSeek.cpp ) diff --git a/src/planner/PlannersRegister.cpp b/src/planner/PlannersRegister.cpp index 6b4df4aee..2ce50533d 100644 --- a/src/planner/PlannersRegister.cpp +++ b/src/planner/PlannersRegister.cpp @@ -12,6 +12,7 @@ #include "planner/match/StartVidFinder.h" #include "planner/match/PropIndexSeek.h" #include "planner/match/VertexIdSeek.h" +#include "planner/match/LabelIndexSeek.h" namespace nebula { namespace graph { @@ -39,8 +40,10 @@ void PlannersRegister::registMatch() { // MATCH(n:Tag) WHERE n.prop = value RETURN n startVidFinders.emplace_back(&PropIndexSeek::make); - // MATCH(n:Tag) RETURN n; - // planners.emplace_back(&MatchTagScanPlanner::match, &MatchTagScanPlanner::make); + // seek by tag or edge(index) + // MATCH(n: tag) RETURN n + // MATCH(s)-[:edge]->(e) RETURN e + startVidFinders.emplace_back(&LabelIndexSeek::make); } } // namespace graph } // namespace nebula diff --git a/src/planner/match/LabelIndexSeek.cpp b/src/planner/match/LabelIndexSeek.cpp new file mode 100644 index 000000000..9a52a11b1 --- /dev/null +++ b/src/planner/match/LabelIndexSeek.cpp @@ -0,0 +1,90 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#include "planner/match/LabelIndexSeek.h" +#include "planner/Query.h" +#include "planner/match/MatchSolver.h" +#include "util/ExpressionUtils.h" + +namespace nebula { +namespace graph { + +bool LabelIndexSeek::matchNode(NodeContext* nodeCtx) { + auto& node = *nodeCtx->info; + // only require the tag + if (node.tid <= 0) { + return false; + } + + nodeCtx->scanInfo.schemaId = node.tid; + nodeCtx->scanInfo.schemaName = node.label; + + return true; +} + +bool LabelIndexSeek::matchEdge(EdgeContext*) { + // TODO + return false; +} + +StatusOr LabelIndexSeek::transformNode(NodeContext* nodeCtx) { + SubPlan plan; + auto* matchClauseCtx = nodeCtx->matchClauseCtx; + using IQC = nebula::storage::cpp2::IndexQueryContext; + auto indexResult = pickTagIndex(nodeCtx); + NG_RETURN_IF_ERROR(indexResult); + IQC iqctx; + iqctx.set_index_id(indexResult.value()); + auto contexts = std::make_unique>(); + contexts->emplace_back(std::move(iqctx)); + auto columns = std::make_unique>(); + columns->emplace_back(kVid); + auto scan = IndexScan::make(matchClauseCtx->qctx, + nullptr, + matchClauseCtx->space.id, + std::move(contexts), + std::move(columns), + false, + nodeCtx->scanInfo.schemaId); + scan->setColNames({kVid}); + plan.tail = scan; + plan.root = scan; + + // initialize start expression in project node + nodeCtx->initialExpr = ExpressionUtils::newVarPropExpr(kVid); + return plan; +} + +StatusOr LabelIndexSeek::transformEdge(EdgeContext*) { + // TODO + return Status::Error("TODO"); +} + +/*static*/ StatusOr LabelIndexSeek::pickTagIndex(const NodeContext* nodeCtx) { + auto tagId = nodeCtx->scanInfo.schemaId; + const auto* qctx = nodeCtx->matchClauseCtx->qctx; + auto tagIndexesResult = qctx->indexMng()->getTagIndexes(nodeCtx->matchClauseCtx->space.id); + NG_RETURN_IF_ERROR(tagIndexesResult); + auto tagIndexes = std::move(tagIndexesResult).value(); + std::shared_ptr candidateIndex{nullptr}; + for (const auto& index : tagIndexes) { + if (index->get_schema_id().get_tag_id() == tagId) { + if (candidateIndex == nullptr) { + candidateIndex = index; + } else { + candidateIndex = selectIndex(candidateIndex, index); + } + } + } + if (candidateIndex == nullptr) { + return Status::SemanticError("No valid index for label `%s'.", + nodeCtx->scanInfo.schemaName->c_str()); + } + return candidateIndex->get_index_id(); +} + +} // namespace graph +} // namespace nebula diff --git a/src/planner/match/LabelIndexSeek.h b/src/planner/match/LabelIndexSeek.h new file mode 100644 index 000000000..1aa20bb03 --- /dev/null +++ b/src/planner/match/LabelIndexSeek.h @@ -0,0 +1,51 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#ifndef PLANNER_MATCH_LABELINDEXSEEK_H_ +#define PLANNER_MATCH_LABELINDEXSEEK_H_ + +#include "planner/match/StartVidFinder.h" + +namespace nebula { +namespace graph { + +/* + * The LabelIndexSeek was designed to find if could get the starting vids by tag index. + */ +class LabelIndexSeek final : public StartVidFinder { +public: + static std::unique_ptr make() { + return std::unique_ptr(new LabelIndexSeek()); + } + +private: + LabelIndexSeek() = default; + + bool matchNode(NodeContext* nodeCtx) override; + + bool matchEdge(EdgeContext* edgeCtx) override; + + StatusOr transformNode(NodeContext* nodeCtx) override; + + StatusOr transformEdge(EdgeContext* edgeCtx) override; + + static StatusOr pickTagIndex(const NodeContext* nodeCtx); + + std::shared_ptr static selectIndex( + const std::shared_ptr candidate, + const std::shared_ptr income) { + // less fields is better + // TODO(shylock) rank for field itself(e.g. INT is better than STRING) + if (candidate->get_fields().size() > income->get_fields().size()) { + return income; + } + return candidate; + } +}; + +} // namespace graph +} // namespace nebula +#endif // PLANNER_MATCH_LABELINDEXSEEK_H_ diff --git a/src/planner/match/TagIndexSeek.h b/src/planner/match/TagIndexSeek.h deleted file mode 100644 index ac565249c..000000000 --- a/src/planner/match/TagIndexSeek.h +++ /dev/null @@ -1,19 +0,0 @@ -/* Copyright (c) 2020 vesoft inc. All rights reserved. - * - * This source code is licensed under Apache 2.0 License, - * attached with Common Clause Condition 1.0, found in the LICENSES directory. - */ - -#ifndef PLANNER_MATCH_TAGINDEXSEEK_H_ -#define PLANNER_MATCH_TAGINDEXSEEK_H_ - -namespace nebula { -namespace graph { -/* - * The TagIndexSeek was designed to find if could get the starting vids by tag index. - */ -class TagIndexSeek final { -}; -} // namespace graph -} // namespace nebula -#endif // PLANNER_MATCH_TAGINDEXSEEK_H_ diff --git a/src/validator/test/CMakeLists.txt b/src/validator/test/CMakeLists.txt index 49b3fe958..91a44e5a2 100644 --- a/src/validator/test/CMakeLists.txt +++ b/src/validator/test/CMakeLists.txt @@ -6,6 +6,7 @@ nebula_add_library( mock_schema_obj OBJECT MockSchemaManager.cpp + MockIndexManager.cpp ) set(VALIDATOR_TEST_LIBS @@ -69,6 +70,7 @@ nebula_add_test( ValidatorTestBase.cpp ExplainValidatorTest.cpp GroupByValidatorTest.cpp + MatchValidatorTest.cpp SymbolsTest.cpp OBJECTS ${VALIDATOR_TEST_LIBS} diff --git a/src/validator/test/MatchValidatorTest.cpp b/src/validator/test/MatchValidatorTest.cpp new file mode 100644 index 000000000..ab1668e5c --- /dev/null +++ b/src/validator/test/MatchValidatorTest.cpp @@ -0,0 +1,53 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#include "validator/MatchValidator.h" + +#include "validator/test/ValidatorTestBase.h" + +namespace nebula { +namespace graph { + +class MatchValidatorTest : public ValidatorTestBase {}; + +TEST_F(MatchValidatorTest, SeekByTagIndex) { + // empty properties index + { + std::string query = "MATCH (v:person) RETURN id(v) AS id;"; + std::vector expected = {PlanNode::Kind::kProject, + PlanNode::Kind::kFilter, + PlanNode::Kind::kProject, + PlanNode::Kind::kProject, + PlanNode::Kind::kGetVertices, + PlanNode::Kind::kDedup, + PlanNode::Kind::kProject, + PlanNode::Kind::kIndexScan, + PlanNode::Kind::kStart}; + EXPECT_TRUE(checkResult(query, expected)); + } + // non empty properties index + { + std::string query = "MATCH (v:book) RETURN id(v) AS id;"; + std::vector expected = {PlanNode::Kind::kProject, + PlanNode::Kind::kFilter, + PlanNode::Kind::kProject, + PlanNode::Kind::kProject, + PlanNode::Kind::kGetVertices, + PlanNode::Kind::kDedup, + PlanNode::Kind::kProject, + PlanNode::Kind::kIndexScan, + PlanNode::Kind::kStart}; + EXPECT_TRUE(checkResult(query, expected)); + } + // non index + { + std::string query = "MATCH (v:room) RETURN id(v) AS id;"; + EXPECT_FALSE(validate(query)); + } +} + +} // namespace graph +} // namespace nebula diff --git a/src/validator/test/MockIndexManager.cpp b/src/validator/test/MockIndexManager.cpp new file mode 100644 index 000000000..40f7e642c --- /dev/null +++ b/src/validator/test/MockIndexManager.cpp @@ -0,0 +1,52 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#include "validator/test/MockIndexManager.h" +#include +#include + +// tag index: +// person() +// book(name(32)) + +namespace nebula { +namespace graph { + +void MockIndexManager::init(meta::MetaClient *) { + // index related + meta::cpp2::IndexItem person_no_props_index; + person_no_props_index.set_index_id(233); + person_no_props_index.set_index_name("person_no_props_index"); + meta::cpp2::SchemaID personSchemaId; + personSchemaId.set_tag_id(2); + person_no_props_index.set_schema_id(std::move(personSchemaId)); + person_no_props_index.set_schema_name("person"); + + meta::cpp2::IndexItem book_name_index; + book_name_index.set_index_id(234); + book_name_index.set_index_name("book_name_index"); + meta::cpp2::SchemaID bookSchemaId; + bookSchemaId.set_tag_id(5); + book_name_index.set_schema_id(std::move(bookSchemaId)); + book_name_index.set_schema_name("book"); + meta::cpp2::ColumnDef field; + field.set_name("name"); + meta::cpp2::ColumnTypeDef type; + type.set_type(meta::cpp2::PropertyType::FIXED_STRING); + type.set_type_length(32); + field.set_type(std::move(type)); + book_name_index.set_fields({}); + book_name_index.fields.emplace_back(std::move(field)); + + tagIndexes_.emplace(1, std::vector>{}); + tagIndexes_[1].emplace_back( + std::make_shared(std::move(person_no_props_index))); + tagIndexes_[1].emplace_back( + std::make_shared(std::move(book_name_index))); +} + +} // namespace graph +} // namespace nebula diff --git a/src/validator/test/MockIndexManager.h b/src/validator/test/MockIndexManager.h new file mode 100644 index 000000000..478818026 --- /dev/null +++ b/src/validator/test/MockIndexManager.h @@ -0,0 +1,87 @@ +/* Copyright (c) 2020 vesoft inc. All rights reserved. + * + * This source code is licensed under Apache 2.0 License, + * attached with Common Clause Condition 1.0, found in the LICENSES directory. + */ + +#ifndef VALIDATOR_MOCKINDEXMANAGER_H_ +#define VALIDATOR_MOCKINDEXMANAGER_H_ + +#include +#include "common/meta/IndexManager.h" + +namespace nebula { +namespace graph { + +class MockIndexManager final : public nebula::meta::IndexManager { +public: + MockIndexManager() = default; + ~MockIndexManager() = default; + + static std::unique_ptr makeUnique() { + auto instance = std::make_unique(); + instance->init(nullptr); + return instance; + } + + void init(meta::MetaClient *) override; + + using IndexItem = meta::cpp2::IndexItem; + + StatusOr> getTagIndex(GraphSpaceID space, IndexID index) override { + UNUSED(space); + UNUSED(index); + LOG(FATAL) << "Unimplemented"; + } + + StatusOr> getEdgeIndex(GraphSpaceID space, IndexID index) override { + UNUSED(space); + UNUSED(index); + LOG(FATAL) << "Unimplemented"; + } + + StatusOr>> getTagIndexes(GraphSpaceID space) override { + auto fd = tagIndexes_.find(space); + if (fd == tagIndexes_.end()) { + return Status::Error("No space for index"); + } + return fd->second; + } + + StatusOr>> getEdgeIndexes(GraphSpaceID space) override { + UNUSED(space); + LOG(FATAL) << "Unimplemented"; + } + + StatusOr toTagIndexID(GraphSpaceID space, std::string tagName) override { + UNUSED(space); + UNUSED(tagName); + LOG(FATAL) << "Unimplemented"; + } + + StatusOr toEdgeIndexID(GraphSpaceID space, std::string edgeName) override { + UNUSED(space); + UNUSED(edgeName); + LOG(FATAL) << "Unimplemented"; + } + + Status checkTagIndexed(GraphSpaceID space, IndexID index) override { + UNUSED(space); + UNUSED(index); + LOG(FATAL) << "Unimplemented"; + } + + Status checkEdgeIndexed(GraphSpaceID space, IndexID index) override { + UNUSED(space); + UNUSED(index); + LOG(FATAL) << "Unimplemented"; + } + +private: + // index related + std::unordered_map>> tagIndexes_; +}; + +} // namespace graph +} // namespace nebula +#endif // VALIDATOR_MOCKINDEXMANAGER_H_ diff --git a/src/validator/test/MockSchemaManager.cpp b/src/validator/test/MockSchemaManager.cpp index aec57a7f7..b91c3d36b 100644 --- a/src/validator/test/MockSchemaManager.cpp +++ b/src/validator/test/MockSchemaManager.cpp @@ -5,6 +5,7 @@ */ #include "validator/test/MockSchemaManager.h" +#include namespace nebula { namespace graph { @@ -12,6 +13,8 @@ namespace graph { // space: test_space // tag: // person(name string, age int8) +// book(name string) +// room(number int8) // edge: // like(start timestamp, end timestamp, likeness int64) void MockSchemaManager::init() { @@ -24,6 +27,8 @@ void MockSchemaManager::init() { edgeIdNames_.emplace(4, "serve"); tagNameIds_.emplace("book", 5); tagIdNames_.emplace(5, "book"); + tagNameIds_.emplace("room", 6); + tagIdNames_.emplace(6, "room"); Tags tagSchemas; // person {name : string, age : int8} @@ -35,6 +40,10 @@ void MockSchemaManager::init() { std::shared_ptr bookSchema(new meta::NebulaSchemaProvider(0)); bookSchema->addField("name", meta::cpp2::PropertyType::STRING); tagSchemas.emplace(5, bookSchema); + // room {number : int8} + std::shared_ptr roomSchema(new meta::NebulaSchemaProvider(0)); + roomSchema->addField("number", meta::cpp2::PropertyType::INT8); + tagSchemas.emplace(5, roomSchema); tagSchemas_.emplace(1, std::move(tagSchemas)); diff --git a/src/validator/test/ValidatorTestBase.h b/src/validator/test/ValidatorTestBase.h index 3f37c32f3..34966b529 100644 --- a/src/validator/test/ValidatorTestBase.h +++ b/src/validator/test/ValidatorTestBase.h @@ -21,6 +21,7 @@ #include "common/base/ObjectPool.h" #include "validator/Validator.h" #include "validator/test/MockSchemaManager.h" +#include "validator/test/MockIndexManager.h" namespace nebula { namespace graph { @@ -35,6 +36,7 @@ class ValidatorTestBase : public ::testing::Test { spaceInfo.spaceDesc.space_name = "test_space"; session_->setSpace(std::move(spaceInfo)); schemaMng_ = CHECK_NOTNULL(MockSchemaManager::makeUnique()); + indexMng_ = CHECK_NOTNULL(MockIndexManager::makeUnique()); pool_ = std::make_unique(); PlannersRegister::registPlanners(); } @@ -57,6 +59,7 @@ class ValidatorTestBase : public ::testing::Test { auto qctx = pool_->add(new QueryContext()); qctx->setRCtx(std::move(rctx)); qctx->setSchemaManager(schemaMng_.get()); + qctx->setIndexManager(indexMng_.get()); qctx->setCharsetInfo(CharsetInfo::instance()); return qctx; } @@ -116,6 +119,7 @@ class ValidatorTestBase : public ::testing::Test { protected: std::shared_ptr session_; std::unique_ptr schemaMng_; + std::unique_ptr indexMng_{nullptr}; std::unique_ptr sentences_; std::unique_ptr pool_; }; diff --git a/tests/data/nba/schema.ngql b/tests/data/nba/schema.ngql index 7da4463bb..b5104ebfe 100644 --- a/tests/data/nba/schema.ngql +++ b/tests/data/nba/schema.ngql @@ -10,3 +10,4 @@ CREATE EDGE IF NOT EXISTS teammate(start_year int, end_year int); CREATE TAG INDEX IF NOT EXISTS player_name_index ON player(name(64)); CREATE TAG INDEX IF NOT EXISTS player_age_index ON player(age); CREATE TAG INDEX IF NOT EXISTS team_name_index ON team(name(64)); +CREATE TAG INDEX IF NOT EXISTS bachelor_index ON bachelor(); diff --git a/tests/query/v2/match/test_match_base.py b/tests/query/v2/match/test_match_base.py index d28b4ef72..3428caf63 100644 --- a/tests/query/v2/match/test_match_base.py +++ b/tests/query/v2/match/test_match_base.py @@ -601,9 +601,9 @@ def test_unimplemented_features(self): self.check_resp_failed(resp) # Scan by label - stmt = 'MATCH (v:player) return v' - resp = self.execute(stmt) - self.check_resp_failed(resp) + # stmt = 'MATCH (v:player) return v' + # resp = self.execute(stmt) + # self.check_resp_failed(resp) # Scan by label stmt = 'MATCH (v:player:person) return v' diff --git a/tests/tck/features/match/SeekByTag.feature b/tests/tck/features/match/SeekByTag.feature new file mode 100644 index 000000000..600903a33 --- /dev/null +++ b/tests/tck/features/match/SeekByTag.feature @@ -0,0 +1,102 @@ +Feature: Match seek by tag + + Background: Prepare space + Given a graph with space named "nba" + + Scenario: seek by empty tag index + When executing query: + """ + MATCH (v:bachelor) + RETURN id(v) AS vid + """ + Then the result should be, in any order: + | vid | + | 'Tim Duncan' | + And no side effects + When executing query: + """ + MATCH (v:bachelor) + RETURN id(v) AS vid, v.age AS age + """ + Then the result should be, in any order: + | vid | age | + | 'Tim Duncan' | 42 | + And no side effects + + Scenario: seek by tag index + When executing query: + """ + MATCH (v:team) + RETURN id(v) + """ + Then the result should be, in any order: + | id(v) | + | 'Nets' | + | 'Pistons' | + | 'Bucks' | + | 'Mavericks' | + | 'Clippers' | + | 'Thunders' | + | 'Lakers' | + | 'Jazz' | + | 'Nuggets' | + | 'Wizards' | + | 'Pacers' | + | 'Timberwolves' | + | 'Hawks' | + | 'Warriors' | + | 'Magic' | + | 'Rockets' | + | 'Pelicans' | + | 'Raptors' | + | 'Spurs' | + | 'Heat' | + | 'Grizzlies' | + | 'Knicks' | + | 'Suns' | + | 'Hornets' | + | 'Cavaliers' | + | 'Kings' | + | 'Celtics' | + | '76ers' | + | 'Trail Blazers' | + | 'Bulls' | + And no side effects + When executing query: + """ + MATCH (v:team) + RETURN id(v) AS vid, v.name AS name + """ + Then the result should be, in any order: + | vid | name | + | 'Nets' | 'Nets' | + | 'Pistons' | 'Pistons' | + | 'Bucks' | 'Bucks' | + | 'Mavericks' | 'Mavericks' | + | 'Clippers' | 'Clippers' | + | 'Thunders' | 'Thunders' | + | 'Lakers' | 'Lakers' | + | 'Jazz' | 'Jazz' | + | 'Nuggets' | 'Nuggets' | + | 'Wizards' | 'Wizards' | + | 'Pacers' | 'Pacers' | + | 'Timberwolves' | 'Timberwolves' | + | 'Hawks' | 'Hawks' | + | 'Warriors' | 'Warriors' | + | 'Magic' | 'Magic' | + | 'Rockets' | 'Rockets' | + | 'Pelicans' | 'Pelicans' | + | 'Raptors' | 'Raptors' | + | 'Spurs' | 'Spurs' | + | 'Heat' | 'Heat' | + | 'Grizzlies' | 'Grizzlies' | + | 'Knicks' | 'Knicks' | + | 'Suns' | 'Suns' | + | 'Hornets' | 'Hornets' | + | 'Cavaliers' | 'Cavaliers' | + | 'Kings' | 'Kings' | + | 'Celtics' | 'Celtics' | + | '76ers' | '76ers' | + | 'Trail Blazers' | 'Trail Blazers' | + | 'Bulls' | 'Bulls' | + And no side effects From a589c6825bc5b824589e73b9a7857771e5fc2b64 Mon Sep 17 00:00:00 2001 From: Shylock Hg <33566796+Shylock-Hg@users.noreply.github.com> Date: Wed, 16 Dec 2020 13:33:58 +0800 Subject: [PATCH 2/7] Add the extend test cases. --- src/validator/test/MatchValidatorTest.cpp | 21 +++++++++++++++++++++ tests/tck/features/match/SeekByTag.feature | 11 +++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/validator/test/MatchValidatorTest.cpp b/src/validator/test/MatchValidatorTest.cpp index ab1668e5c..9154804e0 100644 --- a/src/validator/test/MatchValidatorTest.cpp +++ b/src/validator/test/MatchValidatorTest.cpp @@ -42,6 +42,27 @@ TEST_F(MatchValidatorTest, SeekByTagIndex) { PlanNode::Kind::kStart}; EXPECT_TRUE(checkResult(query, expected)); } + // non empty properties index with extend + { + std::string query = "MATCH (p:person)-[:like]->(b:book) RETURN b.name AS book;"; + std::vector expected = {PlanNode::Kind::kProject, + PlanNode::Kind::kFilter, + PlanNode::Kind::kProject, + PlanNode::Kind::kDataJoin, + PlanNode::Kind::kProject, + PlanNode::Kind::kGetVertices, + PlanNode::Kind::kDedup, + PlanNode::Kind::kProject, + PlanNode::Kind::kFilter, + PlanNode::Kind::kPassThrough, + PlanNode::Kind::kProject, + PlanNode::Kind::kGetNeighbors, + PlanNode::Kind::kDedup, + PlanNode::Kind::kProject, + PlanNode::Kind::kIndexScan, + PlanNode::Kind::kStart}; + EXPECT_TRUE(checkResult(query, expected)); + } // non index { std::string query = "MATCH (v:room) RETURN id(v) AS id;"; diff --git a/tests/tck/features/match/SeekByTag.feature b/tests/tck/features/match/SeekByTag.feature index 600903a33..dd579f953 100644 --- a/tests/tck/features/match/SeekByTag.feature +++ b/tests/tck/features/match/SeekByTag.feature @@ -100,3 +100,14 @@ Feature: Match seek by tag | 'Trail Blazers' | 'Trail Blazers' | | 'Bulls' | 'Bulls' | And no side effects + + Scenario: seek by tag index with extend + When executing query: + """ + MATCH (p:bachelor)-[:serve]->(t) + RETURN t.name AS team + """ + Then the result should be, in any order: + | team | + | 'Spurs' | + And no side effects From cbbb16d08b4944de21f58f64a1240f4ed279662e Mon Sep 17 00:00:00 2001 From: Shylock Hg <33566796+Shylock-Hg@users.noreply.github.com> Date: Wed, 23 Dec 2020 10:42:56 +0800 Subject: [PATCH 3/7] Address @CPWstatic's comment that need check the index when match pattern. --- src/context/ast/QueryAstContext.h | 2 ++ src/planner/match/LabelIndexSeek.cpp | 11 ++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/context/ast/QueryAstContext.h b/src/context/ast/QueryAstContext.h index f0ced1840..92811c69c 100644 --- a/src/context/ast/QueryAstContext.h +++ b/src/context/ast/QueryAstContext.h @@ -60,6 +60,8 @@ struct ScanInfo { Expression *filter{nullptr}; int32_t schemaId{0}; const std::string *schemaName{nullptr}; + // use for seek by index itself + IndexID indexId{-1}; }; struct CypherClauseContextBase : AstContext { diff --git a/src/planner/match/LabelIndexSeek.cpp b/src/planner/match/LabelIndexSeek.cpp index 9a52a11b1..db6fd7951 100644 --- a/src/planner/match/LabelIndexSeek.cpp +++ b/src/planner/match/LabelIndexSeek.cpp @@ -22,6 +22,13 @@ bool LabelIndexSeek::matchNode(NodeContext* nodeCtx) { nodeCtx->scanInfo.schemaId = node.tid; nodeCtx->scanInfo.schemaName = node.label; + auto indexResult = pickTagIndex(nodeCtx); + if (!indexResult.ok()) { + return false; + } + + nodeCtx->scanInfo.indexId = indexResult.value(); + return true; } @@ -34,10 +41,8 @@ StatusOr LabelIndexSeek::transformNode(NodeContext* nodeCtx) { SubPlan plan; auto* matchClauseCtx = nodeCtx->matchClauseCtx; using IQC = nebula::storage::cpp2::IndexQueryContext; - auto indexResult = pickTagIndex(nodeCtx); - NG_RETURN_IF_ERROR(indexResult); IQC iqctx; - iqctx.set_index_id(indexResult.value()); + iqctx.set_index_id(nodeCtx->scanInfo.indexId); auto contexts = std::make_unique>(); contexts->emplace_back(std::move(iqctx)); auto columns = std::make_unique>(); From 44bd0c5a1cda6d547588f4d11aab7f7fbcea71e3 Mon Sep 17 00:00:00 2001 From: Shylock Hg <33566796+Shylock-Hg@users.noreply.github.com> Date: Mon, 28 Dec 2020 17:37:04 +0800 Subject: [PATCH 4/7] Rebase the tag filter. --- src/validator/test/MatchValidatorTest.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/validator/test/MatchValidatorTest.cpp b/src/validator/test/MatchValidatorTest.cpp index 9154804e0..63d0bab25 100644 --- a/src/validator/test/MatchValidatorTest.cpp +++ b/src/validator/test/MatchValidatorTest.cpp @@ -21,6 +21,8 @@ TEST_F(MatchValidatorTest, SeekByTagIndex) { PlanNode::Kind::kFilter, PlanNode::Kind::kProject, PlanNode::Kind::kProject, + // TODO this tag filter could remove in this case + PlanNode::Kind::kFilter, PlanNode::Kind::kGetVertices, PlanNode::Kind::kDedup, PlanNode::Kind::kProject, @@ -35,6 +37,8 @@ TEST_F(MatchValidatorTest, SeekByTagIndex) { PlanNode::Kind::kFilter, PlanNode::Kind::kProject, PlanNode::Kind::kProject, + // TODO this tag filter could remove in this case + PlanNode::Kind::kFilter, PlanNode::Kind::kGetVertices, PlanNode::Kind::kDedup, PlanNode::Kind::kProject, @@ -50,12 +54,14 @@ TEST_F(MatchValidatorTest, SeekByTagIndex) { PlanNode::Kind::kProject, PlanNode::Kind::kDataJoin, PlanNode::Kind::kProject, + PlanNode::Kind::kFilter, PlanNode::Kind::kGetVertices, PlanNode::Kind::kDedup, PlanNode::Kind::kProject, PlanNode::Kind::kFilter, PlanNode::Kind::kPassThrough, PlanNode::Kind::kProject, + PlanNode::Kind::kFilter, PlanNode::Kind::kGetNeighbors, PlanNode::Kind::kDedup, PlanNode::Kind::kProject, From 8ca8030d15ecaca55ccf21fd9cc246ccd59d4a83 Mon Sep 17 00:00:00 2001 From: Shylock Hg <33566796+Shylock-Hg@users.noreply.github.com> Date: Tue, 29 Dec 2020 13:47:06 +0800 Subject: [PATCH 5/7] Fix the memory leak. --- src/planner/match/MatchClausePlanner.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/planner/match/MatchClausePlanner.cpp b/src/planner/match/MatchClausePlanner.cpp index c4791dac2..5287c905c 100644 --- a/src/planner/match/MatchClausePlanner.cpp +++ b/src/planner/match/MatchClausePlanner.cpp @@ -143,7 +143,7 @@ Status MatchClausePlanner::appendFetchVertexPlan(const Expression* nodeFilter, PlanNode* root = gv; if (nodeFilter != nullptr) { - auto filter = nodeFilter->clone().release(); + auto filter = qctx->objPool()->add(nodeFilter->clone().release()); RewriteMatchLabelVisitor visitor( [](const Expression* expr) -> Expression *{ DCHECK(expr->kind() == Expression::Kind::kLabelAttribute || From f5344debbfc77102b5e2d0e478918ed30a26644a Mon Sep 17 00:00:00 2001 From: Shylock Hg <33566796+Shylock-Hg@users.noreply.github.com> Date: Tue, 29 Dec 2020 14:20:29 +0800 Subject: [PATCH 6/7] Fix the format. --- src/planner/match/LabelIndexSeek.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/planner/match/LabelIndexSeek.h b/src/planner/match/LabelIndexSeek.h index 1aa20bb03..c2928c68d 100644 --- a/src/planner/match/LabelIndexSeek.h +++ b/src/planner/match/LabelIndexSeek.h @@ -34,7 +34,7 @@ class LabelIndexSeek final : public StartVidFinder { static StatusOr pickTagIndex(const NodeContext* nodeCtx); - std::shared_ptr static selectIndex( + static std::shared_ptr selectIndex( const std::shared_ptr candidate, const std::shared_ptr income) { // less fields is better From 7aa893d8a66be9719df6cc1013499d6c8d3f35c2 Mon Sep 17 00:00:00 2001 From: Shylock Hg <33566796+Shylock-Hg@users.noreply.github.com> Date: Wed, 30 Dec 2020 11:18:55 +0800 Subject: [PATCH 7/7] Rebase. --- src/planner/match/LabelIndexSeek.cpp | 2 +- src/planner/match/MatchClausePlanner.cpp | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/planner/match/LabelIndexSeek.cpp b/src/planner/match/LabelIndexSeek.cpp index db6fd7951..b811fd5ba 100644 --- a/src/planner/match/LabelIndexSeek.cpp +++ b/src/planner/match/LabelIndexSeek.cpp @@ -59,7 +59,7 @@ StatusOr LabelIndexSeek::transformNode(NodeContext* nodeCtx) { plan.root = scan; // initialize start expression in project node - nodeCtx->initialExpr = ExpressionUtils::newVarPropExpr(kVid); + nodeCtx->initialExpr.reset(ExpressionUtils::newVarPropExpr(kVid)); return plan; } diff --git a/src/planner/match/MatchClausePlanner.cpp b/src/planner/match/MatchClausePlanner.cpp index c5e4c2b6d..e1e860d9f 100644 --- a/src/planner/match/MatchClausePlanner.cpp +++ b/src/planner/match/MatchClausePlanner.cpp @@ -284,7 +284,6 @@ Status MatchClausePlanner::appendFetchVertexPlan(const Expression* nodeFilter, return new VertexExpression(); }); filter->accept(&visitor); - qctx->objPool()->add(filter); root = Filter::make(qctx, root, filter); }