Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@jievince
jievince committed Dec 21, 2021
1 parent 9663de9 commit 267f874
Show file tree
Hide file tree
Showing 18 changed files with 127 additions and 88 deletions.
12 changes: 8 additions & 4 deletions src/clients/meta/MetaClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1714,15 +1714,17 @@ folly::Future<StatusOr<IndexID>> MetaClient::createTagIndex(GraphSpaceID spaceID
std::string tagName,
std::vector<cpp2::IndexFieldDef> fields,
bool ifNotExists,
cpp2::IndexParams indexParams,
const cpp2::IndexParams* indexParams,
const std::string* comment) {
cpp2::CreateTagIndexReq req;
req.set_space_id(spaceID);
req.set_index_name(std::move(indexName));
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);
}
Expand Down Expand Up @@ -1828,15 +1830,17 @@ folly::Future<StatusOr<IndexID>> MetaClient::createEdgeIndex(
std::string edgeName,
std::vector<cpp2::IndexFieldDef> fields,
bool ifNotExists,
cpp2::IndexParams indexParams,
const cpp2::IndexParams* indexParams,
const std::string* comment) {
cpp2::CreateEdgeIndexReq req;
req.set_space_id(spaceID);
req.set_index_name(std::move(indexName));
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);
}
Expand Down
17 changes: 8 additions & 9 deletions src/clients/meta/MetaClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ class MetaClient {
std::string tagName,
std::vector<cpp2::IndexFieldDef> 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
Expand All @@ -333,14 +333,13 @@ class MetaClient {

folly::Future<StatusOr<std::vector<cpp2::IndexStatus>>> listTagIndexStatus(GraphSpaceID spaceId);

folly::Future<StatusOr<IndexID>> createEdgeIndex(
GraphSpaceID spaceID,
std::string indexName,
std::string edgeName,
std::vector<cpp2::IndexFieldDef> fields,
bool ifNotExists = false,
cpp2::IndexParams indexParams = cpp2::IndexParams{},
const std::string* comment = nullptr);
folly::Future<StatusOr<IndexID>> createEdgeIndex(GraphSpaceID spaceID,
std::string indexName,
std::string edgeName,
std::vector<cpp2::IndexFieldDef> fields,
bool ifNotExists = false,
const cpp2::IndexParams* indexParams = nullptr,
const std::string* comment = nullptr);

// Remove the definition of edge index
folly::Future<StatusOr<bool>> dropEdgeIndex(GraphSpaceID spaceId,
Expand Down
14 changes: 8 additions & 6 deletions src/common/utils/IndexKeyUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,14 @@ std::vector<std::string> IndexKeyUtils::encodeValues(std::vector<Value>&& 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 {
Expand Down
3 changes: 2 additions & 1 deletion src/graph/executor/maintain/EdgeIndexExecutor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ folly::Future<Status> CreateEdgeIndexExecutor::execute() {
ceiNode->getSchemaName(),
ceiNode->getFields(),
ceiNode->getIfNotExists(),
ceiNode->getIndexParams())
ceiNode->getIndexParams(),
ceiNode->getComment())
.via(runner())
.thenValue([ceiNode, spaceId](StatusOr<IndexID> resp) {
if (!resp.ok()) {
Expand Down
3 changes: 2 additions & 1 deletion src/graph/executor/maintain/TagIndexExecutor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ folly::Future<Status> CreateTagIndexExecutor::execute() {
ctiNode->getSchemaName(),
ctiNode->getFields(),
ctiNode->getIfNotExists(),
ctiNode->getIndexParams())
ctiNode->getIndexParams(),
ctiNode->getComment())
.via(runner())
.thenValue([ctiNode, spaceId](StatusOr<IndexID> resp) {
if (!resp.ok()) {
Expand Down
14 changes: 8 additions & 6 deletions src/graph/optimizer/rule/GeoPredicateIndexScanBaseRule.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,14 @@ StatusOr<TransformResult> 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<geo::ScanRange> scanRanges;
Expand Down
4 changes: 3 additions & 1 deletion src/graph/planner/plan/Maintain.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ std::unique_ptr<PlanNodeDescription> CreateIndexNode::explain() const {
}
addDescription("fields", folly::toJson(util::toJson(fields)), desc.get());
addDescription("ifNotExists", folly::to<std::string>(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;
}

Expand Down
24 changes: 12 additions & 12 deletions src/graph/planner/plan/Maintain.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,14 +300,14 @@ class CreateIndexNode : public SingleDependencyNode {
std::string indexName,
std::vector<meta::cpp2::IndexFieldDef> fields,
bool ifNotExists,
meta::cpp2::IndexParams indexParams,
std::unique_ptr<meta::cpp2::IndexParams> 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:
Expand All @@ -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_; }

Expand All @@ -330,7 +330,7 @@ class CreateIndexNode : public SingleDependencyNode {
std::string indexName_;
std::vector<meta::cpp2::IndexFieldDef> fields_;
bool ifNotExists_;
meta::cpp2::IndexParams indexParams_;
std::unique_ptr<meta::cpp2::IndexParams> indexParams_;
const std::string* comment_;
};

Expand All @@ -342,15 +342,15 @@ class CreateTagIndex final : public CreateIndexNode {
std::string indexName,
std::vector<meta::cpp2::IndexFieldDef> fields,
bool ifNotExists,
meta::cpp2::IndexParams indexParams,
std::unique_ptr<meta::cpp2::IndexParams> indexParams,
const std::string* comment) {
return qctx->objPool()->add(new CreateTagIndex(qctx,
input,
std::move(tagName),
std::move(indexName),
std::move(fields),
ifNotExists,
indexParams,
std::move(indexParams),
comment));
}

Expand All @@ -361,7 +361,7 @@ class CreateTagIndex final : public CreateIndexNode {
std::string indexName,
std::vector<meta::cpp2::IndexFieldDef> fields,
bool ifNotExists,
meta::cpp2::IndexParams indexParams,
std::unique_ptr<meta::cpp2::IndexParams> indexParams,
const std::string* comment)
: CreateIndexNode(qctx,
input,
Expand All @@ -370,7 +370,7 @@ class CreateTagIndex final : public CreateIndexNode {
std::move(indexName),
std::move(fields),
ifNotExists,
indexParams,
std::move(indexParams),
comment) {}
};

Expand All @@ -382,15 +382,15 @@ class CreateEdgeIndex final : public CreateIndexNode {
std::string indexName,
std::vector<meta::cpp2::IndexFieldDef> fields,
bool ifNotExists,
meta::cpp2::IndexParams indexParams,
std::unique_ptr<meta::cpp2::IndexParams> indexParams,
const std::string* comment) {
return qctx->objPool()->add(new CreateEdgeIndex(qctx,
input,
std::move(edgeName),
std::move(indexName),
std::move(fields),
ifNotExists,
indexParams,
std::move(indexParams),
comment));
}

Expand All @@ -401,7 +401,7 @@ class CreateEdgeIndex final : public CreateIndexNode {
std::string indexName,
std::vector<meta::cpp2::IndexFieldDef> fields,
bool ifNotExists,
meta::cpp2::IndexParams indexParams,
std::unique_ptr<meta::cpp2::IndexParams> indexParams,
const std::string* comment)
: CreateIndexNode(qctx,
input,
Expand All @@ -410,7 +410,7 @@ class CreateEdgeIndex final : public CreateIndexNode {
std::move(indexName),
std::move(fields),
ifNotExists,
indexParams,
std::move(indexParams),
comment) {}
};

Expand Down
30 changes: 18 additions & 12 deletions src/graph/util/IndexUtil.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,25 +87,31 @@ StatusOr<DataSet> 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<std::string> 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;
Expand Down
16 changes: 11 additions & 5 deletions src/graph/validator/MaintainValidator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

#include "graph/validator/MaintainValidator.h"

#include <memory>

#include "common/base/Status.h"
#include "common/charset/Charset.h"
#include "common/expression/ConstantExpression.h"
Expand All @@ -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 {
Expand Down Expand Up @@ -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<meta::cpp2::IndexParams>(std::move(indexParams));
}
return Status::OK();
}
Expand All @@ -311,7 +315,7 @@ Status CreateTagIndexValidator::toPlan() {
*sentence->indexName(),
sentence->fields(),
sentence->isIfNotExist(),
indexParams_,
std::move(indexParams_),
sentence->comment());
root_ = doNode;
tail_ = root_;
Expand All @@ -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<meta::cpp2::IndexParams>(std::move(indexParams));
}
return Status::OK();
}
Expand All @@ -340,7 +346,7 @@ Status CreateEdgeIndexValidator::toPlan() {
*sentence->indexName(),
sentence->fields(),
sentence->isIfNotExist(),
indexParams_,
std::move(indexParams_),
sentence->comment());
root_ = doNode;
tail_ = root_;
Expand Down
4 changes: 2 additions & 2 deletions src/graph/validator/MaintainValidator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ class CreateTagIndexValidator final : public Validator {
std::string index_;
std::vector<meta::cpp2::IndexFieldDef> fields_;
bool ifNotExist_;
meta::cpp2::IndexParams indexParams_;
std::unique_ptr<meta::cpp2::IndexParams> indexParams_;
};

class CreateEdgeIndexValidator final : public Validator {
Expand All @@ -176,7 +176,7 @@ class CreateEdgeIndexValidator final : public Validator {
std::string index_;
std::vector<meta::cpp2::IndexFieldDef> fields_;
bool ifNotExist_;
meta::cpp2::IndexParams indexParams_;
std::unique_ptr<meta::cpp2::IndexParams> indexParams_;
};

class DropTagIndexValidator final : public Validator {
Expand Down
Loading

0 comments on commit 267f874

Please sign in to comment.