diff --git a/src/clients/meta/MetaClient.cpp b/src/clients/meta/MetaClient.cpp index b48248d9309..851791b0f67 100644 --- a/src/clients/meta/MetaClient.cpp +++ b/src/clients/meta/MetaClient.cpp @@ -1714,7 +1714,7 @@ folly::Future> MetaClient::createTagIndex(GraphSpaceID spaceID std::string tagName, std::vector fields, bool ifNotExists, - cpp2::IndexParams indexParams, + const cpp2::IndexParams* indexParams, const std::string* comment) { cpp2::CreateTagIndexReq req; req.set_space_id(spaceID); @@ -1722,7 +1722,9 @@ folly::Future> MetaClient::createTagIndex(GraphSpaceID spaceID req.set_tag_name(std::move(tagName)); req.set_fields(std::move(fields)); req.set_if_not_exists(ifNotExists); - req.set_index_params(std::move(indexParams)); + if (indexParams != nullptr) { + req.set_index_params(*indexParams); + } if (comment != nullptr) { req.set_comment(*comment); } @@ -1828,7 +1830,7 @@ folly::Future> MetaClient::createEdgeIndex( std::string edgeName, std::vector fields, bool ifNotExists, - cpp2::IndexParams indexParams, + const cpp2::IndexParams* indexParams, const std::string* comment) { cpp2::CreateEdgeIndexReq req; req.set_space_id(spaceID); @@ -1836,7 +1838,9 @@ folly::Future> MetaClient::createEdgeIndex( req.set_edge_name(std::move(edgeName)); req.set_fields(std::move(fields)); req.set_if_not_exists(ifNotExists); - req.set_index_params(std::move(indexParams)); + if (indexParams != nullptr) { + req.set_index_params(*indexParams); + } if (comment != nullptr) { req.set_comment(*comment); } diff --git a/src/clients/meta/MetaClient.h b/src/clients/meta/MetaClient.h index 48fe1138b74..5edefa2a000 100644 --- a/src/clients/meta/MetaClient.h +++ b/src/clients/meta/MetaClient.h @@ -317,7 +317,7 @@ class MetaClient { std::string tagName, std::vector fields, bool ifNotExists = false, - meta::cpp2::IndexParams indexParams = cpp2::IndexParams{}, + const meta::cpp2::IndexParams* indexParams = nullptr, const std::string* comment = nullptr); // Remove the define of tag index @@ -333,14 +333,13 @@ class MetaClient { folly::Future>> listTagIndexStatus(GraphSpaceID spaceId); - folly::Future> createEdgeIndex( - GraphSpaceID spaceID, - std::string indexName, - std::string edgeName, - std::vector fields, - bool ifNotExists = false, - cpp2::IndexParams indexParams = cpp2::IndexParams{}, - const std::string* comment = nullptr); + folly::Future> createEdgeIndex(GraphSpaceID spaceID, + std::string indexName, + std::string edgeName, + std::vector fields, + bool ifNotExists = false, + const cpp2::IndexParams* indexParams = nullptr, + const std::string* comment = nullptr); // Remove the definition of edge index folly::Future> dropEdgeIndex(GraphSpaceID spaceId, diff --git a/src/common/utils/IndexKeyUtils.cpp b/src/common/utils/IndexKeyUtils.cpp index 28c6160b7a6..dd43757fb56 100644 --- a/src/common/utils/IndexKeyUtils.cpp +++ b/src/common/utils/IndexKeyUtils.cpp @@ -56,12 +56,14 @@ std::vector IndexKeyUtils::encodeValues(std::vector&& values if (!value.isNull()) { DCHECK(value.type() == Value::Type::GEOGRAPHY); geo::RegionCoverParams rc; - const auto& indexParams = indexItem->get_index_params(); - if (indexParams.s2_max_level_ref().has_value()) { - rc.maxCellLevel_ = indexParams.s2_max_level_ref().value(); - } - if (indexParams.s2_max_cells_ref().has_value()) { - rc.maxCellNum_ = indexParams.s2_max_level_ref().value(); + const auto* indexParams = indexItem->get_index_params(); + if (indexParams) { + if (indexParams->s2_max_level_ref().has_value()) { + rc.maxCellLevel_ = indexParams->s2_max_level_ref().value(); + } + if (indexParams->s2_max_cells_ref().has_value()) { + rc.maxCellNum_ = indexParams->s2_max_cells_ref().value(); + } } indexes = encodeGeography(value.getGeography(), rc); } else { diff --git a/src/graph/executor/maintain/EdgeIndexExecutor.cpp b/src/graph/executor/maintain/EdgeIndexExecutor.cpp index a4e16c2d2f0..cfe1b85ba7d 100644 --- a/src/graph/executor/maintain/EdgeIndexExecutor.cpp +++ b/src/graph/executor/maintain/EdgeIndexExecutor.cpp @@ -23,7 +23,8 @@ folly::Future CreateEdgeIndexExecutor::execute() { ceiNode->getSchemaName(), ceiNode->getFields(), ceiNode->getIfNotExists(), - ceiNode->getIndexParams()) + ceiNode->getIndexParams(), + ceiNode->getComment()) .via(runner()) .thenValue([ceiNode, spaceId](StatusOr resp) { if (!resp.ok()) { diff --git a/src/graph/executor/maintain/TagIndexExecutor.cpp b/src/graph/executor/maintain/TagIndexExecutor.cpp index dc1cbec87c0..86c53301193 100644 --- a/src/graph/executor/maintain/TagIndexExecutor.cpp +++ b/src/graph/executor/maintain/TagIndexExecutor.cpp @@ -23,7 +23,8 @@ folly::Future CreateTagIndexExecutor::execute() { ctiNode->getSchemaName(), ctiNode->getFields(), ctiNode->getIfNotExists(), - ctiNode->getIndexParams()) + ctiNode->getIndexParams(), + ctiNode->getComment()) .via(runner()) .thenValue([ctiNode, spaceId](StatusOr resp) { if (!resp.ok()) { diff --git a/src/graph/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp b/src/graph/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp index b9d7c73f56d..3729afc85b2 100644 --- a/src/graph/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp +++ b/src/graph/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp @@ -89,12 +89,14 @@ StatusOr GeoPredicateIndexScanBaseRule::transform( geoColumnTypeDef.geo_shape_ref().value() == meta::cpp2::GeoShape::POINT; geo::RegionCoverParams rc; - const auto& indexParams = indexItem->get_index_params(); - if (indexParams.s2_max_level_ref().has_value()) { - rc.maxCellLevel_ = indexParams.s2_max_level_ref().value(); - } - if (indexParams.s2_max_cells_ref().has_value()) { - rc.maxCellNum_ = indexParams.s2_max_level_ref().value(); + const auto* indexParams = indexItem->get_index_params(); + if (indexParams) { + if (indexParams->s2_max_level_ref().has_value()) { + rc.maxCellLevel_ = indexParams->s2_max_level_ref().value(); + } + if (indexParams->s2_max_cells_ref().has_value()) { + rc.maxCellNum_ = indexParams->s2_max_cells_ref().value(); + } } geo::GeoIndex geoIndex(rc, isPointColumn); std::vector scanRanges; diff --git a/src/graph/planner/plan/Maintain.cpp b/src/graph/planner/plan/Maintain.cpp index 10327c22955..4a108ce1cc0 100644 --- a/src/graph/planner/plan/Maintain.cpp +++ b/src/graph/planner/plan/Maintain.cpp @@ -52,7 +52,9 @@ std::unique_ptr CreateIndexNode::explain() const { } addDescription("fields", folly::toJson(util::toJson(fields)), desc.get()); addDescription("ifNotExists", folly::to(ifNotExists_), desc.get()); - addDescription("indexParams", folly::toJson(util::toJson(indexParams_)), desc.get()); + if (indexParams_) { + addDescription("indexParams", folly::toJson(util::toJson(*indexParams_)), desc.get()); + } return desc; } diff --git a/src/graph/planner/plan/Maintain.h b/src/graph/planner/plan/Maintain.h index 9f014c854df..988be1e51bd 100644 --- a/src/graph/planner/plan/Maintain.h +++ b/src/graph/planner/plan/Maintain.h @@ -300,14 +300,14 @@ class CreateIndexNode : public SingleDependencyNode { std::string indexName, std::vector fields, bool ifNotExists, - meta::cpp2::IndexParams indexParams, + std::unique_ptr indexParams, const std::string* comment) : SingleDependencyNode(qctx, kind, input), schemaName_(std::move(schemaName)), indexName_(std::move(indexName)), fields_(std::move(fields)), ifNotExists_(ifNotExists), - indexParams_(indexParams), + indexParams_(std::move(indexParams)), comment_(comment) {} public: @@ -319,7 +319,7 @@ class CreateIndexNode : public SingleDependencyNode { bool getIfNotExists() const { return ifNotExists_; } - meta::cpp2::IndexParams getIndexParams() const { return indexParams_; } + const meta::cpp2::IndexParams* getIndexParams() const { return indexParams_.get(); } const std::string* getComment() const { return comment_; } @@ -330,7 +330,7 @@ class CreateIndexNode : public SingleDependencyNode { std::string indexName_; std::vector fields_; bool ifNotExists_; - meta::cpp2::IndexParams indexParams_; + std::unique_ptr indexParams_; const std::string* comment_; }; @@ -342,7 +342,7 @@ class CreateTagIndex final : public CreateIndexNode { std::string indexName, std::vector fields, bool ifNotExists, - meta::cpp2::IndexParams indexParams, + std::unique_ptr indexParams, const std::string* comment) { return qctx->objPool()->add(new CreateTagIndex(qctx, input, @@ -350,7 +350,7 @@ class CreateTagIndex final : public CreateIndexNode { std::move(indexName), std::move(fields), ifNotExists, - indexParams, + std::move(indexParams), comment)); } @@ -361,7 +361,7 @@ class CreateTagIndex final : public CreateIndexNode { std::string indexName, std::vector fields, bool ifNotExists, - meta::cpp2::IndexParams indexParams, + std::unique_ptr indexParams, const std::string* comment) : CreateIndexNode(qctx, input, @@ -370,7 +370,7 @@ class CreateTagIndex final : public CreateIndexNode { std::move(indexName), std::move(fields), ifNotExists, - indexParams, + std::move(indexParams), comment) {} }; @@ -382,7 +382,7 @@ class CreateEdgeIndex final : public CreateIndexNode { std::string indexName, std::vector fields, bool ifNotExists, - meta::cpp2::IndexParams indexParams, + std::unique_ptr indexParams, const std::string* comment) { return qctx->objPool()->add(new CreateEdgeIndex(qctx, input, @@ -390,7 +390,7 @@ class CreateEdgeIndex final : public CreateIndexNode { std::move(indexName), std::move(fields), ifNotExists, - indexParams, + std::move(indexParams), comment)); } @@ -401,7 +401,7 @@ class CreateEdgeIndex final : public CreateIndexNode { std::string indexName, std::vector fields, bool ifNotExists, - meta::cpp2::IndexParams indexParams, + std::unique_ptr indexParams, const std::string* comment) : CreateIndexNode(qctx, input, @@ -410,7 +410,7 @@ class CreateEdgeIndex final : public CreateIndexNode { std::move(indexName), std::move(fields), ifNotExists, - indexParams, + std::move(indexParams), comment) {} }; diff --git a/src/graph/util/IndexUtil.cpp b/src/graph/util/IndexUtil.cpp index b2e0da624c1..67f2e7c0a50 100644 --- a/src/graph/util/IndexUtil.cpp +++ b/src/graph/util/IndexUtil.cpp @@ -87,25 +87,31 @@ StatusOr IndexUtil::toShowCreateIndex(bool isTagIndex, createStr += "\n"; } createStr += ")"; - if (indexItem.comment_ref().has_value()) { - createStr += " comment = \""; - createStr += *indexItem.get_comment(); - createStr += "\""; - } - const auto &indexParams = indexItem.get_index_params(); + + const auto *indexParams = indexItem.get_index_params(); std::vector params; - if (indexParams.s2_max_level_ref().has_value()) { - params.emplace_back("s2_max_level = " + std::to_string(*indexParams.s2_max_level_ref())); - } - if (indexParams.s2_max_cells_ref().has_value()) { - params.emplace_back("s2_max_cells = " + std::to_string(*indexParams.s2_max_cells_ref())); + if (indexParams) { + if (indexParams->s2_max_level_ref().has_value()) { + params.emplace_back("s2_max_level = " + + std::to_string(indexParams->s2_max_level_ref().value())); + } + if (indexParams->s2_max_cells_ref().has_value()) { + params.emplace_back("s2_max_cells = " + + std::to_string(indexParams->s2_max_cells_ref().value())); + } } if (!params.empty()) { - createStr += " WITH("; + createStr += " WITH ("; createStr += folly::join(", ", params); createStr += ")"; } + if (indexItem.comment_ref().has_value()) { + createStr += " comment \""; + createStr += *indexItem.get_comment(); + createStr += "\""; + } + row.emplace_back(std::move(createStr)); dataSet.rows.emplace_back(std::move(row)); return dataSet; diff --git a/src/graph/validator/MaintainValidator.cpp b/src/graph/validator/MaintainValidator.cpp index bcc876f197e..56a906f15e3 100644 --- a/src/graph/validator/MaintainValidator.cpp +++ b/src/graph/validator/MaintainValidator.cpp @@ -5,6 +5,8 @@ #include "graph/validator/MaintainValidator.h" +#include + #include "common/base/Status.h" #include "common/charset/Charset.h" #include "common/expression/ConstantExpression.h" @@ -16,6 +18,7 @@ #include "graph/util/FTIndexUtils.h" #include "graph/util/IndexUtil.h" #include "graph/util/SchemaUtil.h" +#include "interface/gen-cpp2/meta_types.h" #include "parser/MaintainSentences.h" namespace nebula { @@ -295,10 +298,11 @@ Status CreateTagIndexValidator::validateImpl() { index_ = *sentence->indexName(); fields_ = sentence->fields(); ifNotExist_ = sentence->isIfNotExist(); - // TODO(darion) Save the index auto *indexParamList = sentence->getIndexParamList(); if (indexParamList) { - NG_RETURN_IF_ERROR(IndexUtil::validateIndexParams(indexParamList->getParams(), indexParams_)); + meta::cpp2::IndexParams indexParams; + NG_RETURN_IF_ERROR(IndexUtil::validateIndexParams(indexParamList->getParams(), indexParams)); + indexParams_ = std::make_unique(std::move(indexParams)); } return Status::OK(); } @@ -311,7 +315,7 @@ Status CreateTagIndexValidator::toPlan() { *sentence->indexName(), sentence->fields(), sentence->isIfNotExist(), - indexParams_, + std::move(indexParams_), sentence->comment()); root_ = doNode; tail_ = root_; @@ -327,7 +331,9 @@ Status CreateEdgeIndexValidator::validateImpl() { // TODO(darion) Save the index auto *indexParamList = sentence->getIndexParamList(); if (indexParamList) { - NG_RETURN_IF_ERROR(IndexUtil::validateIndexParams(indexParamList->getParams(), indexParams_)); + meta::cpp2::IndexParams indexParams; + NG_RETURN_IF_ERROR(IndexUtil::validateIndexParams(indexParamList->getParams(), indexParams)); + indexParams_ = std::make_unique(std::move(indexParams)); } return Status::OK(); } @@ -340,7 +346,7 @@ Status CreateEdgeIndexValidator::toPlan() { *sentence->indexName(), sentence->fields(), sentence->isIfNotExist(), - indexParams_, + std::move(indexParams_), sentence->comment()); root_ = doNode; tail_ = root_; diff --git a/src/graph/validator/MaintainValidator.h b/src/graph/validator/MaintainValidator.h index 0e1f912b918..508dd2c1598 100644 --- a/src/graph/validator/MaintainValidator.h +++ b/src/graph/validator/MaintainValidator.h @@ -159,7 +159,7 @@ class CreateTagIndexValidator final : public Validator { std::string index_; std::vector fields_; bool ifNotExist_; - meta::cpp2::IndexParams indexParams_; + std::unique_ptr indexParams_; }; class CreateEdgeIndexValidator final : public Validator { @@ -176,7 +176,7 @@ class CreateEdgeIndexValidator final : public Validator { std::string index_; std::vector fields_; bool ifNotExist_; - meta::cpp2::IndexParams indexParams_; + std::unique_ptr indexParams_; }; class DropTagIndexValidator final : public Validator { diff --git a/src/interface/meta.thrift b/src/interface/meta.thrift index 44eded148c2..931b014cf58 100644 --- a/src/interface/meta.thrift +++ b/src/interface/meta.thrift @@ -147,13 +147,13 @@ struct IndexParams { } struct IndexItem { - 1: common.IndexID index_id, - 2: binary index_name, - 3: common.SchemaID schema_id - 4: binary schema_name, - 5: list fields, - 6: optional binary comment, - 7: IndexParams index_params, + 1: common.IndexID index_id, + 2: binary index_name, + 3: common.SchemaID schema_id + 4: binary schema_name, + 5: list fields, + 6: optional binary comment, + 7: optional IndexParams index_params, } enum HostStatus { @@ -597,7 +597,7 @@ struct CreateTagIndexReq { 4: list fields, 5: bool if_not_exists, 6: optional binary comment, - 7: IndexParams index_params, + 7: optional IndexParams index_params, } struct DropTagIndexReq { @@ -634,7 +634,7 @@ struct CreateEdgeIndexReq { 4: list fields, 5: bool if_not_exists, 6: optional binary comment, - 7: IndexParams index_params, + 7: optional IndexParams index_params, } struct DropEdgeIndexReq { diff --git a/src/meta/processors/index/CreateEdgeIndexProcessor.cpp b/src/meta/processors/index/CreateEdgeIndexProcessor.cpp index 08d3a01aafc..b8a3451d57c 100644 --- a/src/meta/processors/index/CreateEdgeIndexProcessor.cpp +++ b/src/meta/processors/index/CreateEdgeIndexProcessor.cpp @@ -190,7 +190,9 @@ void CreateEdgeIndexProcessor::process(const cpp2::CreateEdgeIndexReq& req) { item.set_schema_id(schemaID); item.set_schema_name(edgeName); item.set_fields(std::move(columns)); - item.set_index_params(*req.index_params_ref()); + if (req.index_params_ref().has_value()) { + item.set_index_params(*req.index_params_ref()); + } if (req.comment_ref().has_value()) { item.set_comment(*req.comment_ref()); } diff --git a/src/meta/processors/index/CreateTagIndexProcessor.cpp b/src/meta/processors/index/CreateTagIndexProcessor.cpp index 21cdfec9f3f..6c094d0cfff 100644 --- a/src/meta/processors/index/CreateTagIndexProcessor.cpp +++ b/src/meta/processors/index/CreateTagIndexProcessor.cpp @@ -188,7 +188,9 @@ void CreateTagIndexProcessor::process(const cpp2::CreateTagIndexReq& req) { item.set_schema_id(schemaID); item.set_schema_name(tagName); item.set_fields(std::move(columns)); - item.set_index_params(*req.index_params_ref()); + if (req.index_params_ref().has_value()) { + item.set_index_params(*req.index_params_ref()); + } if (req.comment_ref().has_value()) { item.set_comment(*req.comment_ref()); } diff --git a/src/parser/MaintainSentences.cpp b/src/parser/MaintainSentences.cpp index d7351c45f22..ac70707fdf9 100644 --- a/src/parser/MaintainSentences.cpp +++ b/src/parser/MaintainSentences.cpp @@ -280,13 +280,19 @@ std::string CreateTagIndexSentence::toString() const { folly::join(", ", fieldDefs, fields); buf += fields; buf += ")"; + std::string params; + if (indexParams_ != nullptr) { + params = indexParams_->toString(); + } + if (!params.empty()) { + buf += "WITH ("; + buf += params; + buf += ")"; + } if (comment_ != nullptr) { - buf += "COMMENT = "; + buf += "COMMENT "; buf += *comment_; } - if (indexParams_ != nullptr) { - buf += indexParams_->toString(); - } return buf; } @@ -313,13 +319,19 @@ std::string CreateEdgeIndexSentence::toString() const { folly::join(", ", fieldDefs, fields); buf += fields; buf += ")"; + std::string params; + if (indexParams_ != nullptr) { + params = indexParams_->toString(); + } + if (!params.empty()) { + buf += "WITH ("; + buf += params; + buf += ")"; + } if (comment_ != nullptr) { - buf += "COMMENT = "; + buf += "COMMENT "; buf += *comment_; } - if (indexParams_ != nullptr) { - buf += indexParams_->toString(); - } return buf; } diff --git a/src/parser/MaintainSentences.h b/src/parser/MaintainSentences.h index 48619a5e87f..6b620d9a6ed 100644 --- a/src/parser/MaintainSentences.h +++ b/src/parser/MaintainSentences.h @@ -578,7 +578,7 @@ class CreateTagIndexSentence final : public CreateSentence { return result; } - IndexParamList *getIndexParamList() const { return indexParams_.get(); } + const IndexParamList *getIndexParamList() const { return indexParams_.get(); } const std::string *comment() const { return comment_.get(); } @@ -626,7 +626,7 @@ class CreateEdgeIndexSentence final : public CreateSentence { return result; } - IndexParamList *getIndexParamList() const { return indexParams_.get(); } + const IndexParamList *getIndexParamList() const { return indexParams_.get(); } const std::string *comment() const { return comment_.get(); } diff --git a/tests/tck/features/geo/GeoBase.feature b/tests/tck/features/geo/GeoBase.feature index f9c3e5cfa2a..a0a24e2203d 100644 --- a/tests/tck/features/geo/GeoBase.feature +++ b/tests/tck/features/geo/GeoBase.feature @@ -250,8 +250,8 @@ Feature: Geo base SHOW CREATE TAG INDEX any_shape_geo_index; """ Then the result should be, in any order: - | Tag Index Name | Create Tag Index | - | "any_shape_geo_index" | "CREATE TAG INDEX `any_shape_geo_index` ON `any_shape` (\n `geo`\n) WITH (s2_max_level = 30, s2_max_cells = 8) comment = \"test\"" | + | Tag Index Name | Create Tag Index | + | "any_shape_geo_index" | "CREATE TAG INDEX `any_shape_geo_index` ON `any_shape` (\n `geo`\n) WITH (s2_max_level = 30, s2_max_cells = 8) comment \"test\"" | # Rebuild the geo index When submit a job: """ diff --git a/tests/tck/features/schema/Comment.feature b/tests/tck/features/schema/Comment.feature index 638478bed77..94c4ecc8911 100644 --- a/tests/tck/features/schema/Comment.feature +++ b/tests/tck/features/schema/Comment.feature @@ -148,7 +148,7 @@ Feature: Schema Comment When executing query: """ CREATE tag index test_comment_tag_index ON test_comment_tag(name(8)) - comment = 'The tag index of person name.'; + comment 'The tag index of person name.'; """ Then the execution should be successful When executing query: @@ -156,8 +156,8 @@ Feature: Schema Comment SHOW CREATE TAG INDEX test_comment_tag_index; """ Then the result should be, in any order: - | Tag Index Name | Create Tag Index | - | "test_comment_tag_index" | 'CREATE TAG INDEX `test_comment_tag_index` ON `test_comment_tag` (\n `name`(8)\n) comment = "The tag index of person name."' | + | Tag Index Name | Create Tag Index | + | "test_comment_tag_index" | 'CREATE TAG INDEX `test_comment_tag_index` ON `test_comment_tag` (\n `name`(8)\n) comment "The tag index of person name."' | # edge When executing query: """ @@ -212,7 +212,7 @@ Feature: Schema Comment When executing query: """ CREATE edge index test_comment_edge_index ON test_comment_edge(name(8)) - comment = 'The edge index of person name.'; + comment 'The edge index of person name.'; """ Then the execution should be successful When executing query: @@ -220,8 +220,8 @@ Feature: Schema Comment SHOW CREATE EDGE INDEX test_comment_edge_index; """ Then the result should be, in any order: - | Edge Index Name | Create Edge Index | - | "test_comment_edge_index" | 'CREATE EDGE INDEX `test_comment_edge_index` ON `test_comment_edge` (\n `name`(8)\n) comment = "The edge index of person name."' | + | Edge Index Name | Create Edge Index | + | "test_comment_edge_index" | 'CREATE EDGE INDEX `test_comment_edge_index` ON `test_comment_edge` (\n `name`(8)\n) comment "The edge index of person name."' | Examples: | tag_of_person_comment | tag_of_person_comment_modified | edge_of_person_comment | edge_of_person_comment_modified |