From eaf5cdb904185a3cb4de992281abfaedc53b4461 Mon Sep 17 00:00:00 2001 From: jimingquan Date: Tue, 7 Dec 2021 20:12:07 +0800 Subject: [PATCH 1/5] support properties in where clause --- src/graph/validator/GoValidator.cpp | 84 ++++--------------- src/graph/validator/GoValidator.h | 4 +- src/graph/visitor/DeducePropsVisitor.cpp | 24 +++++- src/graph/visitor/DeducePropsVisitor.h | 1 - src/graph/visitor/ExtractPropExprVisitor.cpp | 23 ++++- src/graph/visitor/ExtractPropExprVisitor.h | 4 +- tests/tck/features/go/GO.feature | 1 + .../tck/features/go/GoYieldVertexEdge.feature | 1 + tests/tck/features/go/GroupbyLimit.feature | 1 + 9 files changed, 65 insertions(+), 78 deletions(-) diff --git a/src/graph/validator/GoValidator.cpp b/src/graph/validator/GoValidator.cpp index 2f2bb84fa3d..af0c206a19b 100644 --- a/src/graph/validator/GoValidator.cpp +++ b/src/graph/validator/GoValidator.cpp @@ -15,9 +15,8 @@ namespace nebula { namespace graph { -static const char* COLNAME_EDGE = "EDGE"; -static const char* SRC_VERTEX = "$^"; -static const char* DST_VERTEX = "$$"; +// static const char* SRC_VERTEX = "$^"; +// static const char* DST_VERTEX = "$$"; Status GoValidator::validateImpl() { auto* goSentence = static_cast(sentence_); @@ -140,17 +139,9 @@ Status GoValidator::validateYield(YieldClause* yield) { return Status::SemanticError("`%s' is not support in go sentence.", col->toString().c_str()); } - const auto& vertexExprs = ExpressionUtils::collectAll(col->expr(), {Expression::Kind::kVertex}); - for (const auto* expr : vertexExprs) { - const auto& colName = static_cast(expr)->name(); - if (colName == SRC_VERTEX) { - NG_RETURN_IF_ERROR(extractVertexProp(exprProps, true)); - } else if (colName == DST_VERTEX) { - NG_RETURN_IF_ERROR(extractVertexProp(exprProps, false)); - } else { - return Status::SemanticError("`%s' is not support in go sentence.", - col->toString().c_str()); - } + auto vertexExpr = ExpressionUtils::findAny(col->expr(), {Expression::Kind::kVertex}); + if (static_cast(vertexExpr)->name() == "VERTEX") { + return Status::SemanticError("`%s' is not support in go sentence.", col->toString().c_str()); } col->setExpr(ExpressionUtils::rewriteLabelAttr2EdgeProp(col->expr())); @@ -180,25 +171,6 @@ Status GoValidator::validateYield(YieldClause* yield) { return Status::OK(); } -Status GoValidator::extractVertexProp(ExpressionProps& exprProps, bool isSrc) { - const auto tagStatus = qctx_->schemaMng()->getAllLatestVerTagSchema(space_.id); - NG_RETURN_IF_ERROR(tagStatus); - for (const auto& tag : tagStatus.value()) { - auto tagID = tag.first; - const auto& tagSchema = tag.second; - if (isSrc) { - for (size_t i = 0; i < tagSchema->getNumFields(); ++i) { - exprProps.insertSrcTagProp(tagID, tagSchema->getFieldName(i)); - } - } else { - for (size_t i = 0; i < tagSchema->getNumFields(); ++i) { - exprProps.insertDstTagProp(tagID, tagSchema->getFieldName(i)); - } - } - } - return Status::OK(); -} - Status GoValidator::extractEdgeProp(ExpressionProps& exprProps) { const auto& edgeTypes = goCtx_->over.edgeTypes; for (const auto& edgeType : edgeTypes) { @@ -214,9 +186,14 @@ Status GoValidator::extractEdgeProp(ExpressionProps& exprProps) { return Status::OK(); } -void GoValidator::extractPropExprs(const Expression* expr) { - ExtractPropExprVisitor visitor( - vctx_, goCtx_->srcEdgePropsExpr, goCtx_->dstPropsExpr, inputPropCols_, propExprColMap_); +void GoValidator::extractPropExprs(const Expression* expr, + std::unordered_set& uniqueExpr) { + ExtractPropExprVisitor visitor(vctx_, + goCtx_->srcEdgePropsExpr, + goCtx_->dstPropsExpr, + inputPropCols_, + propExprColMap_, + uniqueExpr); const_cast(expr)->accept(&visitor); } @@ -258,42 +235,17 @@ Status GoValidator::buildColumns() { inputPropCols_ = pool->add(new YieldColumns()); } + std::unordered_set uniqueEdgeVertexExpr; auto filter = goCtx_->filter; if (filter != nullptr) { - extractPropExprs(filter); - auto newFilter = filter->clone(); - goCtx_->filter = rewrite2VarProp(newFilter); + extractPropExprs(filter, uniqueEdgeVertexExpr); + goCtx_->filter = rewrite2VarProp(filter); } - std::unordered_set existExpr; auto* newYieldExpr = pool->add(new YieldColumns()); for (auto* col : goCtx_->yieldExpr->columns()) { - const auto& vertexExprs = ExpressionUtils::collectAll(col->expr(), {Expression::Kind::kVertex}); - auto existEdge = ExpressionUtils::hasAny(col->expr(), {Expression::Kind::kEdge}); - if (!existEdge && vertexExprs.empty()) { - extractPropExprs(col->expr()); - newYieldExpr->addColumn(new YieldColumn(rewrite2VarProp(col->expr()), col->alias())); - } else { - if (existEdge) { - if (existExpr.emplace(COLNAME_EDGE).second) { - goCtx_->srcEdgePropsExpr->addColumn( - new YieldColumn(EdgeExpression::make(pool), COLNAME_EDGE)); - } - } - for (const auto* expr : vertexExprs) { - const auto& colName = static_cast(expr)->name(); - if (existExpr.emplace(colName).second) { - if (colName == SRC_VERTEX) { - goCtx_->srcEdgePropsExpr->addColumn( - new YieldColumn(VertexExpression::make(pool), SRC_VERTEX)); - } else { - goCtx_->dstPropsExpr->addColumn( - new YieldColumn(VertexExpression::make(pool), DST_VERTEX)); - } - } - } - newYieldExpr->addColumn(col->clone().release()); - } + extractPropExprs(col->expr(), uniqueEdgeVertexExpr); + newYieldExpr->addColumn(new YieldColumn(rewrite2VarProp(col->expr()), col->alias())); } goCtx_->yieldExpr = newYieldExpr; return Status::OK(); diff --git a/src/graph/validator/GoValidator.h b/src/graph/validator/GoValidator.h index 3c42ec13023..fb034797705 100644 --- a/src/graph/validator/GoValidator.h +++ b/src/graph/validator/GoValidator.h @@ -32,12 +32,10 @@ class GoValidator final : public Validator { Status buildColumns(); - void extractPropExprs(const Expression* expr); + void extractPropExprs(const Expression* expr, std::unordered_set& uniqueCols); Expression* rewrite2VarProp(const Expression* expr); - Status extractVertexProp(ExpressionProps& exprProps, bool isSrc); - Status extractEdgeProp(ExpressionProps& exprProps); private: diff --git a/src/graph/visitor/DeducePropsVisitor.cpp b/src/graph/visitor/DeducePropsVisitor.cpp index a9ac325364f..8991932a95d 100644 --- a/src/graph/visitor/DeducePropsVisitor.cpp +++ b/src/graph/visitor/DeducePropsVisitor.cpp @@ -173,13 +173,31 @@ void DeducePropsVisitor::visit(VersionedVariableExpression *expr) { reportError( void DeducePropsVisitor::visit(LabelExpression *expr) { reportError(expr); } -void DeducePropsVisitor::visit(AttributeExpression *expr) { UNUSED(expr); } - void DeducePropsVisitor::visit(LabelAttributeExpression *expr) { reportError(expr); } void DeducePropsVisitor::visit(ConstantExpression *expr) { UNUSED(expr); } -void DeducePropsVisitor::visit(VertexExpression *expr) { UNUSED(expr); } +void DeducePropsVisitor::visit(VertexExpression *expr) { + const auto &colName = expr->name(); + auto tagStatus = qctx_->schemaMng()->getAllLatestVerTagSchema(space_); + if (!tagStatus.ok()) { + status_ = std::move(tagStatus).status(); + return; + } + for (const auto &tag : tagStatus.value()) { + auto tagID = tag.first; + const auto &tagSchema = tag.second; + if (colName == "$^") { + for (size_t i = 0; i < tagSchema->getNumFields(); ++i) { + exprProps_->insertSrcTagProp(tagID, tagSchema->getFieldName(i)); + } + } else if (colName == "$$") { + for (size_t i = 0; i < tagSchema->getNumFields(); ++i) { + exprProps_->insertDstTagProp(tagID, tagSchema->getFieldName(i)); + } + } + } +} void DeducePropsVisitor::visit(EdgeExpression *expr) { UNUSED(expr); } diff --git a/src/graph/visitor/DeducePropsVisitor.h b/src/graph/visitor/DeducePropsVisitor.h index bf71f25d7b3..05cdb5e4a4d 100644 --- a/src/graph/visitor/DeducePropsVisitor.h +++ b/src/graph/visitor/DeducePropsVisitor.h @@ -93,7 +93,6 @@ class DeducePropsVisitor : public ExprVisitorImpl { void visit(VariableExpression* expr) override; void visit(VersionedVariableExpression* expr) override; void visit(LabelExpression* expr) override; - void visit(AttributeExpression* expr) override; void visit(LabelAttributeExpression* expr) override; void visit(ConstantExpression* expr) override; void visit(VertexExpression* expr) override; diff --git a/src/graph/visitor/ExtractPropExprVisitor.cpp b/src/graph/visitor/ExtractPropExprVisitor.cpp index 52c3cbd1b08..1aea3c3af8e 100644 --- a/src/graph/visitor/ExtractPropExprVisitor.cpp +++ b/src/graph/visitor/ExtractPropExprVisitor.cpp @@ -13,18 +13,33 @@ ExtractPropExprVisitor::ExtractPropExprVisitor( YieldColumns* srcAndEdgePropCols, YieldColumns* dstPropCols, YieldColumns* inputPropCols, - std::unordered_map& propExprColMap) + std::unordered_map& propExprColMap, + std::unordered_set& uniqueEdgeVertexCol) : vctx_(DCHECK_NOTNULL(vctx)), srcAndEdgePropCols_(srcAndEdgePropCols), dstPropCols_(dstPropCols), inputPropCols_(inputPropCols), - propExprColMap_(propExprColMap) {} + propExprColMap_(propExprColMap), + uniqueEdgeVertexCol_(uniqueEdgeVertexCol) {} void ExtractPropExprVisitor::visit(ConstantExpression* expr) { UNUSED(expr); } -void ExtractPropExprVisitor::visit(VertexExpression* expr) { UNUSED(expr); } +void ExtractPropExprVisitor::visit(VertexExpression* expr) { + const auto& colName = expr->name(); + if (uniqueEdgeVertexCol_.emplace(colName).second) { + if (colName == "$^") { + srcAndEdgePropCols_->addColumn(new YieldColumn(expr->clone(), colName)); + } else { + dstPropCols_->addColumn(new YieldColumn(expr->clone(), colName)); + } + } +} -void ExtractPropExprVisitor::visit(EdgeExpression* expr) { UNUSED(expr); } +void ExtractPropExprVisitor::visit(EdgeExpression* expr) { + if (uniqueEdgeVertexCol_.emplace(expr->toString()).second) { + srcAndEdgePropCols_->addColumn(new YieldColumn(expr->clone(), expr->toString())); + } +} void ExtractPropExprVisitor::visit(VariableExpression* expr) { UNUSED(expr); } diff --git a/src/graph/visitor/ExtractPropExprVisitor.h b/src/graph/visitor/ExtractPropExprVisitor.h index 5eda485de80..6a8e0665dcb 100644 --- a/src/graph/visitor/ExtractPropExprVisitor.h +++ b/src/graph/visitor/ExtractPropExprVisitor.h @@ -20,7 +20,8 @@ class ExtractPropExprVisitor final : public ExprVisitorImpl { YieldColumns *srcAndEdgePropCols, YieldColumns *dstPropCols, YieldColumns *inputPropCols, - std::unordered_map &propExprColMap); + std::unordered_map &propExprColMap, + std::unordered_set &uniqueEdgeVertexCol); ~ExtractPropExprVisitor() = default; @@ -68,6 +69,7 @@ class ExtractPropExprVisitor final : public ExprVisitorImpl { YieldColumns *dstPropCols_{nullptr}; YieldColumns *inputPropCols_{nullptr}; std::unordered_map &propExprColMap_; + std::unordered_set &uniqueEdgeVertexCol_; Status status_; }; diff --git a/tests/tck/features/go/GO.feature b/tests/tck/features/go/GO.feature index 9ee57c3eb26..39beff1c987 100644 --- a/tests/tck/features/go/GO.feature +++ b/tests/tck/features/go/GO.feature @@ -1,6 +1,7 @@ # Copyright (c) 2021 vesoft inc. All rights reserved. # # This source code is licensed under Apache 2.0 License. +@jmq Feature: Go Sentence Background: diff --git a/tests/tck/features/go/GoYieldVertexEdge.feature b/tests/tck/features/go/GoYieldVertexEdge.feature index 81b0e8c1129..6fe55ba0ef4 100644 --- a/tests/tck/features/go/GoYieldVertexEdge.feature +++ b/tests/tck/features/go/GoYieldVertexEdge.feature @@ -1,6 +1,7 @@ # Copyright (c) 2021 vesoft inc. All rights reserved. # # This source code is licensed under Apache 2.0 License. +@jmq Feature: Go Yield Vertex And Edge Sentence Background: diff --git a/tests/tck/features/go/GroupbyLimit.feature b/tests/tck/features/go/GroupbyLimit.feature index 7706c27c6b2..96989572b5e 100644 --- a/tests/tck/features/go/GroupbyLimit.feature +++ b/tests/tck/features/go/GroupbyLimit.feature @@ -1,3 +1,4 @@ +@jmq Feature: Groupby & limit Sentence Background: Prepare space From 95e4551aa366d9ef4ceef808dee88feccb452ea2 Mon Sep 17 00:00:00 2001 From: jimingquan Date: Thu, 9 Dec 2021 15:47:47 +0800 Subject: [PATCH 2/5] refactor deduceProps --- src/graph/validator/FetchEdgesValidator.cpp | 22 +------ src/graph/validator/FetchEdgesValidator.h | 2 - .../validator/FetchVerticesValidator.cpp | 19 +----- src/graph/validator/FetchVerticesValidator.h | 3 +- src/graph/validator/GoValidator.cpp | 29 ++-------- src/graph/validator/GoValidator.h | 2 - src/graph/validator/LookupValidator.cpp | 11 +++- src/graph/validator/LookupValidator.h | 1 + src/graph/validator/Validator.cpp | 8 ++- src/graph/validator/Validator.h | 5 +- src/graph/validator/YieldValidator.cpp | 2 +- src/graph/visitor/DeducePropsVisitor.cpp | 58 ++++++++++++++----- src/graph/visitor/DeducePropsVisitor.h | 6 +- 13 files changed, 80 insertions(+), 88 deletions(-) diff --git a/src/graph/validator/FetchEdgesValidator.cpp b/src/graph/validator/FetchEdgesValidator.cpp index d0f5c0a2b4e..a30e5d53cde 100644 --- a/src/graph/validator/FetchEdgesValidator.cpp +++ b/src/graph/validator/FetchEdgesValidator.cpp @@ -136,18 +136,6 @@ Status FetchEdgesValidator::validateEdgeKey() { return Status::OK(); } -void FetchEdgesValidator::extractEdgeProp(ExpressionProps &exprProps) { - exprProps.insertEdgeProp(edgeType_, kSrc); - exprProps.insertEdgeProp(edgeType_, kDst); - exprProps.insertEdgeProp(edgeType_, kRank); - exprProps.insertEdgeProp(edgeType_, kType); - - for (std::size_t i = 0; i < edgeSchema_->getNumFields(); ++i) { - const auto propName = edgeSchema_->getFieldName(i); - exprProps.insertEdgeProp(edgeType_, propName); - } -} - Status FetchEdgesValidator::validateYield(const YieldClause *yield) { if (yield == nullptr) { return Status::SemanticError("Missing yield clause."); @@ -158,12 +146,6 @@ Status FetchEdgesValidator::validateYield(const YieldClause *yield) { exprProps.insertEdgeProp(edgeType_, nebula::kDst); exprProps.insertEdgeProp(edgeType_, nebula::kRank); - for (const auto &col : yield->columns()) { - if (ExpressionUtils::hasAny(col->expr(), {Expression::Kind::kEdge})) { - extractEdgeProp(exprProps); - break; - } - } auto size = yield->columns().size(); outputs_.reserve(size); @@ -182,8 +164,8 @@ Status FetchEdgesValidator::validateYield(const YieldClause *yield) { NG_RETURN_IF_ERROR(typeStatus); outputs_.emplace_back(col->name(), typeStatus.value()); newCols->addColumn(col->clone().release()); - - NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps)); + std::vector edgeTypes{edgeType_}; + NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps, nullptr, &edgeTypes)); } if (exprProps.hasInputVarProperty()) { diff --git a/src/graph/validator/FetchEdgesValidator.h b/src/graph/validator/FetchEdgesValidator.h index d78e3c012e6..e038f976068 100644 --- a/src/graph/validator/FetchEdgesValidator.h +++ b/src/graph/validator/FetchEdgesValidator.h @@ -26,8 +26,6 @@ class FetchEdgesValidator final : public Validator { Status validateEdgeKey(); - void extractEdgeProp(ExpressionProps& exprProps); - Status validateYield(const YieldClause* yieldClause); AstContext* getAstContext() override { return fetchCtx_.get(); } diff --git a/src/graph/validator/FetchVerticesValidator.cpp b/src/graph/validator/FetchVerticesValidator.cpp index dd902d79002..40bbd3a2b60 100644 --- a/src/graph/validator/FetchVerticesValidator.cpp +++ b/src/graph/validator/FetchVerticesValidator.cpp @@ -30,6 +30,7 @@ Status FetchVerticesValidator::validateTag(const NameLabelList *nameLabels) { NG_RETURN_IF_ERROR(tagStatus); for (const auto &tag : tagStatus.value()) { tagsSchema_.emplace(tag.first, tag.second); + tagIds_.emplace_back(tag.first); } } else { auto labels = nameLabels->labels(); @@ -43,6 +44,7 @@ Status FetchVerticesValidator::validateTag(const NameLabelList *nameLabels) { return Status::SemanticError("no schema found for `%s'", label->c_str()); } tagsSchema_.emplace(tagID, tagSchema); + tagIds_.emplace_back(tagID); } } return Status::OK(); @@ -76,12 +78,8 @@ Status FetchVerticesValidator::validateYield(YieldClause *yield) { col->setAlias(col->name()); col->setExpr(InputPropertyExpression::make(pool, nebula::kVid)); } - if (ExpressionUtils::hasAny(colExpr, {Expression::Kind::kVertex})) { - extractVertexProp(exprProps); - } newCols->addColumn(col->clone().release()); - - NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps)); + NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps, &tagIds_)); } if (exprProps.tagProps().empty()) { for (const auto &tagSchema : tagsSchema_) { @@ -105,16 +103,5 @@ Status FetchVerticesValidator::validateYield(YieldClause *yield) { return Status::OK(); } -void FetchVerticesValidator::extractVertexProp(ExpressionProps &exprProps) { - for (const auto &tagSchema : tagsSchema_) { - auto tagID = tagSchema.first; - exprProps.insertTagProp(tagID, nebula::kTag); - for (std::size_t i = 0; i < tagSchema.second->getNumFields(); ++i) { - const auto propName = tagSchema.second->getFieldName(i); - exprProps.insertTagProp(tagID, propName); - } - } -} - } // namespace graph } // namespace nebula diff --git a/src/graph/validator/FetchVerticesValidator.h b/src/graph/validator/FetchVerticesValidator.h index 88f87f6762a..4f5bea407b7 100644 --- a/src/graph/validator/FetchVerticesValidator.h +++ b/src/graph/validator/FetchVerticesValidator.h @@ -27,10 +27,9 @@ class FetchVerticesValidator final : public Validator { AstContext* getAstContext() override { return fetchCtx_.get(); } - void extractVertexProp(ExpressionProps& exprProps); - private: std::map> tagsSchema_; + std::vector tagIds_; std::unique_ptr fetchCtx_; }; diff --git a/src/graph/validator/GoValidator.cpp b/src/graph/validator/GoValidator.cpp index af0c206a19b..d13e45f083b 100644 --- a/src/graph/validator/GoValidator.cpp +++ b/src/graph/validator/GoValidator.cpp @@ -15,9 +15,6 @@ namespace nebula { namespace graph { -// static const char* SRC_VERTEX = "$^"; -// static const char* DST_VERTEX = "$$"; - Status GoValidator::validateImpl() { auto* goSentence = static_cast(sentence_); goCtx_ = getContext(); @@ -89,7 +86,7 @@ Status GoValidator::validateWhere(WhereClause* where) { return Status::SemanticError(ss.str()); } - NG_RETURN_IF_ERROR(deduceProps(filter, goCtx_->exprProps)); + NG_RETURN_IF_ERROR(deduceProps(filter, goCtx_->exprProps, nullptr, &goCtx_->over.edgeTypes)); goCtx_->filter = filter; return Status::OK(); } @@ -140,7 +137,8 @@ Status GoValidator::validateYield(YieldClause* yield) { } auto vertexExpr = ExpressionUtils::findAny(col->expr(), {Expression::Kind::kVertex}); - if (static_cast(vertexExpr)->name() == "VERTEX") { + if (vertexExpr != nullptr && + static_cast(vertexExpr)->name() == "VERTEX") { return Status::SemanticError("`%s' is not support in go sentence.", col->toString().c_str()); } @@ -148,15 +146,11 @@ Status GoValidator::validateYield(YieldClause* yield) { NG_RETURN_IF_ERROR(ValidateUtil::invalidLabelIdentifiers(col->expr())); auto* colExpr = col->expr(); - if (ExpressionUtils::hasAny(colExpr, {Expression::Kind::kEdge})) { - extractEdgeProp(exprProps); - } - auto typeStatus = deduceExprType(colExpr); NG_RETURN_IF_ERROR(typeStatus); auto type = typeStatus.value(); outputs_.emplace_back(col->name(), type); - NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps)); + NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps, nullptr, &goCtx_->over.edgeTypes)); } const auto& over = goCtx_->over; @@ -171,21 +165,6 @@ Status GoValidator::validateYield(YieldClause* yield) { return Status::OK(); } -Status GoValidator::extractEdgeProp(ExpressionProps& exprProps) { - const auto& edgeTypes = goCtx_->over.edgeTypes; - for (const auto& edgeType : edgeTypes) { - const auto& edgeSchema = qctx_->schemaMng()->getEdgeSchema(space_.id, std::abs(edgeType)); - exprProps.insertEdgeProp(edgeType, kType); - exprProps.insertEdgeProp(edgeType, kSrc); - exprProps.insertEdgeProp(edgeType, kDst); - exprProps.insertEdgeProp(edgeType, kRank); - for (size_t i = 0; i < edgeSchema->getNumFields(); ++i) { - exprProps.insertEdgeProp(edgeType, edgeSchema->getFieldName(i)); - } - } - return Status::OK(); -} - void GoValidator::extractPropExprs(const Expression* expr, std::unordered_set& uniqueExpr) { ExtractPropExprVisitor visitor(vctx_, diff --git a/src/graph/validator/GoValidator.h b/src/graph/validator/GoValidator.h index fb034797705..1d3bbb3bb7d 100644 --- a/src/graph/validator/GoValidator.h +++ b/src/graph/validator/GoValidator.h @@ -36,8 +36,6 @@ class GoValidator final : public Validator { Expression* rewrite2VarProp(const Expression* expr); - Status extractEdgeProp(ExpressionProps& exprProps); - private: std::unique_ptr goCtx_; diff --git a/src/graph/validator/LookupValidator.cpp b/src/graph/validator/LookupValidator.cpp index 6426b8975fc..7da955281b5 100644 --- a/src/graph/validator/LookupValidator.cpp +++ b/src/graph/validator/LookupValidator.cpp @@ -53,6 +53,7 @@ Status LookupValidator::validateFrom() { NG_RETURN_IF_ERROR(ret); lookupCtx_->isEdge = ret.value().first; lookupCtx_->schemaId = ret.value().second; + schemaIds_.emplace_back(ret.value().second); return Status::OK(); } @@ -124,7 +125,7 @@ Status LookupValidator::validateYieldEdge() { NG_RETURN_IF_ERROR(typeStatus); outputs_.emplace_back(col->name(), typeStatus.value()); yieldExpr->addColumn(col->clone().release()); - NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps_)); + NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps_, nullptr, &schemaIds_)); } return Status::OK(); } @@ -154,7 +155,7 @@ Status LookupValidator::validateYieldTag() { NG_RETURN_IF_ERROR(typeStatus); outputs_.emplace_back(col->name(), typeStatus.value()); yieldExpr->addColumn(col->clone().release()); - NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps_)); + NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps_, &schemaIds_)); } return Status::OK(); } @@ -211,7 +212,11 @@ Status LookupValidator::validateFilter() { // Make sure the type of the rewritted filter expr is right NG_RETURN_IF_ERROR(deduceExprType(lookupCtx_->filter)); } - NG_RETURN_IF_ERROR(deduceProps(lookupCtx_->filter, exprProps_)); + if (lookupCtx_->isEdge) { + NG_RETURN_IF_ERROR(deduceProps(lookupCtx_->filter, exprProps_, nullptr, &schemaIds_)); + } else { + NG_RETURN_IF_ERROR(deduceProps(lookupCtx_->filter, exprProps_, &schemaIds_)); + } return Status::OK(); } diff --git a/src/graph/validator/LookupValidator.h b/src/graph/validator/LookupValidator.h index 54713d46ef4..56236bdd92c 100644 --- a/src/graph/validator/LookupValidator.h +++ b/src/graph/validator/LookupValidator.h @@ -60,6 +60,7 @@ class LookupValidator final : public Validator { std::vector tsClients_; ExpressionProps exprProps_; std::vector idxReturnCols_; + std::vector schemaIds_; }; } // namespace graph diff --git a/src/graph/validator/Validator.cpp b/src/graph/validator/Validator.cpp index 022e60a023c..8d18dab7641 100644 --- a/src/graph/validator/Validator.cpp +++ b/src/graph/validator/Validator.cpp @@ -358,8 +358,12 @@ StatusOr Validator::deduceExprType(const Expression* expr) const { return visitor.type(); } -Status Validator::deduceProps(const Expression* expr, ExpressionProps& exprProps) { - DeducePropsVisitor visitor(qctx_, space_.id, &exprProps, &userDefinedVarNameList_); +Status Validator::deduceProps(const Expression* expr, + ExpressionProps& exprProps, + std::vector* tagIds, + std::vector* edgeTypes) { + DeducePropsVisitor visitor( + qctx_, space_.id, &exprProps, &userDefinedVarNameList_, tagIds, edgeTypes); const_cast(expr)->accept(&visitor); return std::move(visitor).status(); } diff --git a/src/graph/validator/Validator.h b/src/graph/validator/Validator.h index 0568d0eb054..686ec654b17 100644 --- a/src/graph/validator/Validator.h +++ b/src/graph/validator/Validator.h @@ -107,7 +107,10 @@ class Validator { StatusOr deduceExprType(const Expression* expr) const; - Status deduceProps(const Expression* expr, ExpressionProps& exprProps); + Status deduceProps(const Expression* expr, + ExpressionProps& exprProps, + std::vector* tagIds = nullptr, + std::vector* edgeTypes = nullptr); static StatusOr checkPropNonexistOrDuplicate(const ColsDef& cols, folly::StringPiece prop, diff --git a/src/graph/validator/YieldValidator.cpp b/src/graph/validator/YieldValidator.cpp index 901774e8697..e622eda743d 100644 --- a/src/graph/validator/YieldValidator.cpp +++ b/src/graph/validator/YieldValidator.cpp @@ -65,7 +65,7 @@ Status YieldValidator::makeOutputColumn(YieldColumn *column) { DCHECK(colExpr != nullptr); auto expr = colExpr->clone(); - NG_RETURN_IF_ERROR(deduceProps(expr, exprProps_)); + NG_RETURN_IF_ERROR(deduceProps(expr, exprProps_, {}, {})); auto status = deduceExprType(expr); NG_RETURN_IF_ERROR(status); diff --git a/src/graph/visitor/DeducePropsVisitor.cpp b/src/graph/visitor/DeducePropsVisitor.cpp index 8991932a95d..d7c494913d2 100644 --- a/src/graph/visitor/DeducePropsVisitor.cpp +++ b/src/graph/visitor/DeducePropsVisitor.cpp @@ -108,11 +108,15 @@ void ExpressionProps::unionProps(ExpressionProps exprProps) { DeducePropsVisitor::DeducePropsVisitor(QueryContext *qctx, GraphSpaceID space, ExpressionProps *exprProps, - std::set *userDefinedVarNameList) + std::set *userDefinedVarNameList, + std::vector *tagIds, + std::vector *edgeTypes) : qctx_(qctx), space_(space), exprProps_(exprProps), - userDefinedVarNameList_(userDefinedVarNameList) { + userDefinedVarNameList_(userDefinedVarNameList), + tagIds_(tagIds), + edgeTypes_(edgeTypes) { DCHECK(qctx != nullptr); DCHECK(exprProps != nullptr); DCHECK(userDefinedVarNameList != nullptr); @@ -177,31 +181,59 @@ void DeducePropsVisitor::visit(LabelAttributeExpression *expr) { reportError(exp void DeducePropsVisitor::visit(ConstantExpression *expr) { UNUSED(expr); } +void DeducePropsVisitor::visit(ColumnExpression *expr) { UNUSED(expr); } + void DeducePropsVisitor::visit(VertexExpression *expr) { - const auto &colName = expr->name(); - auto tagStatus = qctx_->schemaMng()->getAllLatestVerTagSchema(space_); - if (!tagStatus.ok()) { - status_ = std::move(tagStatus).status(); - return; + std::vector tagIds; + if (tagIds_ == nullptr) { + auto tagStatus = qctx_->schemaMng()->getAllLatestVerTagSchema(space_); + if (!tagStatus.ok()) { + status_ = std::move(tagStatus).status(); + return; + } + for (const auto &tag : tagStatus.value()) { + tagIds.emplace_back(tag.first); + } + tagIds_ = &tagIds; } - for (const auto &tag : tagStatus.value()) { - auto tagID = tag.first; - const auto &tagSchema = tag.second; + const auto &colName = expr->name(); + for (const auto &tagID : *tagIds_) { + const auto &tagSchema = qctx_->schemaMng()->getTagSchema(space_, tagID); if (colName == "$^") { + exprProps_->insertSrcTagProp(tagID, nebula::kTag); for (size_t i = 0; i < tagSchema->getNumFields(); ++i) { exprProps_->insertSrcTagProp(tagID, tagSchema->getFieldName(i)); } } else if (colName == "$$") { + exprProps_->insertDstTagProp(tagID, nebula::kTag); for (size_t i = 0; i < tagSchema->getNumFields(); ++i) { exprProps_->insertDstTagProp(tagID, tagSchema->getFieldName(i)); } + } else { + exprProps_->insertTagProp(tagID, nebula::kTag); + for (size_t i = 0; i < tagSchema->getNumFields(); ++i) { + exprProps_->insertTagProp(tagID, tagSchema->getFieldName(i)); + } } } } -void DeducePropsVisitor::visit(EdgeExpression *expr) { UNUSED(expr); } - -void DeducePropsVisitor::visit(ColumnExpression *expr) { UNUSED(expr); } +void DeducePropsVisitor::visit(EdgeExpression *expr) { + if (edgeTypes_ == nullptr) { + UNUSED(expr); + return; + } + for (const auto &edgeType : *edgeTypes_) { + const auto &edgeSchema = qctx_->schemaMng()->getEdgeSchema(space_, std::abs(edgeType)); + exprProps_->insertEdgeProp(edgeType, kType); + exprProps_->insertEdgeProp(edgeType, kSrc); + exprProps_->insertEdgeProp(edgeType, kDst); + exprProps_->insertEdgeProp(edgeType, kRank); + for (size_t i = 0; i < edgeSchema->getNumFields(); ++i) { + exprProps_->insertEdgeProp(edgeType, edgeSchema->getFieldName(i)); + } + } +} void DeducePropsVisitor::visitEdgePropExpr(PropertyExpression *expr) { auto status = qctx_->schemaMng()->toEdgeType(space_, expr->sym()); diff --git a/src/graph/visitor/DeducePropsVisitor.h b/src/graph/visitor/DeducePropsVisitor.h index 05cdb5e4a4d..ef8a603d1f0 100644 --- a/src/graph/visitor/DeducePropsVisitor.h +++ b/src/graph/visitor/DeducePropsVisitor.h @@ -71,7 +71,9 @@ class DeducePropsVisitor : public ExprVisitorImpl { DeducePropsVisitor(QueryContext* qctx, GraphSpaceID space, ExpressionProps* exprProps, - std::set* userDefinedVarNameList); + std::set* userDefinedVarNameList, + std::vector* tagIds, + std::vector* edgeTypes); bool ok() const override { return status_.ok(); } @@ -106,6 +108,8 @@ class DeducePropsVisitor : public ExprVisitorImpl { GraphSpaceID space_; ExpressionProps* exprProps_{nullptr}; std::set* userDefinedVarNameList_{nullptr}; + std::vector* tagIds_{nullptr}; + std::vector* edgeTypes_{nullptr}; Status status_; }; From 5c89005493e5bcad1483bf4e259c6c5f9a76ae0c Mon Sep 17 00:00:00 2001 From: jimingquan Date: Wed, 15 Dec 2021 20:50:11 +0800 Subject: [PATCH 3/5] add testcases --- src/graph/visitor/DeducePropsVisitor.cpp | 7 +- tests/tck/features/go/GO.feature | 1 - .../tck/features/go/GoYieldVertexEdge.feature | 115 +++++++++++++++++- tests/tck/features/go/GroupbyLimit.feature | 1 - 4 files changed, 119 insertions(+), 5 deletions(-) diff --git a/src/graph/visitor/DeducePropsVisitor.cpp b/src/graph/visitor/DeducePropsVisitor.cpp index d7c494913d2..3b3f331ede4 100644 --- a/src/graph/visitor/DeducePropsVisitor.cpp +++ b/src/graph/visitor/DeducePropsVisitor.cpp @@ -194,10 +194,13 @@ void DeducePropsVisitor::visit(VertexExpression *expr) { for (const auto &tag : tagStatus.value()) { tagIds.emplace_back(tag.first); } - tagIds_ = &tagIds; + } else { + for (const auto &tagID : *tagIds_) { + tagIds.emplace_back(tagID); + } } const auto &colName = expr->name(); - for (const auto &tagID : *tagIds_) { + for (const auto &tagID : tagIds) { const auto &tagSchema = qctx_->schemaMng()->getTagSchema(space_, tagID); if (colName == "$^") { exprProps_->insertSrcTagProp(tagID, nebula::kTag); diff --git a/tests/tck/features/go/GO.feature b/tests/tck/features/go/GO.feature index 39beff1c987..9ee57c3eb26 100644 --- a/tests/tck/features/go/GO.feature +++ b/tests/tck/features/go/GO.feature @@ -1,7 +1,6 @@ # Copyright (c) 2021 vesoft inc. All rights reserved. # # This source code is licensed under Apache 2.0 License. -@jmq Feature: Go Sentence Background: diff --git a/tests/tck/features/go/GoYieldVertexEdge.feature b/tests/tck/features/go/GoYieldVertexEdge.feature index 6fe55ba0ef4..105d126e78f 100644 --- a/tests/tck/features/go/GoYieldVertexEdge.feature +++ b/tests/tck/features/go/GoYieldVertexEdge.feature @@ -1,7 +1,6 @@ # Copyright (c) 2021 vesoft inc. All rights reserved. # # This source code is licensed under Apache 2.0 License. -@jmq Feature: Go Yield Vertex And Edge Sentence Background: @@ -1810,3 +1809,117 @@ Feature: Go Yield Vertex And Edge Sentence | "Grant Hill" | "Grant Hill" | [:like "Grant Hill"->"Tracy McGrady" @0 {likeness: 90}] | ("Grant Hill" :player{age: 46, name: "Grant Hill"}) | ("Tracy McGrady" :player{age: 39, name: "Tracy McGrady"}) | | "Vince Carter" | "Vince Carter" | [:like "Vince Carter"->"Tracy McGrady" @0 {likeness: 90}] | ("Vince Carter" :player{age: 42, name: "Vince Carter"}) | ("Tracy McGrady" :player{age: 39, name: "Tracy McGrady"}) | | "Yao Ming" | "Yao Ming" | [:like "Yao Ming"->"Tracy McGrady" @0 {likeness: 90}] | ("Yao Ming" :player{age: 38, name: "Yao Ming"}) | ("Tracy McGrady" :player{age: 39, name: "Tracy McGrady"}) | + + Scenario: support properties function in where + When executing query: + """ + GO FROM 'Tim Duncan' OVER like WHERE properties($$).age > 38 YIELD edge as e, $$ as dst + """ + Then the result should be, in any order, with relax comparison: + | e | dst | + | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) | + When executing query: + """ + GO FROM 'Tim Duncan' OVER like WHERE properties(edge).age > 38 YIELD edge as e, $$ as dst + """ + Then the result should be, in any order, with relax comparison: + | e | dst | + When executing query: + """ + GO FROM 'Tim Duncan' OVER like WHERE properties(edge).likeness > 80 YIELD edge as e, $$ as dst + """ + Then the result should be, in any order, with relax comparison: + | e | dst | + | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) | + | [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | + When executing query: + """ + GO 1 TO 2 STEPS FROM 'Russell Westbrook' OVER * where properties($$).age > 20 YIELD $$ as dst + """ + Then the result should be, in any order, with relax comparison: + | dst | + | ("James Harden" :player{age: 29, name: "James Harden"}) | + | ("Paul George" :player{age: 28, name: "Paul George"}) | + | ("Russell Westbrook" :player{age: 30, name: "Russell Westbrook"}) | + | ("Russell Westbrook" :player{age: 30, name: "Russell Westbrook"}) | + When executing query: + """ + GO 1 TO 2 STEPS FROM 'Tony Parker' OVER like BIDIRECT where properties($$).age > 30 YIELD DISTINCT properties(edge) as props, edge as e + """ + Then the result should be, in any order, with relax comparison: + | props | e | + | {likeness: 80} | [:like "Boris Diaw"->"Tony Parker" @0 {likeness: 80}] | + | {likeness: 95} | [:like "Tony Parker"->"Tim Duncan" @0 {likeness: 95}] | + | {likeness: 75} | [:like "LaMarcus Aldridge"->"Tony Parker" @0 {likeness: 75}] | + | {likeness: 50} | [:like "Marco Belinelli"->"Tony Parker" @0 {likeness: 50}] | + | {likeness: 95} | [:like "Tim Duncan"->"Tony Parker" @0 {likeness: 95}] | + | {likeness: 90} | [:like "Tony Parker"->"LaMarcus Aldridge" @0 {likeness: 90}] | + | {likeness: 95} | [:like "Tony Parker"->"Manu Ginobili" @0 {likeness: 95}] | + | {likeness: 83} | [:like "Danny Green"->"Marco Belinelli" @0 {likeness: 83}] | + | {likeness: 90} | [:like "Manu Ginobili"->"Tim Duncan" @0 {likeness: 90}] | + | {likeness: 60} | [:like "Marco Belinelli"->"Danny Green" @0 {likeness: 60}] | + | {likeness: 55} | [:like "Marco Belinelli"->"Tim Duncan" @0 {likeness: 55}] | + | {likeness: 90} | [:like "Tiago Splitter"->"Manu Ginobili" @0 {likeness: 90}] | + | {likeness: 80} | [:like "Aron Baynes"->"Tim Duncan" @0 {likeness: 80}] | + | {likeness: 80} | [:like "Boris Diaw"->"Tim Duncan" @0 {likeness: 80}] | + | {likeness: 70} | [:like "Danny Green"->"Tim Duncan" @0 {likeness: 70}] | + | {likeness: 99} | [:like "Dejounte Murray"->"Danny Green" @0 {likeness: 99}] | + | {likeness: 75} | [:like "LaMarcus Aldridge"->"Tim Duncan" @0 {likeness: 75}] | + | {likeness: 99} | [:like "Dejounte Murray"->"Marco Belinelli" @0 {likeness: 99}] | + | {likeness: 99} | [:like "Dejounte Murray"->"Manu Ginobili" @0 {likeness: 99}] | + | {likeness: 80} | [:like "Shaquille O'Neal"->"Tim Duncan" @0 {likeness: 80}] | + | {likeness: 80} | [:like "Tiago Splitter"->"Tim Duncan" @0 {likeness: 80}] | + | {likeness: 99} | [:like "Dejounte Murray"->"Chris Paul" @0 {likeness: 99}] | + | {likeness: 95} | [:like "Tim Duncan"->"Manu Ginobili" @0 {likeness: 95}] | + | {likeness: 99} | [:like "Dejounte Murray"->"Tony Parker" @0 {likeness: 99}] | + | {likeness: 99} | [:like "Dejounte Murray"->"LeBron James" @0 {likeness: 99}] | + | {likeness: 70} | [:like "Rudy Gay"->"LaMarcus Aldridge" @0 {likeness: 70}] | + | {likeness: 99} | [:like "Dejounte Murray"->"Tim Duncan" @0 {likeness: 99}] | + When executing query: + """ + GO FROM 'Danny Green' OVER like YIELD src(edge) AS src, dst(edge) AS dst | + GO FROM $-.dst OVER teammate where properties($$).age > 35 YIELD $-.src AS src, $-.dst, $$ as dst + """ + Then the result should be, in any order, with relax comparison: + | src | $-.dst | dst | + | "Danny Green" | "Tim Duncan" | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | + | "Danny Green" | "Tim Duncan" | ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) | + When executing query: + """ + GO FROM 'Danny Green' OVER like YIELD src(edge) AS src, dst(edge) AS dst | + GO FROM $-.dst OVER teammate where properties($^).age > 35 YIELD $-.src AS src, $-.dst, $$ as dst + """ + Then the result should be, in any order, with relax comparison: + | src | $-.dst | dst | + | "Danny Green" | "Tim Duncan" | ("Danny Green" :player{age: 31, name: "Danny Green"}) | + | "Danny Green" | "Tim Duncan" | ("LaMarcus Aldridge" :player{age: 33, name: "LaMarcus Aldridge"}) | + | "Danny Green" | "Tim Duncan" | ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) | + | "Danny Green" | "Tim Duncan" | ("Tony Parker" :player{age: 36, name: "Tony Parker"}) | + When executing query: + """ + GO 2 STEPS FROM 'Kobe Bryant' OVER like REVERSELY WHERE properties(edge).likeness != 80 YIELD $$ as dst, edge as e + """ + Then the result should be, in any order, with relax comparison: + | dst | e | + | ("Marc Gasol" :player{age: 34, name: "Marc Gasol"}) | [:like "Marc Gasol"->"Paul Gasol" @0 {likeness: 99}] | + | ("Grant Hill" :player{age: 46, name: "Grant Hill"}) | [:like "Grant Hill"->"Tracy McGrady" @0 {likeness: 90}] | + | ("Vince Carter" :player{age: 42, name: "Vince Carter"}) | [:like "Vince Carter"->"Tracy McGrady" @0 {likeness: 90}] | + | ("Yao Ming" :player{age: 38, name: "Yao Ming"}) | [:like "Yao Ming"->"Tracy McGrady" @0 {likeness: 90}] | + When executing query: + """ + $var = GO FROM "Tim Duncan", "Chris Paul" OVER like WHERE properties($$).age > 20 YIELD id($$) as id; + GO FROM $var.id OVER * WHERE properties(edge).likeness > 80 YIELD $$ as dst + """ + Then the result should be, in any order, with relax comparison: + | dst | + | ("LaMarcus Aldridge" :player{age: 33, name: "LaMarcus Aldridge"}) | + | ("Manu Ginobili" :player{age: 41, name: "Manu Ginobili"}) | + | ("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"}) | + | ("LeBron James" :player{age: 34, name: "LeBron James"}) | + | ("Chris Paul" :player{age: 33, name: "Chris Paul"}) | + | ("Carmelo Anthony" :player{age: 34, name: "Carmelo Anthony"}) | + | ("Tim Duncan" :bachelor{name: "Tim Duncan", speciality: "psychology"} :player{age: 42, name: "Tim Duncan"}) | + | ("Ray Allen" :player{age: 43, name: "Ray Allen"}) | + | ("LeBron James" :player{age: 34, name: "LeBron James"}) | + | ("Chris Paul" :player{age: 33, name: "Chris Paul"}) | + | ("Dwyane Wade" :player{age: 37, name: "Dwyane Wade"}) | diff --git a/tests/tck/features/go/GroupbyLimit.feature b/tests/tck/features/go/GroupbyLimit.feature index 96989572b5e..7706c27c6b2 100644 --- a/tests/tck/features/go/GroupbyLimit.feature +++ b/tests/tck/features/go/GroupbyLimit.feature @@ -1,4 +1,3 @@ -@jmq Feature: Groupby & limit Sentence Background: Prepare space From 381f91639e6a10033c55ba9521f58feba7b5ecf0 Mon Sep 17 00:00:00 2001 From: jimingquan Date: Wed, 15 Dec 2021 20:55:55 +0800 Subject: [PATCH 4/5] fix test error --- src/graph/validator/LookupValidator.cpp | 23 ++--------------------- src/graph/validator/LookupValidator.h | 3 +-- src/graph/validator/YieldValidator.cpp | 2 +- 3 files changed, 4 insertions(+), 24 deletions(-) diff --git a/src/graph/validator/LookupValidator.cpp b/src/graph/validator/LookupValidator.cpp index 7da955281b5..5395ebaed6f 100644 --- a/src/graph/validator/LookupValidator.cpp +++ b/src/graph/validator/LookupValidator.cpp @@ -41,7 +41,7 @@ Status LookupValidator::validateImpl() { lookupCtx_ = getContext(); NG_RETURN_IF_ERROR(validateFrom()); - NG_RETURN_IF_ERROR(validateFilter()); + NG_RETURN_IF_ERROR(validateWhere()); NG_RETURN_IF_ERROR(validateYield()); return Status::OK(); } @@ -57,19 +57,6 @@ Status LookupValidator::validateFrom() { return Status::OK(); } -Status LookupValidator::extractSchemaProp() { - shared_ptr schema; - NG_RETURN_IF_ERROR(getSchemaProvider(&schema)); - for (std::size_t i = 0; i < schema->getNumFields(); ++i) { - const auto& propName = schema->getFieldName(i); - auto iter = std::find(idxReturnCols_.begin(), idxReturnCols_.end(), propName); - if (iter == idxReturnCols_.end()) { - idxReturnCols_.emplace_back(propName); - } - } - return Status::OK(); -} - void LookupValidator::extractExprProps() { auto addProps = [this](const std::set& propNames) { for (const auto& propName : propNames) { @@ -108,9 +95,6 @@ Status LookupValidator::validateYieldEdge() { {Expression::Kind::kAggregate, Expression::Kind::kVertex})) { return Status::SemanticError("illegal yield clauses `%s'", col->toString().c_str()); } - if (ExpressionUtils::hasAny(col->expr(), {Expression::Kind::kEdge})) { - NG_RETURN_IF_ERROR(extractSchemaProp()); - } if (col->expr()->kind() == Expression::Kind::kLabelAttribute) { const auto& schemaName = static_cast(col->expr())->left()->name(); if (schemaName != sentence()->from()) { @@ -138,9 +122,6 @@ Status LookupValidator::validateYieldTag() { {Expression::Kind::kAggregate, Expression::Kind::kEdge})) { return Status::SemanticError("illegal yield clauses `%s'", col->toString().c_str()); } - if (ExpressionUtils::hasAny(col->expr(), {Expression::Kind::kVertex})) { - NG_RETURN_IF_ERROR(extractSchemaProp()); - } if (col->expr()->kind() == Expression::Kind::kLabelAttribute) { const auto& schemaName = static_cast(col->expr())->left()->name(); if (schemaName != sentence()->from()) { @@ -188,7 +169,7 @@ Status LookupValidator::validateYield() { return Status::OK(); } -Status LookupValidator::validateFilter() { +Status LookupValidator::validateWhere() { auto whereClause = sentence()->whereClause(); if (whereClause == nullptr) { return Status::OK(); diff --git a/src/graph/validator/LookupValidator.h b/src/graph/validator/LookupValidator.h index 56236bdd92c..9889d99971e 100644 --- a/src/graph/validator/LookupValidator.h +++ b/src/graph/validator/LookupValidator.h @@ -29,7 +29,7 @@ class LookupValidator final : public Validator { Status validateFrom(); Status validateYield(); - Status validateFilter(); + Status validateWhere(); Status validateYieldTag(); Status validateYieldEdge(); @@ -53,7 +53,6 @@ class LookupValidator final : public Validator { Status getSchemaProvider(std::shared_ptr* provider) const; StatusOr genTsFilter(Expression* filter); StatusOr handleLogicalExprOperands(LogicalExpression* lExpr); - Status extractSchemaProp(); void extractExprProps(); std::unique_ptr lookupCtx_; diff --git a/src/graph/validator/YieldValidator.cpp b/src/graph/validator/YieldValidator.cpp index e622eda743d..901774e8697 100644 --- a/src/graph/validator/YieldValidator.cpp +++ b/src/graph/validator/YieldValidator.cpp @@ -65,7 +65,7 @@ Status YieldValidator::makeOutputColumn(YieldColumn *column) { DCHECK(colExpr != nullptr); auto expr = colExpr->clone(); - NG_RETURN_IF_ERROR(deduceProps(expr, exprProps_, {}, {})); + NG_RETURN_IF_ERROR(deduceProps(expr, exprProps_)); auto status = deduceExprType(expr); NG_RETURN_IF_ERROR(status); From 4ac7d4f270cbc7adf29df2693da0f31541dab9c6 Mon Sep 17 00:00:00 2001 From: jimingquan Date: Mon, 20 Dec 2021 20:20:50 +0800 Subject: [PATCH 5/5] address comment --- src/graph/validator/GoValidator.cpp | 14 ++++++++++++-- src/graph/validator/GoValidator.h | 3 +++ src/graph/visitor/DeducePropsVisitor.cpp | 17 +++-------------- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/graph/validator/GoValidator.cpp b/src/graph/validator/GoValidator.cpp index d13e45f083b..69b5a9bbd39 100644 --- a/src/graph/validator/GoValidator.cpp +++ b/src/graph/validator/GoValidator.cpp @@ -23,6 +23,7 @@ Status GoValidator::validateImpl() { NG_RETURN_IF_ERROR(ValidateUtil::validateStep(goSentence->stepClause(), goCtx_->steps)); NG_RETURN_IF_ERROR(validateStarts(goSentence->fromClause(), goCtx_->from)); NG_RETURN_IF_ERROR(ValidateUtil::validateOver(qctx_, goSentence->overClause(), goCtx_->over)); + NG_RETURN_IF_ERROR(extractTagIds()); NG_RETURN_IF_ERROR(validateWhere(goSentence->whereClause())); NG_RETURN_IF_ERROR(validateYield(goSentence->yieldClause())); NG_RETURN_IF_ERROR(validateTruncate(goSentence->truncateClause())); @@ -86,7 +87,7 @@ Status GoValidator::validateWhere(WhereClause* where) { return Status::SemanticError(ss.str()); } - NG_RETURN_IF_ERROR(deduceProps(filter, goCtx_->exprProps, nullptr, &goCtx_->over.edgeTypes)); + NG_RETURN_IF_ERROR(deduceProps(filter, goCtx_->exprProps, &tagIds_, &goCtx_->over.edgeTypes)); goCtx_->filter = filter; return Status::OK(); } @@ -150,7 +151,7 @@ Status GoValidator::validateYield(YieldClause* yield) { NG_RETURN_IF_ERROR(typeStatus); auto type = typeStatus.value(); outputs_.emplace_back(col->name(), type); - NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps, nullptr, &goCtx_->over.edgeTypes)); + NG_RETURN_IF_ERROR(deduceProps(colExpr, exprProps, &tagIds_, &goCtx_->over.edgeTypes)); } const auto& over = goCtx_->over; @@ -165,6 +166,15 @@ Status GoValidator::validateYield(YieldClause* yield) { return Status::OK(); } +Status GoValidator::extractTagIds() { + auto tagStatus = qctx_->schemaMng()->getAllLatestVerTagSchema(space_.id); + NG_RETURN_IF_ERROR(tagStatus); + for (const auto& tag : tagStatus.value()) { + tagIds_.emplace_back(tag.first); + } + return Status::OK(); +} + void GoValidator::extractPropExprs(const Expression* expr, std::unordered_set& uniqueExpr) { ExtractPropExprVisitor visitor(vctx_, diff --git a/src/graph/validator/GoValidator.h b/src/graph/validator/GoValidator.h index 1d3bbb3bb7d..cc2b32860aa 100644 --- a/src/graph/validator/GoValidator.h +++ b/src/graph/validator/GoValidator.h @@ -32,6 +32,8 @@ class GoValidator final : public Validator { Status buildColumns(); + Status extractTagIds(); + void extractPropExprs(const Expression* expr, std::unordered_set& uniqueCols); Expression* rewrite2VarProp(const Expression* expr); @@ -41,6 +43,7 @@ class GoValidator final : public Validator { YieldColumns* inputPropCols_{nullptr}; std::unordered_map propExprColMap_; + std::vector tagIds_; }; } // namespace graph } // namespace nebula diff --git a/src/graph/visitor/DeducePropsVisitor.cpp b/src/graph/visitor/DeducePropsVisitor.cpp index 3b3f331ede4..8f36f262206 100644 --- a/src/graph/visitor/DeducePropsVisitor.cpp +++ b/src/graph/visitor/DeducePropsVisitor.cpp @@ -184,23 +184,12 @@ void DeducePropsVisitor::visit(ConstantExpression *expr) { UNUSED(expr); } void DeducePropsVisitor::visit(ColumnExpression *expr) { UNUSED(expr); } void DeducePropsVisitor::visit(VertexExpression *expr) { - std::vector tagIds; if (tagIds_ == nullptr) { - auto tagStatus = qctx_->schemaMng()->getAllLatestVerTagSchema(space_); - if (!tagStatus.ok()) { - status_ = std::move(tagStatus).status(); - return; - } - for (const auto &tag : tagStatus.value()) { - tagIds.emplace_back(tag.first); - } - } else { - for (const auto &tagID : *tagIds_) { - tagIds.emplace_back(tagID); - } + UNUSED(expr); + return; } const auto &colName = expr->name(); - for (const auto &tagID : tagIds) { + for (const auto &tagID : *tagIds_) { const auto &tagSchema = qctx_->schemaMng()->getTagSchema(space_, tagID); if (colName == "$^") { exprProps_->insertSrcTagProp(tagID, nebula::kTag);