From b555bc2f6b01c3ba15a02a995e0083ca3d8593d3 Mon Sep 17 00:00:00 2001 From: Shylock Hg <33566796+Shylock-Hg@users.noreply.github.com> Date: Mon, 19 Dec 2022 15:37:09 +0800 Subject: [PATCH 1/2] Fix checking update filter expression. --- src/graph/validator/MutateValidator.cpp | 76 +++- src/graph/visitor/CMakeLists.txt | 1 - src/graph/visitor/RewriteSymExprVisitor.cpp | 350 ------------------ src/graph/visitor/RewriteSymExprVisitor.h | 98 ----- tests/tck/features/bugfix/CompareDate.feature | 30 ++ 5 files changed, 101 insertions(+), 454 deletions(-) delete mode 100644 src/graph/visitor/RewriteSymExprVisitor.cpp delete mode 100644 src/graph/visitor/RewriteSymExprVisitor.h create mode 100644 tests/tck/features/bugfix/CompareDate.feature diff --git a/src/graph/validator/MutateValidator.cpp b/src/graph/validator/MutateValidator.cpp index 84a120e103c..508fba56459 100644 --- a/src/graph/validator/MutateValidator.cpp +++ b/src/graph/validator/MutateValidator.cpp @@ -4,12 +4,14 @@ */ #include "graph/validator/MutateValidator.h" +#include "common/datatypes/Value.h" +#include "common/expression/Expression.h" #include "common/expression/LabelAttributeExpression.h" #include "graph/planner/plan/Mutate.h" #include "graph/planner/plan/Query.h" #include "graph/util/ExpressionUtils.h" #include "graph/util/SchemaUtil.h" -#include "graph/visitor/RewriteSymExprVisitor.h" +#include "graph/visitor/RewriteVisitor.h" namespace nebula { namespace graph { @@ -750,10 +752,74 @@ Expression *UpdateValidator::rewriteSymExpr(Expression *expr, const std::string &sym, bool &hasWrongType, bool isEdge) { - RewriteSymExprVisitor visitor(qctx_->objPool(), sym, isEdge); - expr->accept(&visitor); - hasWrongType = visitor.hasWrongType(); - return std::move(visitor).expr(); + // RewriteSymExprVisitor visitor(qctx_->objPool(), sym, isEdge); + // expr->accept(&visitor); + // hasWrongType = visitor.hasWrongType(); + // return expr; + std::unordered_set invalidExprs{ + Expression::Kind::kVersionedVar, + Expression::Kind::kVarProperty, + Expression::Kind::kInputProperty, + Expression::Kind::kVar, + // Expression::Kind::kLabelAttribute, valid only for update edge + Expression::Kind::kAttribute, + Expression::Kind::kSubscript, + Expression::Kind::kUUID, + Expression::Kind::kTagProperty, + Expression::Kind::kLabelTagProperty, + Expression::Kind::kDstProperty, + Expression::Kind::kEdgeSrc, + Expression::Kind::kEdgeType, + Expression::Kind::kEdgeRank, + Expression::Kind::kEdgeDst, + }; + if (isEdge) { + invalidExprs.emplace(Expression::Kind::kSrcProperty); + } else { + invalidExprs.emplace(Expression::Kind::kLabelAttribute); + invalidExprs.emplace(Expression::Kind::kEdgeProperty); + } + auto *r = ExpressionUtils::findAny(expr, invalidExprs); + if (r != nullptr) { + hasWrongType = true; + return nullptr; + } + + auto *pool = qctx_->objPool(); + RewriteVisitor::Matcher matcher = [](const Expression *e) -> bool { + switch (e->kind()) { + case Expression::Kind::kLabel: + case Expression::Kind::kLabelAttribute: + return true; + default: + return false; + } + }; + RewriteVisitor::Rewriter rewriter = [pool, sym, isEdge](const Expression *e) -> Expression * { + switch (e->kind()) { + case Expression::Kind::kLabel: { + auto laExpr = static_cast(e); + if (isEdge) { + return EdgePropertyExpression::make(pool, sym, laExpr->name()); + } else { + return SourcePropertyExpression::make(pool, sym, laExpr->name()); + } + } + case Expression::Kind::kLabelAttribute: { + auto laExpr = static_cast(e); + if (isEdge) { + return EdgePropertyExpression::make( + pool, laExpr->left()->name(), laExpr->right()->value().getStr()); + } else { + return nullptr; + } + } + default: + return nullptr; + } + }; + auto *newExpr = RewriteVisitor::transform(expr, matcher, rewriter); + return newExpr; } Status UpdateVertexValidator::validateImpl() { diff --git a/src/graph/visitor/CMakeLists.txt b/src/graph/visitor/CMakeLists.txt index 01ccba3cd1a..23e6295f592 100644 --- a/src/graph/visitor/CMakeLists.txt +++ b/src/graph/visitor/CMakeLists.txt @@ -11,7 +11,6 @@ nebula_add_library( ExtractPropExprVisitor.cpp ExtractFilterExprVisitor.cpp FoldConstantExprVisitor.cpp - RewriteSymExprVisitor.cpp RewriteVisitor.cpp FindVisitor.cpp VidExtractVisitor.cpp diff --git a/src/graph/visitor/RewriteSymExprVisitor.cpp b/src/graph/visitor/RewriteSymExprVisitor.cpp deleted file mode 100644 index 5558bcc2999..00000000000 --- a/src/graph/visitor/RewriteSymExprVisitor.cpp +++ /dev/null @@ -1,350 +0,0 @@ -/* Copyright (c) 2020 vesoft inc. All rights reserved. - * - * This source code is licensed under Apache 2.0 License. - */ - -#include "graph/visitor/RewriteSymExprVisitor.h" - -namespace nebula { -namespace graph { - -RewriteSymExprVisitor::RewriteSymExprVisitor(ObjectPool *objPool, - const std::string &sym, - bool isEdge) - : pool_(objPool), sym_(sym), isEdge_(isEdge) {} - -void RewriteSymExprVisitor::visit(ConstantExpression *expr) { - UNUSED(expr); - expr_ = nullptr; -} - -void RewriteSymExprVisitor::visit(UnaryExpression *expr) { - expr->operand()->accept(this); - if (expr_) { - expr->setOperand(expr_); - } -} - -void RewriteSymExprVisitor::visit(TypeCastingExpression *expr) { - expr->operand()->accept(this); - if (expr_) { - expr->setOperand(expr_); - } -} - -void RewriteSymExprVisitor::visit(LabelExpression *expr) { - if (isEdge_) { - expr_ = EdgePropertyExpression::make(pool_, sym_, expr->name()); - } else { - expr_ = SourcePropertyExpression::make(pool_, sym_, expr->name()); - } -} - -void RewriteSymExprVisitor::visit(LabelAttributeExpression *expr) { - if (isEdge_) { - expr_ = - EdgePropertyExpression::make(pool_, expr->left()->name(), expr->right()->value().getStr()); - hasWrongType_ = false; - } else { - hasWrongType_ = true; - expr_ = nullptr; - } -} - -void RewriteSymExprVisitor::visit(ArithmeticExpression *expr) { - visitBinaryExpr(expr); -} - -void RewriteSymExprVisitor::visit(RelationalExpression *expr) { - visitBinaryExpr(expr); -} - -void RewriteSymExprVisitor::visit(SubscriptExpression *expr) { - UNUSED(expr); - hasWrongType_ = true; -} - -void RewriteSymExprVisitor::visit(AttributeExpression *expr) { - UNUSED(expr); - hasWrongType_ = true; -} - -void RewriteSymExprVisitor::visit(LogicalExpression *expr) { - auto &operands = expr->operands(); - for (auto i = 0u; i < operands.size(); i++) { - operands[i]->accept(this); - if (expr_) { - expr->setOperand(i, expr_); - } - } -} - -// function call -void RewriteSymExprVisitor::visit(FunctionCallExpression *expr) { - const auto &args = expr->args()->args(); - for (size_t i = 0; i < args.size(); ++i) { - auto &arg = args[i]; - arg->accept(this); - if (expr_) { - expr->args()->setArg(i, std::move(expr_)); - } - } -} - -void RewriteSymExprVisitor::visit(AggregateExpression *expr) { - auto *arg = expr->arg(); - arg->accept(this); - if (expr_) { - expr->setArg(std::move(expr_)); - } -} - -void RewriteSymExprVisitor::visit(UUIDExpression *expr) { - UNUSED(expr); - hasWrongType_ = true; -} - -// variable expression -void RewriteSymExprVisitor::visit(VariableExpression *expr) { - UNUSED(expr); - hasWrongType_ = true; -} - -void RewriteSymExprVisitor::visit(VersionedVariableExpression *expr) { - UNUSED(expr); - hasWrongType_ = true; -} - -// container expression -void RewriteSymExprVisitor::visit(ListExpression *expr) { - const auto &items = expr->items(); - for (size_t i = 0; i < items.size(); ++i) { - items[i]->accept(this); - if (expr_) { - expr->setItem(i, std::move(expr_)); - } - } -} - -void RewriteSymExprVisitor::visit(SetExpression *expr) { - const auto &items = expr->items(); - for (size_t i = 0; i < items.size(); ++i) { - items[i]->accept(this); - if (expr_) { - expr->setItem(i, std::move(expr_)); - } - } -} - -void RewriteSymExprVisitor::visit(MapExpression *expr) { - const auto &items = expr->items(); - for (size_t i = 0; i < items.size(); ++i) { - items[i].second->accept(this); - if (expr_) { - expr->setItem(i, {items[i].first, std::move(expr_)}); - } - } -} - -// property Expression -void RewriteSymExprVisitor::visit(TagPropertyExpression *expr) { - UNUSED(expr); - hasWrongType_ = true; -} - -void RewriteSymExprVisitor::visit(LabelTagPropertyExpression *expr) { - UNUSED(expr); - hasWrongType_ = true; -} - -void RewriteSymExprVisitor::visit(EdgePropertyExpression *expr) { - UNUSED(expr); - if (!isEdge_) { - hasWrongType_ = true; - } -} - -void RewriteSymExprVisitor::visit(InputPropertyExpression *expr) { - UNUSED(expr); - hasWrongType_ = true; -} - -void RewriteSymExprVisitor::visit(VariablePropertyExpression *expr) { - UNUSED(expr); - hasWrongType_ = true; -} - -void RewriteSymExprVisitor::visit(DestPropertyExpression *expr) { - UNUSED(expr); - hasWrongType_ = true; -} - -void RewriteSymExprVisitor::visit(SourcePropertyExpression *expr) { - UNUSED(expr); - if (isEdge_) { - hasWrongType_ = true; - } -} - -void RewriteSymExprVisitor::visit(EdgeSrcIdExpression *expr) { - UNUSED(expr); - hasWrongType_ = true; -} - -void RewriteSymExprVisitor::visit(EdgeTypeExpression *expr) { - UNUSED(expr); - hasWrongType_ = true; -} - -void RewriteSymExprVisitor::visit(EdgeRankExpression *expr) { - UNUSED(expr); - hasWrongType_ = true; -} - -void RewriteSymExprVisitor::visit(EdgeDstIdExpression *expr) { - UNUSED(expr); - hasWrongType_ = true; -} - -void RewriteSymExprVisitor::visit(VertexExpression *expr) { - UNUSED(expr); - expr_ = nullptr; -} - -void RewriteSymExprVisitor::visit(EdgeExpression *expr) { - UNUSED(expr); - expr_ = nullptr; -} - -void RewriteSymExprVisitor::visit(ColumnExpression *expr) { - UNUSED(expr); - expr_ = nullptr; -} - -void RewriteSymExprVisitor::visit(CaseExpression *expr) { - if (expr->hasCondition()) { - expr->condition()->accept(this); - if (expr_) { - expr->setCondition(expr_); - } - } - if (expr->hasDefault()) { - expr->defaultResult()->accept(this); - if (expr_) { - expr->setDefault(expr_); - } - } - auto &cases = expr->cases(); - for (size_t i = 0; i < cases.size(); ++i) { - auto when = cases[i].when; - auto then = cases[i].then; - when->accept(this); - if (expr_) { - expr->setWhen(i, expr_); - } - then->accept(this); - if (expr_) { - expr->setThen(i, expr_); - } - } -} - -void RewriteSymExprVisitor::visitBinaryExpr(BinaryExpression *expr) { - expr->left()->accept(this); - if (expr_) { - expr->setLeft(expr_); - } - expr->right()->accept(this); - if (expr_) { - expr->setRight(expr_); - } -} - -void RewriteSymExprVisitor::visit(PathBuildExpression *expr) { - const auto &items = expr->items(); - for (size_t i = 0; i < items.size(); ++i) { - items[i]->accept(this); - if (expr_) { - expr->setItem(i, std::move(expr_)); - } - } -} - -void RewriteSymExprVisitor::visit(ListComprehensionExpression *expr) { - expr->collection()->accept(this); - if (expr_) { - expr->setCollection(expr_); - } - if (expr->hasFilter()) { - expr->filter()->accept(this); - if (expr_) { - expr->setFilter(expr_); - } - } - if (expr->hasMapping()) { - expr->mapping()->accept(this); - if (expr_) { - expr->setMapping(expr_); - } - } -} - -void RewriteSymExprVisitor::visit(PredicateExpression *expr) { - expr->collection()->accept(this); - if (expr_) { - expr->setCollection(expr_); - } - if (expr->hasFilter()) { - expr->filter()->accept(this); - if (expr_) { - expr->setFilter(expr_); - } - } -} - -void RewriteSymExprVisitor::visit(ReduceExpression *expr) { - expr->initial()->accept(this); - if (expr_) { - expr->setInitial(expr_); - } - expr->collection()->accept(this); - if (expr_) { - expr->setCollection(expr_); - } - expr->mapping()->accept(this); - if (expr_) { - expr->setMapping(expr_); - } -} - -void RewriteSymExprVisitor::visit(SubscriptRangeExpression *expr) { - expr->list()->accept(this); - if (expr_) { - expr->setList(expr_); - } - if (expr->lo() != nullptr) { - expr->lo()->accept(this); - if (expr_) { - expr->setLo(expr_); - } - } - if (expr->hi() != nullptr) { - expr->hi()->accept(this); - if (expr_) { - expr->setHi(expr_); - } - } -} - -void RewriteSymExprVisitor::visit(MatchPathPatternExpression *expr) { - if (expr->genList() != nullptr) { - expr->genList()->accept(this); - if (expr_) { - expr->setGenList(expr_); - expr_ = nullptr; - } - } -} - -} // namespace graph -} // namespace nebula diff --git a/src/graph/visitor/RewriteSymExprVisitor.h b/src/graph/visitor/RewriteSymExprVisitor.h deleted file mode 100644 index 4a45a2f11a9..00000000000 --- a/src/graph/visitor/RewriteSymExprVisitor.h +++ /dev/null @@ -1,98 +0,0 @@ -/* Copyright (c) 2020 vesoft inc. All rights reserved. - * - * This source code is licensed under Apache 2.0 License. - */ - -#ifndef GRAPH_VISITOR_REWRITESYMEXPRVISITOR_H_ -#define GRAPH_VISITOR_REWRITESYMEXPRVISITOR_H_ - -#include - -#include "common/expression/ExprVisitor.h" - -namespace nebula { - -class Expression; - -namespace graph { - -class RewriteSymExprVisitor final : public ExprVisitor { - public: - RewriteSymExprVisitor(ObjectPool *objPool, const std::string &sym, bool isEdge); - - bool hasWrongType() const { - return hasWrongType_; - } - - Expression *expr() { - return expr_; - } - - void visit(ConstantExpression *expr) override; - void visit(UnaryExpression *expr) override; - void visit(TypeCastingExpression *expr) override; - void visit(LabelExpression *expr) override; - void visit(LabelAttributeExpression *expr) override; - // binary expression - void visit(ArithmeticExpression *expr) override; - void visit(RelationalExpression *expr) override; - void visit(SubscriptExpression *expr) override; - void visit(AttributeExpression *expr) override; - void visit(LogicalExpression *expr) override; - // function call - void visit(FunctionCallExpression *expr) override; - void visit(AggregateExpression *expr) override; - void visit(UUIDExpression *expr) override; - // variable expression - void visit(VariableExpression *expr) override; - void visit(VersionedVariableExpression *expr) override; - // container expression - void visit(ListExpression *expr) override; - void visit(SetExpression *expr) override; - void visit(MapExpression *expr) override; - // property Expression - void visit(LabelTagPropertyExpression *expr) override; - void visit(TagPropertyExpression *expr) override; - void visit(EdgePropertyExpression *expr) override; - void visit(InputPropertyExpression *expr) override; - void visit(VariablePropertyExpression *expr) override; - void visit(DestPropertyExpression *expr) override; - void visit(SourcePropertyExpression *expr) override; - void visit(EdgeSrcIdExpression *expr) override; - void visit(EdgeTypeExpression *expr) override; - void visit(EdgeRankExpression *expr) override; - void visit(EdgeDstIdExpression *expr) override; - // vertex/edge expression - void visit(VertexExpression *expr) override; - void visit(EdgeExpression *expr) override; - // case expression - void visit(CaseExpression *expr) override; - // path build expression - void visit(PathBuildExpression *expr) override; - // column expression - void visit(ColumnExpression *expr) override; - // predicate expression - void visit(PredicateExpression *expr) override; - // list comprehension expression - void visit(ListComprehensionExpression *expr) override; - // reduce expression - void visit(ReduceExpression *expr) override; - // subscript range expression - void visit(SubscriptRangeExpression *expr) override; - // match path pattern expression - void visit(MatchPathPatternExpression *expr) override; - - private: - void visitBinaryExpr(BinaryExpression *expr); - - ObjectPool *pool_; - const std::string &sym_; - bool hasWrongType_{false}; - bool isEdge_{false}; - Expression *expr_{nullptr}; -}; - -} // namespace graph -} // namespace nebula - -#endif // GRAPH_VISITOR_REWRITESYMEXPRVISITOR_H_ diff --git a/tests/tck/features/bugfix/CompareDate.feature b/tests/tck/features/bugfix/CompareDate.feature new file mode 100644 index 00000000000..32f04db9ee2 --- /dev/null +++ b/tests/tck/features/bugfix/CompareDate.feature @@ -0,0 +1,30 @@ +# Copyright (c) 2022 vesoft inc. All rights reserved. +# +# This source code is licensed under Apache 2.0 License. +Feature: Compare date value + + # #5046 + Scenario: Compare date value + Given an empty graph + And create a space with following options: + | partition_num | 1 | + | replica_factor | 1 | + | vid_type | FIXED_STRING(30) | + | charset | utf8 | + | collate | utf8_bin | + When executing query: + """ + create tag date_comp(i1 int, d1 date); + """ + Then the execution should be successful + When try to execute query: + """ + INSERT VERTEX date_comp(i1, d1) VALUES 'xxx':(1, date()); + """ + Then the execution should be successful + When try to execute query: + """ + UPDATE VERTEX ON date_comp 'xxx' SET i1=3 WHEN d1 == date() + YIELD i1; + """ + Then the execution should be successful From 3ed2d2c46c7f157b7e740a858f4bf511d608b767 Mon Sep 17 00:00:00 2001 From: Shylock Hg <33566796+Shylock-Hg@users.noreply.github.com> Date: Mon, 19 Dec 2022 15:45:32 +0800 Subject: [PATCH 2/2] Remove comments. --- src/graph/validator/MutateValidator.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/graph/validator/MutateValidator.cpp b/src/graph/validator/MutateValidator.cpp index 508fba56459..486dd72e57c 100644 --- a/src/graph/validator/MutateValidator.cpp +++ b/src/graph/validator/MutateValidator.cpp @@ -752,10 +752,6 @@ Expression *UpdateValidator::rewriteSymExpr(Expression *expr, const std::string &sym, bool &hasWrongType, bool isEdge) { - // RewriteSymExprVisitor visitor(qctx_->objPool(), sym, isEdge); - // expr->accept(&visitor); - // hasWrongType = visitor.hasWrongType(); - // return expr; std::unordered_set invalidExprs{ Expression::Kind::kVersionedVar, Expression::Kind::kVarProperty,