diff --git a/src/context/Iterator.cpp b/src/context/Iterator.cpp index 9fc92de07..344849276 100644 --- a/src/context/Iterator.cpp +++ b/src/context/Iterator.cpp @@ -298,6 +298,9 @@ const Value& GetNeighborsIter::getTagProp(const std::string& tag, auto colId = index->second.colIdx; auto& row = *currentRow_; DCHECK_GT(row.size(), colId); + if (row[colId].empty()) { + return Value::kEmpty; + } if (!row[colId].isList()) { return Value::kNullBadType; } diff --git a/src/context/Symbols.h b/src/context/Symbols.h index b77073b71..4b6ca906a 100644 --- a/src/context/Symbols.h +++ b/src/context/Symbols.h @@ -47,16 +47,8 @@ struct Variable { std::unordered_set readBy; std::unordered_set writtenBy; - // None means will used in later - // non-positive means static lifetime - // positive means last user id - folly::Optional lastUser; - - void setLastUser(int64_t id) { - if (!lastUser.hasValue()) { - lastUser = id; - } - } + // the count of use the variable + std::atomic userCount; }; class SymbolTable final { diff --git a/src/executor/Executor.cpp b/src/executor/Executor.cpp index 6323d3c6f..08ddf02e4 100644 --- a/src/executor/Executor.cpp +++ b/src/executor/Executor.cpp @@ -8,6 +8,7 @@ #include #include +#include #include "common/base/Memory.h" #include "common/base/ObjectPool.h" @@ -581,16 +582,20 @@ folly::Future Executor::error(Status status) const { void Executor::drop() { for (const auto &inputVar : node()->inputVars()) { if (inputVar != nullptr) { - if (inputVar->lastUser.value() == node()->id()) { - ectx_->dropResult(inputVar->name); - VLOG(1) << "Drop variable " << node()->outputVar(); + // Make sure use the variable happened-before decrement count + if (inputVar->userCount.fetch_sub(1, std::memory_order_release) == 1) { + // Make sure drop happened-after count decrement + CHECK_EQ(inputVar->userCount.load(std::memory_order_acquire), 0); + ectx_->dropResult(inputVar->name); + VLOG(1) << "Drop variable " << node()->outputVar(); } } } } Status Executor::finish(Result &&result) { - if (!FLAGS_enable_lifetime_optimize || node()->outputVarPtr()->lastUser.hasValue()) { + if (!FLAGS_enable_lifetime_optimize || + node()->outputVarPtr()->userCount.load(std::memory_order_relaxed) != 0) { numRows_ = result.size(); ectx_->setResult(node()->outputVar(), std::move(result)); } else { diff --git a/src/executor/query/IndexScanExecutor.cpp b/src/executor/query/IndexScanExecutor.cpp index 2d270d313..7fce3bd45 100644 --- a/src/executor/query/IndexScanExecutor.cpp +++ b/src/executor/query/IndexScanExecutor.cpp @@ -6,6 +6,8 @@ #include "executor/query/IndexScanExecutor.h" +#include + #include "planner/plan/PlanNode.h" #include "context/QueryContext.h" #include "service/GraphFlags.h" @@ -28,8 +30,16 @@ folly::Future IndexScanExecutor::indexScan() { DataSet dataSet({"dummy"}); return finish(ResultBuilder().value(Value(std::move(dataSet))).finish()); } + + const auto &ictxs = lookup->queryContext(); + auto iter = std::find_if( + ictxs.begin(), ictxs.end(), [](auto &ictx) { return !ictx.index_id_ref().is_set(); }); + if (ictxs.empty() || iter != ictxs.end()) { + return Status::Error("There is no index to use at runtime"); + } + return storageClient->lookupIndex(lookup->space(), - lookup->queryContext(), + ictxs, lookup->isEdge(), lookup->schemaId(), lookup->returnColumns()) diff --git a/src/executor/test/FilterTest.cpp b/src/executor/test/FilterTest.cpp index 8788e1d05..7fc830a82 100644 --- a/src/executor/test/FilterTest.cpp +++ b/src/executor/test/FilterTest.cpp @@ -25,15 +25,14 @@ class FilterTest : public QueryTestBase { #define FILTER_RESUTL_CHECK(inputName, outputName, sentence, expected) \ do { \ qctx_->symTable()->newVariable(outputName); \ - auto* pool = qctx_->objPool(); \ auto yieldSentence = getYieldSentence(sentence, qctx_.get()); \ auto columns = yieldSentence->columns(); \ for (auto& col : columns) { \ - col->setExpr(ExpressionUtils::rewriteLabelAttr2EdgeProp(pool, col->expr())); \ + col->setExpr(ExpressionUtils::rewriteLabelAttr2EdgeProp(col->expr())); \ } \ auto* whereSentence = yieldSentence->where(); \ whereSentence->setFilter( \ - ExpressionUtils::rewriteLabelAttr2EdgeProp(pool, whereSentence->filter())); \ + ExpressionUtils::rewriteLabelAttr2EdgeProp(whereSentence->filter())); \ auto* filterNode = Filter::make(qctx_.get(), nullptr, yieldSentence->where()->filter()); \ filterNode->setInputVar(inputName); \ filterNode->setOutputVar(outputName); \ diff --git a/src/optimizer/rule/PushFilterDownLeftJoinRule.cpp b/src/optimizer/rule/PushFilterDownLeftJoinRule.cpp index 7d2cf6258..40d7073fa 100644 --- a/src/optimizer/rule/PushFilterDownLeftJoinRule.cpp +++ b/src/optimizer/rule/PushFilterDownLeftJoinRule.cpp @@ -48,7 +48,6 @@ StatusOr PushFilterDownLeftJoinRule::transform( const std::pair& leftVar = oldLeftJoinNode->leftVar(); auto symTable = octx->qctx()->symTable(); std::vector leftVarColNames = symTable->getVar(leftVar.first)->colNames; - auto objPool = octx->qctx()->objPool(); // split the `condition` based on whether the varPropExpr comes from the left child auto picker = [&leftVarColNames](const Expression* e) -> bool { @@ -73,7 +72,7 @@ StatusOr PushFilterDownLeftJoinRule::transform( }; Expression* filterPicked = nullptr; Expression* filterUnpicked = nullptr; - graph::ExpressionUtils::splitFilter(objPool, condition, picker, &filterPicked, &filterUnpicked); + graph::ExpressionUtils::splitFilter(condition, picker, &filterPicked, &filterUnpicked); if (!filterPicked) { return TransformResult::noTransform(); @@ -83,7 +82,7 @@ StatusOr PushFilterDownLeftJoinRule::transform( auto* newLeftFilterNode = graph::Filter::make( octx->qctx(), const_cast(oldLeftJoinNode->dep()), - graph::ExpressionUtils::rewriteInnerVar(objPool, filterPicked, leftVar.first)); + graph::ExpressionUtils::rewriteInnerVar(filterPicked, leftVar.first)); newLeftFilterNode->setInputVar(leftVar.first); newLeftFilterNode->setColNames(leftVarColNames); auto newFilterGroup = OptGroup::create(octx); @@ -100,7 +99,7 @@ StatusOr PushFilterDownLeftJoinRule::transform( std::vector newHashKeys; for (auto* k : hashKeys) { newHashKeys.emplace_back( - graph::ExpressionUtils::rewriteInnerVar(objPool, k, newLeftFilterOutputVar)); + graph::ExpressionUtils::rewriteInnerVar(k, newLeftFilterOutputVar)); } newLeftJoinNode->setHashKeys(newHashKeys); diff --git a/src/optimizer/rule/PushFilterDownProjectRule.cpp b/src/optimizer/rule/PushFilterDownProjectRule.cpp index 739f3087a..89aa1d6e6 100644 --- a/src/optimizer/rule/PushFilterDownProjectRule.cpp +++ b/src/optimizer/rule/PushFilterDownProjectRule.cpp @@ -43,7 +43,6 @@ StatusOr PushFilterDownProjectRule::transform( DCHECK_EQ(projNode->kind(), PlanNode::Kind::kProject); const auto* oldProjNode = static_cast(projNode); const auto* condition = oldFilterNode->condition(); - auto objPool = octx->qctx()->objPool(); auto projColNames = oldProjNode->colNames(); auto projColumns = oldProjNode->columns()->columns(); @@ -86,7 +85,7 @@ StatusOr PushFilterDownProjectRule::transform( }; Expression* filterPicked = nullptr; Expression* filterUnpicked = nullptr; - graph::ExpressionUtils::splitFilter(objPool, condition, picker, &filterPicked, &filterUnpicked); + graph::ExpressionUtils::splitFilter(condition, picker, &filterPicked, &filterUnpicked); if (!filterPicked) { return TransformResult::noTransform(); diff --git a/src/planner/match/LabelIndexSeek.cpp b/src/planner/match/LabelIndexSeek.cpp index 056d6d777..82af49188 100644 --- a/src/planner/match/LabelIndexSeek.cpp +++ b/src/planner/match/LabelIndexSeek.cpp @@ -117,8 +117,7 @@ StatusOr LabelIndexSeek::transformNode(NodeContext* nodeCtx) { } } if (canBeEmbeded2IndexScan) { - auto* srcFilter = - ExpressionUtils::rewriteLabelAttr2TagProp(pool, flattenFilter); + auto* srcFilter = ExpressionUtils::rewriteLabelAttr2TagProp(flattenFilter); storage::cpp2::IndexQueryContext ctx; ctx.set_filter(Expression::encode(*srcFilter)); scan->setIndexQueryContext({ctx}); diff --git a/src/planner/match/MatchPlanner.cpp b/src/planner/match/MatchPlanner.cpp index 4a023ad1d..ccad81301 100644 --- a/src/planner/match/MatchPlanner.cpp +++ b/src/planner/match/MatchPlanner.cpp @@ -67,15 +67,17 @@ StatusOr MatchPlanner::transform(AstContext* astCtx) { } } - auto finalPlan = connectSegments(subplans, matchCtx->clauses); + auto finalPlan = connectSegments(astCtx, subplans, matchCtx->clauses); NG_RETURN_IF_ERROR(finalPlan); return std::move(finalPlan).value(); } StatusOr MatchPlanner::connectSegments( + AstContext* astCtx, std::vector& subplans, std::vector>& clauses) { DCHECK(!subplans.empty()); + subplans.front().tail->setInputVar(astCtx->qctx->vctx()->anonVarGen()->getVar()); if (subplans.size() == 1) { return subplans.front(); } diff --git a/src/planner/match/MatchPlanner.h b/src/planner/match/MatchPlanner.h index 0d97659de..0dfabdef9 100644 --- a/src/planner/match/MatchPlanner.h +++ b/src/planner/match/MatchPlanner.h @@ -24,6 +24,7 @@ class MatchPlanner final : public Planner { private: StatusOr connectSegments( + AstContext* astCtx, std::vector& subplans, std::vector>& clauses); }; diff --git a/src/scheduler/AsyncMsgNotifyBasedScheduler.cpp b/src/scheduler/AsyncMsgNotifyBasedScheduler.cpp index eea6c9eb2..e658079d9 100644 --- a/src/scheduler/AsyncMsgNotifyBasedScheduler.cpp +++ b/src/scheduler/AsyncMsgNotifyBasedScheduler.cpp @@ -16,7 +16,9 @@ AsyncMsgNotifyBasedScheduler::AsyncMsgNotifyBasedScheduler(QueryContext* qctx) : folly::Future AsyncMsgNotifyBasedScheduler::schedule() { if (FLAGS_enable_lifetime_optimize) { - qctx_->plan()->root()->outputVarPtr()->setLastUser(-1); // special for root + // special for root + qctx_->plan()->root()->outputVarPtr()->userCount.store(std::numeric_limits::max(), + std::memory_order_relaxed); analyzeLifetime(qctx_->plan()->root()); } auto executor = Executor::create(qctx_->plan()->root(), qctx_); diff --git a/src/scheduler/Scheduler.cpp b/src/scheduler/Scheduler.cpp index d76325367..a233accf5 100644 --- a/src/scheduler/Scheduler.cpp +++ b/src/scheduler/Scheduler.cpp @@ -5,6 +5,8 @@ */ #include "scheduler/Scheduler.h" +#include +#include #include "context/QueryContext.h" #include "executor/ExecutionError.h" @@ -28,10 +30,15 @@ namespace graph { const auto currentInLoop = std::get<1>(current); for (auto& inputVar : currentNode->inputVars()) { if (inputVar != nullptr) { - inputVar->setLastUser( - (currentNode->kind() == PlanNode::Kind::kLoop || currentInLoop) - ? -1 - : currentNode->id()); + if (currentNode->kind() == PlanNode::Kind::kLoop || currentInLoop) { + inputVar->userCount.store(std::numeric_limits::max(), + std::memory_order_relaxed); + } else { + if (inputVar->userCount.load(std::memory_order_relaxed) != + std::numeric_limits::max()) { + inputVar->userCount.fetch_add(1, std::memory_order_relaxed); + } + } } } stack.pop(); @@ -42,13 +49,18 @@ namespace graph { switch (currentNode->kind()) { case PlanNode::Kind::kSelect: { auto sel = static_cast(currentNode); + // used by scheduler + sel->outputVarPtr()->userCount.store(std::numeric_limits::max(), + std::memory_order_relaxed); stack.push(std::make_tuple(sel->then(), currentInLoop)); stack.push(std::make_tuple(sel->otherwise(), currentInLoop)); break; } case PlanNode::Kind::kLoop: { auto loop = static_cast(currentNode); - loop->outputVarPtr()->setLastUser(-1); + // used by scheduler + loop->outputVarPtr()->userCount.store(std::numeric_limits::max(), + std::memory_order_relaxed); stack.push(std::make_tuple(loop->body(), true)); break; } diff --git a/src/util/ExpressionUtils.cpp b/src/util/ExpressionUtils.cpp index 6bdae94d9..90bf825fd 100644 --- a/src/util/ExpressionUtils.cpp +++ b/src/util/ExpressionUtils.cpp @@ -109,11 +109,12 @@ bool ExpressionUtils::isEvaluableExpr(const Expression *expr) { } // rewrite LabelAttr to EdgeProp -Expression *ExpressionUtils::rewriteLabelAttr2EdgeProp(ObjectPool *pool, const Expression *expr) { +Expression *ExpressionUtils::rewriteLabelAttr2EdgeProp(const Expression *expr) { + ObjectPool *pool = expr->getObjPool(); auto matcher = [](const Expression *e) -> bool { return e->kind() == Expression::Kind::kLabelAttribute; }; - auto rewriter = [&pool](const Expression *e) -> Expression * { + auto rewriter = [pool](const Expression *e) -> Expression * { DCHECK_EQ(e->kind(), Expression::Kind::kLabelAttribute); auto labelAttrExpr = static_cast(e); auto leftName = labelAttrExpr->left()->name(); @@ -125,13 +126,12 @@ Expression *ExpressionUtils::rewriteLabelAttr2EdgeProp(ObjectPool *pool, const E } // rewrite var in VariablePropExpr to another var -Expression *ExpressionUtils::rewriteInnerVar(ObjectPool *pool, - const Expression *expr, - std::string newVar) { +Expression *ExpressionUtils::rewriteInnerVar(const Expression *expr, std::string newVar) { + ObjectPool* pool = expr->getObjPool(); auto matcher = [](const Expression *e) -> bool { return e->kind() == Expression::Kind::kVarProperty; }; - auto rewriter = [&](const Expression *e) -> Expression * { + auto rewriter = [pool, newVar](const Expression *e) -> Expression * { DCHECK_EQ(e->kind(), Expression::Kind::kVarProperty); auto varPropExpr = static_cast(e); auto newProp = varPropExpr->prop(); @@ -142,11 +142,12 @@ Expression *ExpressionUtils::rewriteInnerVar(ObjectPool *pool, } // rewrite LabelAttr to tagProp -Expression *ExpressionUtils::rewriteLabelAttr2TagProp(ObjectPool* pool, const Expression *expr) { +Expression *ExpressionUtils::rewriteLabelAttr2TagProp(const Expression *expr) { + ObjectPool* pool = expr->getObjPool(); auto matcher = [](const Expression *e) -> bool { return e->kind() == Expression::Kind::kLabelAttribute; }; - auto rewriter = [&pool](const Expression *e) -> Expression * { + auto rewriter = [pool](const Expression *e) -> Expression * { DCHECK_EQ(e->kind(), Expression::Kind::kLabelAttribute); auto labelAttrExpr = static_cast(e); auto leftName = labelAttrExpr->left()->name(); @@ -158,19 +159,20 @@ Expression *ExpressionUtils::rewriteLabelAttr2TagProp(ObjectPool* pool, const Ex } // rewrite Agg to VarProp -Expression *ExpressionUtils::rewriteAgg2VarProp(ObjectPool *pool, const Expression *expr) { +Expression *ExpressionUtils::rewriteAgg2VarProp(const Expression *expr) { + ObjectPool* pool = expr->getObjPool(); auto matcher = [](const Expression *e) -> bool { return e->kind() == Expression::Kind::kAggregate; }; - auto rewriter = [&pool](const Expression *e) -> Expression * { + auto rewriter = [pool](const Expression *e) -> Expression * { return VariablePropertyExpression::make(pool, "", e->toString()); }; return RewriteVisitor::transform(expr, std::move(matcher), std::move(rewriter)); } -StatusOr ExpressionUtils::foldConstantExpr(ObjectPool *objPool, - const Expression *expr) { +StatusOr ExpressionUtils::foldConstantExpr(const Expression *expr) { + ObjectPool* objPool = expr->getObjPool(); auto newExpr = expr->clone(); FoldConstantExprVisitor visitor(objPool); newExpr->accept(&visitor); @@ -187,7 +189,7 @@ StatusOr ExpressionUtils::foldConstantExpr(ObjectPool *objPool, return newExpr; } -Expression *ExpressionUtils::reduceUnaryNotExpr(const Expression *expr, ObjectPool *pool) { +Expression *ExpressionUtils::reduceUnaryNotExpr(const Expression *expr) { // Match the operand auto operandMatcher = [](const Expression *operandExpr) -> bool { return (operandExpr->kind() == Expression::Kind::kUnaryNot || @@ -214,10 +216,10 @@ Expression *ExpressionUtils::reduceUnaryNotExpr(const Expression *expr, ObjectPo reducedExpr = castedExpr->operand(); } else if (reducedExpr->isRelExpr() && reducedExpr->kind() != Expression::Kind::kRelREG) { auto castedExpr = static_cast(reducedExpr); - reducedExpr = reverseRelExpr(pool, castedExpr); + reducedExpr = reverseRelExpr(castedExpr); } else if (reducedExpr->isLogicalExpr()) { auto castedExpr = static_cast(reducedExpr); - reducedExpr = reverseLogicalExpr(pool, castedExpr); + reducedExpr = reverseLogicalExpr(castedExpr); } // Rewrite the output of rewrite if possible return operandMatcher(reducedExpr) @@ -228,7 +230,8 @@ Expression *ExpressionUtils::reduceUnaryNotExpr(const Expression *expr, ObjectPo return RewriteVisitor::transform(expr, rootMatcher, rewriter); } -Expression *ExpressionUtils::rewriteRelExpr(const Expression *expr, ObjectPool *pool) { +Expression *ExpressionUtils::rewriteRelExpr(const Expression *expr) { + ObjectPool* pool = expr->getObjPool(); // Match relational expressions containing at least one airthmetic expr auto matcher = [](const Expression *e) -> bool { if (e->isRelExpr()) { @@ -247,7 +250,7 @@ Expression *ExpressionUtils::rewriteRelExpr(const Expression *expr, ObjectPool * }; // Simplify relational expressions involving boolean literals - auto simplifyBoolOperand = [&pool](RelationalExpression *relExpr, + auto simplifyBoolOperand = [pool](RelationalExpression *relExpr, Expression *lExpr, Expression *rExpr) -> Expression * { QueryExpressionContext ctx(nullptr); @@ -288,7 +291,7 @@ Expression *ExpressionUtils::rewriteRelExpr(const Expression *expr, ObjectPool * } // Move all evaluable expression to the right side auto relRightOperandExpr = relExpr->right()->clone(); - auto relLeftOperandExpr = rewriteRelExprHelper(pool, relExpr->left(), relRightOperandExpr); + auto relLeftOperandExpr = rewriteRelExprHelper(relExpr->left(), relRightOperandExpr); return RelationalExpression::makeKind( pool, relExpr->kind(), relLeftOperandExpr->clone(), relRightOperandExpr->clone()); }; @@ -296,9 +299,9 @@ Expression *ExpressionUtils::rewriteRelExpr(const Expression *expr, ObjectPool * return RewriteVisitor::transform(expr, matcher, rewriter); } -Expression *ExpressionUtils::rewriteRelExprHelper(ObjectPool *pool, - const Expression *expr, +Expression *ExpressionUtils::rewriteRelExprHelper(const Expression *expr, Expression *&relRightOperandExpr) { + ObjectPool* pool = expr->getObjPool(); // TODO: Support rewrite mul/div expressoion after fixing overflow auto matcher = [](const Expression *e) -> bool { if (!e->isArithmeticExpr() || e->kind() == Expression::Kind::kMultiply || @@ -342,19 +345,19 @@ Expression *ExpressionUtils::rewriteRelExprHelper(ObjectPool *pool, break; } - return rewriteRelExprHelper(pool, root, relRightOperandExpr); + return rewriteRelExprHelper(root, relRightOperandExpr); } -StatusOr ExpressionUtils::filterTransform(const Expression *filter, ObjectPool *pool) { +StatusOr ExpressionUtils::filterTransform(const Expression *filter) { auto rewrittenExpr = const_cast(filter); // Rewrite relational expression - rewrittenExpr = rewriteRelExpr(rewrittenExpr, pool); + rewrittenExpr = rewriteRelExpr(rewrittenExpr); // Fold constant expression - auto constantFoldRes = foldConstantExpr(pool, rewrittenExpr); + auto constantFoldRes = foldConstantExpr(rewrittenExpr); NG_RETURN_IF_ERROR(constantFoldRes); rewrittenExpr = constantFoldRes.value(); // Reduce Unary expression - rewrittenExpr = reduceUnaryNotExpr(rewrittenExpr, pool); + rewrittenExpr = reduceUnaryNotExpr(rewrittenExpr); return rewrittenExpr; } @@ -428,11 +431,11 @@ Expression* ExpressionUtils::flattenInnerLogicalExpr(const Expression *expr) { } // pick the subparts of expression that meet picker's criteria -void ExpressionUtils::splitFilter(ObjectPool *pool, - const Expression *expr, +void ExpressionUtils::splitFilter(const Expression *expr, std::function picker, Expression **filterPicked, Expression **filterUnpicked) { + ObjectPool* pool = expr->getObjPool(); // Pick the non-LogicalAndExpr directly if (expr->kind() != Expression::Kind::kLogicalAnd) { if (picker(expr)) { @@ -641,8 +644,8 @@ bool ExpressionUtils::findInnerRandFunction(const Expression *expr) { } // Negate the given relational expr -RelationalExpression *ExpressionUtils::reverseRelExpr(ObjectPool *pool, - RelationalExpression *expr) { +RelationalExpression *ExpressionUtils::reverseRelExpr(RelationalExpression *expr) { + ObjectPool *pool = expr->getObjPool(); auto left = static_cast(expr)->left(); auto right = static_cast(expr)->right(); auto negatedKind = getNegatedRelExprKind(expr->kind()); @@ -706,7 +709,8 @@ Expression::Kind ExpressionUtils::getNegatedArithmeticType(const Expression::Kin } } -LogicalExpression*ExpressionUtils::reverseLogicalExpr(ObjectPool* pool, LogicalExpression *expr) { +LogicalExpression*ExpressionUtils::reverseLogicalExpr(LogicalExpression *expr) { + ObjectPool* pool = expr->getObjPool(); std::vector operands; if (expr->kind() == Expression::Kind::kLogicalAnd) { pullAnds(expr); diff --git a/src/util/ExpressionUtils.h b/src/util/ExpressionUtils.h index c2d04ed5a..a51c7380b 100644 --- a/src/util/ExpressionUtils.h +++ b/src/util/ExpressionUtils.h @@ -56,36 +56,33 @@ class ExpressionUtils { static bool isEvaluableExpr(const Expression* expr); - static Expression* rewriteLabelAttr2TagProp(ObjectPool* pool, const Expression* expr); + static Expression* rewriteLabelAttr2TagProp(const Expression* expr); - static Expression* rewriteLabelAttr2EdgeProp(ObjectPool* pool, const Expression* expr); + static Expression* rewriteLabelAttr2EdgeProp(const Expression* expr); - static Expression* rewriteAgg2VarProp(ObjectPool* pool, const Expression* expr); + static Expression* rewriteAgg2VarProp(const Expression* expr); - static Expression* rewriteInnerVar(ObjectPool* pool, - const Expression* expr, - std::string newVar); + static Expression* rewriteInnerVar(const Expression* expr, std::string newVar); // Rewrite relational expression, gather evaluable expressions to one side - static Expression* rewriteRelExpr(const Expression* expr, ObjectPool* pool); - static Expression* rewriteRelExprHelper(ObjectPool* pool, - const Expression* expr, + static Expression* rewriteRelExpr(const Expression* expr); + static Expression* rewriteRelExprHelper(const Expression* expr, Expression*& relRightOperandExpr); // Clone and fold constant expression - static StatusOr foldConstantExpr(ObjectPool* objPool, const Expression* expr); + static StatusOr foldConstantExpr(const Expression* expr); // Clone and reduce unaryNot expression - static Expression* reduceUnaryNotExpr(const Expression* expr, ObjectPool* pool); + static Expression* reduceUnaryNotExpr(const Expression* expr); // Transform filter using multiple expression rewrite strategies - static StatusOr filterTransform(const Expression* expr, ObjectPool* objPool); + static StatusOr filterTransform(const Expression* expr); // Negate the given logical expr: (A && B) -> (!A || !B) - static LogicalExpression* reverseLogicalExpr(ObjectPool* pool, LogicalExpression* expr); + static LogicalExpression* reverseLogicalExpr(LogicalExpression* expr); // Negate the given relational expr: (A > B) -> (A <= B) - static RelationalExpression* reverseRelExpr(ObjectPool* pool, RelationalExpression* expr); + static RelationalExpression* reverseRelExpr(RelationalExpression* expr); // Return the negation of the given relational kind static Expression::Kind getNegatedRelExprKind(const Expression::Kind kind); @@ -119,8 +116,7 @@ class ExpressionUtils { static Expression* flattenInnerLogicalExpr(const Expression* expr); - static void splitFilter(ObjectPool* pool, - const Expression* expr, + static void splitFilter(const Expression* expr, std::function picker, Expression** filterPicked, Expression** filterUnpicked); diff --git a/src/util/test/ExpressionUtilsTest.cpp b/src/util/test/ExpressionUtilsTest.cpp index 23fa4b26d..57c703618 100644 --- a/src/util/test/ExpressionUtilsTest.cpp +++ b/src/util/test/ExpressionUtilsTest.cpp @@ -519,7 +519,7 @@ TEST_F(ExpressionUtilsTest, splitFilter) { }; Expression *newExpr1 = nullptr; Expression *newExpr2 = nullptr; - ExpressionUtils::splitFilter(pool, expr, picker, &newExpr1, &newExpr2); + ExpressionUtils::splitFilter(expr, picker, &newExpr1, &newExpr2); ASSERT_EQ(*expected1, *newExpr1); ASSERT_EQ(*second, *newExpr2); } @@ -534,7 +534,7 @@ TEST_F(ExpressionUtilsTest, splitFilter) { }; Expression *newExpr1 = nullptr; Expression *newExpr2 = nullptr; - ExpressionUtils::splitFilter(pool, expr, picker, &newExpr1, &newExpr2); + ExpressionUtils::splitFilter(expr, picker, &newExpr1, &newExpr2); ASSERT_EQ(*expr, *newExpr1); ASSERT_EQ(nullptr, newExpr2); } diff --git a/src/validator/FetchEdgesValidator.cpp b/src/validator/FetchEdgesValidator.cpp index 89872177b..4f592f9a0 100644 --- a/src/validator/FetchEdgesValidator.cpp +++ b/src/validator/FetchEdgesValidator.cpp @@ -182,7 +182,7 @@ Status FetchEdgesValidator::preparePropertiesWithYield(const YieldClause *yield) propsName.reserve(newYield_->columns().size()); dedup_ = newYield_->isDistinct(); for (auto col : newYield_->columns()) { - col->setExpr(ExpressionUtils::rewriteLabelAttr2EdgeProp(pool, col->expr())); + col->setExpr(ExpressionUtils::rewriteLabelAttr2EdgeProp(col->expr())); NG_RETURN_IF_ERROR(invalidLabelIdentifiers(col->expr())); const auto *invalidExpr = findInvalidYieldExpression(col->expr()); if (invalidExpr != nullptr) { diff --git a/src/validator/FetchVerticesValidator.cpp b/src/validator/FetchVerticesValidator.cpp index 3cd133077..e75aef67f 100644 --- a/src/validator/FetchVerticesValidator.cpp +++ b/src/validator/FetchVerticesValidator.cpp @@ -153,7 +153,7 @@ Status FetchVerticesValidator::preparePropertiesWithYield(const YieldClause *yie auto* pool = qctx_->objPool(); for (auto col : yield->columns()) { - col->setExpr(ExpressionUtils::rewriteLabelAttr2TagProp(pool, col->expr())); + col->setExpr(ExpressionUtils::rewriteLabelAttr2TagProp(col->expr())); NG_RETURN_IF_ERROR(invalidLabelIdentifiers(col->expr())); col->expr()->accept(&deducePropsVisitor); if (!deducePropsVisitor.ok()) { diff --git a/src/validator/FindPathValidator.cpp b/src/validator/FindPathValidator.cpp index ef97ebf0d..a82b57c42 100644 --- a/src/validator/FindPathValidator.cpp +++ b/src/validator/FindPathValidator.cpp @@ -45,8 +45,7 @@ Status FindPathValidator::validateWhere(WhereClause* where) { return Status::SemanticError("Not support `%s' in where sentence.", expr->toString().c_str()); } - auto* pool = qctx_->objPool(); - where->setFilter(ExpressionUtils::rewriteLabelAttr2EdgeProp(pool, expr)); + where->setFilter(ExpressionUtils::rewriteLabelAttr2EdgeProp(expr)); auto filter = where->filter(); auto typeStatus = deduceExprType(filter); diff --git a/src/validator/GoValidator.cpp b/src/validator/GoValidator.cpp index 63fef2a90..66a23b604 100644 --- a/src/validator/GoValidator.cpp +++ b/src/validator/GoValidator.cpp @@ -56,9 +56,8 @@ Status GoValidator::validateWhere(WhereClause* where) { return Status::SemanticError("`%s', not support aggregate function in where sentence.", expr->toString().c_str()); } - auto pool = qctx()->objPool(); - where->setFilter(ExpressionUtils::rewriteLabelAttr2EdgeProp(pool, expr)); - auto foldRes = ExpressionUtils::foldConstantExpr(pool, where->filter()); + where->setFilter(ExpressionUtils::rewriteLabelAttr2EdgeProp(expr)); + auto foldRes = ExpressionUtils::foldConstantExpr(where->filter()); NG_RETURN_IF_ERROR(foldRes); auto filter = foldRes.value(); @@ -141,7 +140,7 @@ Status GoValidator::validateYield(YieldClause* yield) { } for (auto col : cols) { - col->setExpr(ExpressionUtils::rewriteLabelAttr2EdgeProp(pool, col->expr())); + col->setExpr(ExpressionUtils::rewriteLabelAttr2EdgeProp(col->expr())); NG_RETURN_IF_ERROR(invalidLabelIdentifiers(col->expr())); auto* colExpr = col->expr(); diff --git a/src/validator/GroupByValidator.cpp b/src/validator/GroupByValidator.cpp index ff696f08b..93e2d697f 100644 --- a/src/validator/GroupByValidator.cpp +++ b/src/validator/GroupByValidator.cpp @@ -50,7 +50,7 @@ Status GroupByValidator::validateYield(const YieldClause* yieldClause) { needGenProject_ = true; } if (!aggs.empty()) { - auto* colRewrited = ExpressionUtils::rewriteAgg2VarProp(pool, colExpr); + auto* colRewrited = ExpressionUtils::rewriteAgg2VarProp(colExpr); projCols_->addColumn(new YieldColumn(colRewrited, colOldName)); continue; } diff --git a/src/validator/LookupValidator.cpp b/src/validator/LookupValidator.cpp index 85cfd83f7..a6d628b21 100644 --- a/src/validator/LookupValidator.cpp +++ b/src/validator/LookupValidator.cpp @@ -203,7 +203,7 @@ StatusOr LookupValidator::rewriteRelExpr(RelationalExpression* expr // fold constant expression auto pool = qctx_->objPool(); - auto foldRes = ExpressionUtils::foldConstantExpr(pool, expr); + auto foldRes = ExpressionUtils::foldConstantExpr(expr); NG_RETURN_IF_ERROR(foldRes); expr = static_cast(foldRes.value()); DCHECK_EQ(expr->left()->kind(), Expression::Kind::kLabelAttribute); @@ -216,9 +216,9 @@ StatusOr LookupValidator::rewriteRelExpr(RelationalExpression* expr // rewrite PropertyExpression if (lookupCtx_->isEdge) { - expr->setLeft(ExpressionUtils::rewriteLabelAttr2EdgeProp(pool, la)); + expr->setLeft(ExpressionUtils::rewriteLabelAttr2EdgeProp(la)); } else { - expr->setLeft(ExpressionUtils::rewriteLabelAttr2TagProp(pool, la)); + expr->setLeft(ExpressionUtils::rewriteLabelAttr2TagProp(la)); } return expr; } diff --git a/src/validator/MaintainValidator.cpp b/src/validator/MaintainValidator.cpp index 8b992f265..02d262c34 100644 --- a/src/validator/MaintainValidator.cpp +++ b/src/validator/MaintainValidator.cpp @@ -47,9 +47,8 @@ Status SchemaValidator::validateColumns(const std::vector property->defaultValue()->toString().c_str()); } auto *defaultValueExpr = property->defaultValue(); - auto pool = qctx()->objPool(); // some expression is evaluable but not pure so only fold instead of eval here - auto foldRes = ExpressionUtils::foldConstantExpr(pool, defaultValueExpr); + auto foldRes = ExpressionUtils::foldConstantExpr(defaultValueExpr); NG_RETURN_IF_ERROR(foldRes); column.set_default_value(foldRes.value()->encode()); } else if (property->isComment()) { diff --git a/src/validator/MatchValidator.cpp b/src/validator/MatchValidator.cpp index 401a94dc3..9d88b6a5d 100644 --- a/src/validator/MatchValidator.cpp +++ b/src/validator/MatchValidator.cpp @@ -278,8 +278,7 @@ Status MatchValidator::buildEdgeInfo(const MatchPath *path, Status MatchValidator::validateFilter(const Expression *filter, WhereClauseContext &whereClauseCtx) const { - auto pool = whereClauseCtx.qctx->objPool(); - auto transformRes = ExpressionUtils::filterTransform(filter, pool); + auto transformRes = ExpressionUtils::filterTransform(filter); NG_RETURN_IF_ERROR(transformRes); whereClauseCtx.filter = transformRes.value(); @@ -692,7 +691,7 @@ Status MatchValidator::validateGroup(YieldClauseContext &yieldCtx) const { yieldCtx.aggOutputColumnNames_.emplace_back(agg->toString()); } if (!aggs.empty()) { - auto *rewritedExpr = ExpressionUtils::rewriteAgg2VarProp(pool, colExpr); + auto *rewritedExpr = ExpressionUtils::rewriteAgg2VarProp(colExpr); yieldCtx.projCols_->addColumn(new YieldColumn(rewritedExpr, colOldName)); yieldCtx.projOutputColumnNames_.emplace_back(colOldName); continue; diff --git a/src/validator/YieldValidator.cpp b/src/validator/YieldValidator.cpp index 4d49ece9d..6dc7bf54a 100644 --- a/src/validator/YieldValidator.cpp +++ b/src/validator/YieldValidator.cpp @@ -50,12 +50,6 @@ Status YieldValidator::validateImpl() { return Status::SemanticError("Only one variable allowed to use."); } - if (!groupByValidator_ && exprProps_.inputProps().empty() - && exprProps_.varProps().empty() && inputVarName_.empty()) { - // generate constant expression result into querycontext - genConstantExprValues(); - } - if (!exprProps_.varProps().empty() && !userDefinedVarNameList_.empty()) { // TODO: Support Multiple userDefinedVars if (userDefinedVarNameList_.size() != 1) { @@ -70,7 +64,6 @@ Status YieldValidator::validateImpl() { Status YieldValidator::makeOutputColumn(YieldColumn *column) { columns_->addColumn(column); - auto* pool = qctx()->objPool(); auto colExpr = column->expr(); DCHECK(colExpr != nullptr); @@ -83,7 +76,7 @@ Status YieldValidator::makeOutputColumn(YieldColumn *column) { auto name = column->name(); // Constant expression folding must be after type deduction - auto foldedExpr = ExpressionUtils::foldConstantExpr(pool, expr); + auto foldedExpr = ExpressionUtils::foldConstantExpr(expr); NG_RETURN_IF_ERROR(foldedExpr); auto foldedExprCopy = std::move(foldedExpr).value()->clone(); column->setExpr(foldedExprCopy); @@ -91,20 +84,6 @@ Status YieldValidator::makeOutputColumn(YieldColumn *column) { return Status::OK(); } -void YieldValidator::genConstantExprValues() { - constantExprVar_ = vctx_->anonVarGen()->getVar(); - DataSet ds; - ds.colNames = getOutColNames(); - QueryExpressionContext ctx; - Row row; - for (auto &column : columns_->columns()) { - row.values.emplace_back(Expression::eval(column->expr(), ctx(nullptr))); - } - ds.emplace_back(std::move(row)); - qctx_->ectx()->setResult(constantExprVar_, - ResultBuilder().value(Value(std::move(ds))).finish()); -} - Status YieldValidator::makeImplicitGroupByValidator() { auto* groupSentence = qctx()->objPool()->add( new GroupBySentence( @@ -178,8 +157,7 @@ Status YieldValidator::validateWhere(const WhereClause *clause) { } if (filter != nullptr) { NG_RETURN_IF_ERROR(deduceProps(filter, exprProps_)); - auto pool = qctx_->objPool(); - auto foldRes = ExpressionUtils::foldConstantExpr(pool, filter); + auto foldRes = ExpressionUtils::foldConstantExpr(filter); NG_RETURN_IF_ERROR(foldRes); filterCondition_ = foldRes.value(); } @@ -194,8 +172,10 @@ Status YieldValidator::toPlan() { if (!userDefinedVarName_.empty()) { inputVar = userDefinedVarName_; colNames = qctx_->symTable()->getVar(inputVar)->colNames; - } else if (!constantExprVar_.empty()) { - inputVar = constantExprVar_; + } else if (exprProps_.inputProps().empty() + && exprProps_.varProps().empty() && inputVarName_.empty()) { + // generate constant expression result into querycontext + inputVar = vctx_->anonVarGen()->getVar(); } else { std::transform( inputs_.cbegin(), inputs_.cend(), colNames.begin(), [](auto &col) { return col.name; }); diff --git a/src/visitor/test/FilterTransformTest.cpp b/src/visitor/test/FilterTransformTest.cpp index 16a64be3e..956722ee1 100644 --- a/src/visitor/test/FilterTransformTest.cpp +++ b/src/visitor/test/FilterTransformTest.cpp @@ -20,7 +20,7 @@ TEST_F(FilterTransformTest, TestComplexExprRewrite) { // !!!(v.age - 1 < 40) => (v.age >= 41) auto expr = notExpr(notExpr( notExpr(ltExpr(minusExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(40))))); - auto res = ExpressionUtils::filterTransform(expr, pool); + auto res = ExpressionUtils::filterTransform(expr); auto expected = geExpr(laExpr("v", "age"), constantExpr(41)); ASSERT_EQ(*res.value(), *expected) << res.value()->toString() << " vs. " << expected->toString(); @@ -31,7 +31,7 @@ TEST_F(FilterTransformTest, TestCalculationOverflow) { { auto expr = ltExpr(minusExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(9223372036854775807)); - auto res = ExpressionUtils::filterTransform(expr, pool); + auto res = ExpressionUtils::filterTransform(expr); auto expected = Status::Error("result of (9223372036854775807+1) cannot be represented as an integer"); ASSERT(!res.status().ok()); @@ -42,7 +42,7 @@ TEST_F(FilterTransformTest, TestCalculationOverflow) { { auto expr = ltExpr(addExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(INT64_MIN)); - auto res = ExpressionUtils::filterTransform(expr, pool); + auto res = ExpressionUtils::filterTransform(expr); auto expected = Status::Error("result of (-9223372036854775808-1) cannot be represented as an integer"); ASSERT(!res.status().ok()); @@ -53,7 +53,7 @@ TEST_F(FilterTransformTest, TestCalculationOverflow) { { auto expr = ltExpr(minusExpr(laExpr("v", "age"), constantExpr(1)), addExpr(constantExpr(9223372036854775807), constantExpr(1))); - auto res = ExpressionUtils::filterTransform(expr, pool); + auto res = ExpressionUtils::filterTransform(expr); auto expected = Status::Error("result of (9223372036854775807+1) cannot be represented as an integer"); ASSERT(!res.status().ok()); @@ -64,7 +64,7 @@ TEST_F(FilterTransformTest, TestCalculationOverflow) { { auto expr = ltExpr(addExpr(laExpr("v", "age"), constantExpr(1)), minusExpr(constantExpr(INT64_MIN), constantExpr(1))); - auto res = ExpressionUtils::filterTransform(expr, pool); + auto res = ExpressionUtils::filterTransform(expr); auto expected = Status::Error("result of (-9223372036854775808-1) cannot be represented as an integer"); ASSERT(!res.status().ok()); @@ -75,7 +75,7 @@ TEST_F(FilterTransformTest, TestCalculationOverflow) { { auto expr = notExpr(notExpr(notExpr(ltExpr( minusExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(9223372036854775807))))); - auto res = ExpressionUtils::filterTransform(expr, pool); + auto res = ExpressionUtils::filterTransform(expr); auto expected = Status::Error("result of (9223372036854775807+1) cannot be represented as an integer"); ASSERT(!res.status().ok()); @@ -86,7 +86,7 @@ TEST_F(FilterTransformTest, TestCalculationOverflow) { { auto expr = notExpr(notExpr(notExpr( ltExpr(addExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(INT64_MIN))))); - auto res = ExpressionUtils::filterTransform(expr, pool); + auto res = ExpressionUtils::filterTransform(expr); auto expected = Status::Error("result of (-9223372036854775808-1) cannot be represented as an integer"); ASSERT(!res.status().ok()); diff --git a/src/visitor/test/RewriteRelExprVisitorTest.cpp b/src/visitor/test/RewriteRelExprVisitorTest.cpp index c46b07e66..bfbd59579 100644 --- a/src/visitor/test/RewriteRelExprVisitorTest.cpp +++ b/src/visitor/test/RewriteRelExprVisitorTest.cpp @@ -19,21 +19,21 @@ TEST_F(RewriteRelExprVisitorTest, TestArithmeticalExpr) { // (label + 1 < 40) => (label < 40 - 1) { auto expr = ltExpr(addExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(40)); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = ltExpr(laExpr("v", "age"), minusExpr(constantExpr(40), constantExpr(1))); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } // (1 + label < 40) => (label < 40 - 1) { auto expr = ltExpr(addExpr(constantExpr(1), laExpr("v", "age")), constantExpr(40)); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = ltExpr(laExpr("v", "age"), minusExpr(constantExpr(40), constantExpr(1))); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } // (-1 + label < 40) => (label < 40 - (-1)) { auto expr = ltExpr(addExpr(constantExpr(-1), laExpr("v", "age")), constantExpr(40)); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = ltExpr(laExpr("v", "age"), minusExpr(constantExpr(40), constantExpr(-1))); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } @@ -41,21 +41,21 @@ TEST_F(RewriteRelExprVisitorTest, TestArithmeticalExpr) { // TODO: replace list with set in object pool and avoid copy { auto expr = ltExpr(addExpr(laExpr("v", "age"), laExpr("v2", "age2")), constantExpr(40)); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = expr; ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } // (label * 2 < 40) => (2*label < 40) Unchanged { auto expr = ltExpr(multiplyExpr(laExpr("v", "age"), constantExpr(2)), constantExpr(40)); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = expr; ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } // (label / 3 < 40) => (label / 3 < 40) Unchanged { auto expr = ltExpr(divideExpr(laExpr("v", "age"), constantExpr(3)), constantExpr(40)); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = expr; ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } @@ -66,7 +66,7 @@ TEST_F(RewriteRelExprVisitorTest, TestNestedArithmeticalExpr) { { auto expr = ltExpr(addExpr(addExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(2)), constantExpr(40)); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = ltExpr(laExpr("v", "age"), minusExpr(minusExpr(constantExpr(40), constantExpr(2)), constantExpr(1))); @@ -76,7 +76,7 @@ TEST_F(RewriteRelExprVisitorTest, TestNestedArithmeticalExpr) { { auto expr = ltExpr(minusExpr(addExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(2)), constantExpr(40)); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = ltExpr(laExpr("v", "age"), minusExpr(addExpr(constantExpr(40), constantExpr(2)), constantExpr(1))); @@ -88,7 +88,7 @@ TEST_F(RewriteRelExprVisitorTest, TestNestedArithmeticalExpr) { ltExpr(addExpr(minusExpr(addExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(2)), constantExpr(3)), constantExpr(40)); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = ltExpr(laExpr("v", "age"), minusExpr(addExpr(minusExpr(constantExpr(40), constantExpr(3)), constantExpr(2)), @@ -100,7 +100,7 @@ TEST_F(RewriteRelExprVisitorTest, TestNestedArithmeticalExpr) { auto expr = ltExpr(addExpr(multiplyExpr(constantExpr(2), laExpr("v", "age")), constantExpr(1)), constantExpr(40)); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = ltExpr(multiplyExpr(constantExpr(2), laExpr("v", "age")), minusExpr(constantExpr(40), constantExpr(1))); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); @@ -110,7 +110,7 @@ TEST_F(RewriteRelExprVisitorTest, TestNestedArithmeticalExpr) { auto expr = ltExpr(minusExpr(divideExpr(laExpr("v", "age"), constantExpr(3)), constantExpr(1)), constantExpr(40)); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = ltExpr(divideExpr(laExpr("v", "age"), constantExpr(3)), addExpr(constantExpr(40), constantExpr(1))); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); @@ -121,14 +121,14 @@ TEST_F(RewriteRelExprVisitorTest, TestReduceBoolNullExpr) { // (v.age > 40 == true) => (v.age > 40) { auto expr = eqExpr(gtExpr(laExpr("v", "age"), constantExpr(40)), constantExpr(true)); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = gtExpr(laExpr("v", "age"), constantExpr(40)); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } // (v.age > 40 == false) => !(v.age > 40) { auto expr = eqExpr(gtExpr(laExpr("v", "age"), constantExpr(40)), constantExpr(false)); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = notExpr(gtExpr(laExpr("v", "age"), constantExpr(40))); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } @@ -136,14 +136,14 @@ TEST_F(RewriteRelExprVisitorTest, TestReduceBoolNullExpr) { { auto expr = eqExpr(gtExpr(laExpr("v", "age"), constantExpr(40)), constantExpr(Value(NullType::__NULL__))); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = constantExpr(Value(NullType::__NULL__)); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } // (v.age <= null) => (null) { auto expr = leExpr(laExpr("v", "age"), constantExpr(Value(NullType::__NULL__))); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = constantExpr(Value(NullType::__NULL__)); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } @@ -151,7 +151,7 @@ TEST_F(RewriteRelExprVisitorTest, TestReduceBoolNullExpr) { { auto expr = gtExpr(addExpr(laExpr("v", "age"), constantExpr(10)), constantExpr(Value(NullType::__NULL__))); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = constantExpr(Value(NullType::__NULL__)); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } @@ -167,7 +167,7 @@ TEST_F(RewriteRelExprVisitorTest, TestLogicalExpr) { ltExpr(addExpr(minusExpr(addExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(2)), constantExpr(3)), constantExpr(40))); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = andExpr( ltExpr(laExpr("v", "age"), minusExpr(minusExpr(constantExpr(40), constantExpr(2)), constantExpr(1))), @@ -189,7 +189,7 @@ TEST_F(RewriteRelExprVisitorTest, TestContainer) { constantExpr(40)), ltExpr(minusExpr(addExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(2)), constantExpr(40))}); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = listExpr( {ltExpr(laExpr("v", "age"), minusExpr(minusExpr(constantExpr(40), constantExpr(2)), constantExpr(1))), @@ -207,7 +207,7 @@ TEST_F(RewriteRelExprVisitorTest, TestContainer) { constantExpr(40)), ltExpr(minusExpr(addExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(2)), constantExpr(40))}); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = setExpr( {ltExpr(laExpr("v", "age"), minusExpr(minusExpr(constantExpr(40), constantExpr(2)), constantExpr(1))), @@ -227,7 +227,7 @@ TEST_F(RewriteRelExprVisitorTest, TestContainer) { {"k2", ltExpr(minusExpr(addExpr(laExpr("v", "age"), constantExpr(1)), constantExpr(2)), constantExpr(40))}}); - auto res = ExpressionUtils::rewriteRelExpr(expr, pool); + auto res = ExpressionUtils::rewriteRelExpr(expr); auto expected = mapExpr( {{"k1", ltExpr(laExpr("v", "age"), diff --git a/src/visitor/test/RewriteUnaryNotExprVisitorTest.cpp b/src/visitor/test/RewriteUnaryNotExprVisitorTest.cpp index 5a7706ade..bf4546bb7 100644 --- a/src/visitor/test/RewriteUnaryNotExprVisitorTest.cpp +++ b/src/visitor/test/RewriteUnaryNotExprVisitorTest.cpp @@ -20,21 +20,21 @@ TEST_F(RewriteUnaryNotExprVisitorTest, TestNestedMultipleUnaryNotExpr) { // !!(5 == 10) => (5 == 10) { auto expr = notExpr(notExpr(eqExpr(constantExpr(5), constantExpr(10)))); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = eqExpr(constantExpr(5), constantExpr(10)); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } // !!!!(5 == 10) => (5 == 10) { auto expr = notExpr(notExpr(notExpr(notExpr(eqExpr(constantExpr(5), constantExpr(10)))))); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = eqExpr(constantExpr(5), constantExpr(10)); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } // !!!(5 == 10) => (5 != 10) { auto expr = notExpr(notExpr(notExpr(eqExpr(constantExpr(5), constantExpr(10))))); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = neExpr(constantExpr(5), constantExpr(10)); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } @@ -42,7 +42,7 @@ TEST_F(RewriteUnaryNotExprVisitorTest, TestNestedMultipleUnaryNotExpr) { { auto expr = notExpr(notExpr(notExpr(notExpr(notExpr(eqExpr(constantExpr(5), constantExpr(10))))))); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = neExpr(constantExpr(5), constantExpr(10)); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } @@ -53,7 +53,7 @@ TEST_F(RewriteUnaryNotExprVisitorTest, TestMultipleUnaryNotExprLogicalRelExpr) { { auto expr = andExpr(notExpr(notExpr(eqExpr(constantExpr(5), constantExpr(10)))), notExpr(notExpr(gtExpr(constantExpr(30), constantExpr(20))))); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = andExpr(eqExpr(constantExpr(5), constantExpr(10)), gtExpr(constantExpr(30), constantExpr(20))); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); @@ -62,7 +62,7 @@ TEST_F(RewriteUnaryNotExprVisitorTest, TestMultipleUnaryNotExprLogicalRelExpr) { { auto expr = andExpr(notExpr(notExpr(notExpr(leExpr(constantExpr(5), constantExpr(10))))), notExpr(gtExpr(constantExpr(30), constantExpr(20)))); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = andExpr(gtExpr(constantExpr(5), constantExpr(10)), leExpr(constantExpr(30), constantExpr(20))); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); @@ -76,7 +76,7 @@ TEST_F(RewriteUnaryNotExprVisitorTest, TestMultipleUnaryNotContainerExpr) { auto expr = listExpr({notExpr(notExpr(eqExpr(constantExpr(5), constantExpr(10)))), notExpr(notExpr(notExpr(gtExpr(constantExpr(30), constantExpr(20)))))}); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = listExpr({eqExpr(constantExpr(5), constantExpr(10)), leExpr(constantExpr(30), constantExpr(20))}); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); @@ -87,7 +87,7 @@ TEST_F(RewriteUnaryNotExprVisitorTest, TestMultipleUnaryNotContainerExpr) { auto expr = setExpr({notExpr(notExpr(eqExpr(constantExpr(5), constantExpr(10)))), notExpr(notExpr(notExpr(gtExpr(constantExpr(30), constantExpr(20)))))}); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = setExpr({eqExpr(constantExpr(5), constantExpr(10)), leExpr(constantExpr(30), constantExpr(20))}); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); @@ -98,7 +98,7 @@ TEST_F(RewriteUnaryNotExprVisitorTest, TestMultipleUnaryNotContainerExpr) { // {"k1":!!(5 == 10), "k2":!!!(30 > 20)}} => {"k1":(5 == 10), "k2":(30 <= 20)} auto expr = mapExpr({{"k1", notExpr(notExpr(eqExpr(constantExpr(5), constantExpr(10))))}, { "k2", notExpr(notExpr(notExpr(gtExpr(constantExpr(30), constantExpr(20)))))}}); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = mapExpr({{"k1", eqExpr(constantExpr(5), constantExpr(10))}, {"k2", leExpr(constantExpr(30), constantExpr(20))}}); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); @@ -110,34 +110,34 @@ TEST_F(RewriteUnaryNotExprVisitorTest, TestRelExpr) { // no change should be made to the orginal expression { auto original = eqExpr(constantExpr(5), constantExpr(10)); - auto res = ExpressionUtils::reduceUnaryNotExpr(original, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(original); ASSERT_EQ(*original, *res) << original->toString() << " vs. " << res->toString(); } // !(5 == 10) => (5 != 10) { auto expr = notExpr(eqExpr(constantExpr(5), constantExpr(10))); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = neExpr(constantExpr(5), constantExpr(10)); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } // !(5 > 10) => (5 <= 10) { auto expr = notExpr(gtExpr(constantExpr(5), constantExpr(10))); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = leExpr(constantExpr(5), constantExpr(10)); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } // !(5 >= 10) => (5 < 10) { auto expr = notExpr(geExpr(constantExpr(5), constantExpr(10))); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = ltExpr(constantExpr(5), constantExpr(10)); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } // !("bcd" IN "abcde") => ("bcd" NOT IN "abcde") { auto expr = notExpr(inExpr(constantExpr("bcd"), constantExpr("abcde"))); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = notInExpr(constantExpr("bcd"), constantExpr("abcde")); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } @@ -145,7 +145,7 @@ TEST_F(RewriteUnaryNotExprVisitorTest, TestRelExpr) { // !("bcd" NOT IN "abcde") => ("bcd" IN "abcde") { auto expr = notExpr(notInExpr(constantExpr("bcd"), constantExpr("abcde"))); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = inExpr(constantExpr("bcd"), constantExpr("abcde")); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } @@ -153,7 +153,7 @@ TEST_F(RewriteUnaryNotExprVisitorTest, TestRelExpr) { // !("bcd" STARTS WITH "abc") => ("bcd" NOT STARTS WITH "abc") { auto expr = notExpr(startsWithExpr(constantExpr("bcd"), constantExpr("abcde"))); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = notStartsWithExpr(constantExpr("bcd"), constantExpr("abcde")); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } @@ -161,7 +161,7 @@ TEST_F(RewriteUnaryNotExprVisitorTest, TestRelExpr) { // !("bcd" ENDS WITH "abc") => ("bcd" NOT ENDS WITH "abc") { auto expr = notExpr(endsWithExpr(constantExpr("bcd"), constantExpr("abcde"))); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = notEndsWithExpr(constantExpr("bcd"), constantExpr("abcde")); ASSERT_EQ(*res, *expected) << res->toString() << " vs. " << expected->toString(); } @@ -173,7 +173,7 @@ TEST_F(RewriteUnaryNotExprVisitorTest, TestLogicalExpr) { auto expr = notExpr(andExpr(andExpr(neExpr(constantExpr(1), constantExpr(1)), geExpr(constantExpr(2), constantExpr(3))), leExpr(constantExpr(30), constantExpr(20)))); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = LogicalExpression::makeOr(pool); expected->addOperand(eqExpr(constantExpr(1), constantExpr(1))); @@ -187,7 +187,7 @@ TEST_F(RewriteUnaryNotExprVisitorTest, TestLogicalExpr) { auto expr = notExpr(orExpr(orExpr(neExpr(constantExpr(1), constantExpr(1)), geExpr(constantExpr(2), constantExpr(3))), leExpr(constantExpr(30), constantExpr(20)))); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = LogicalExpression::makeAnd(pool); expected->addOperand(eqExpr(constantExpr(1), constantExpr(1))); @@ -201,7 +201,7 @@ TEST_F(RewriteUnaryNotExprVisitorTest, TestLogicalExpr) { auto expr = notExpr(orExpr(andExpr(neExpr(constantExpr(1), constantExpr(1)), geExpr(constantExpr(2), constantExpr(3))), leExpr(constantExpr(30), constantExpr(20)))); - auto res = ExpressionUtils::reduceUnaryNotExpr(expr, pool); + auto res = ExpressionUtils::reduceUnaryNotExpr(expr); auto expected = andExpr(orExpr(eqExpr(constantExpr(1), constantExpr(1)), ltExpr(constantExpr(2), constantExpr(3))), diff --git a/tests/tck/features/go/GO.feature b/tests/tck/features/go/GO.feature index c930d225b..58d39f289 100644 --- a/tests/tck/features/go/GO.feature +++ b/tests/tck/features/go/GO.feature @@ -1821,3 +1821,14 @@ Feature: Go Sentence """ Then the result should be, in any order, with relax comparison: | like._dst | + + Scenario: with no existed tag + When executing query: + """ + GO FROM 'Tony Parker' OVER like YIELD $$.player.name, $^.team.name + """ + Then the result should be, in any order, with relax comparison: + | $$.player.name | $^.team.name | + | "LaMarcus Aldridge" | EMPTY | + | "Manu Ginobili" | EMPTY | + | "Tim Duncan" | EMPTY | diff --git a/tests/tck/features/lookup/LookUp.IntVid.feature b/tests/tck/features/lookup/LookUp.IntVid.feature index 1ec7040c9..3628a5cd4 100644 --- a/tests/tck/features/lookup/LookUp.IntVid.feature +++ b/tests/tck/features/lookup/LookUp.IntVid.feature @@ -1,6 +1,6 @@ Feature: LookUpTest_Vid_Int - Scenario: LookupTest IntVid VertexIndexHint + Background: Given an empty graph And create a space with following options: | partition_num | 9 | @@ -8,7 +8,9 @@ Feature: LookUpTest_Vid_Int | vid_type | int64 | | charset | utf8 | | collate | utf8_bin | - And having executed: + + Scenario: LookupTest IntVid VertexIndexHint + Given having executed: """ CREATE TAG lookup_tag_1(col1 int, col2 int, col3 int); CREATE TAG lookup_tag_2(col1 bool, col2 int, col3 double, col4 bool); @@ -45,14 +47,7 @@ Feature: LookUpTest_Vid_Int Then drop the used space Scenario: LookupTest IntVid EdgeIndexHint - Given an empty graph - And create a space with following options: - | partition_num | 9 | - | replica_factor | 1 | - | vid_type | int64 | - | charset | utf8 | - | collate | utf8_bin | - And having executed: + Given having executed: """ CREATE EDGE lookup_edge_1(col1 int, col2 int, col3 int); CREATE EDGE lookup_edge_2(col1 bool,col2 int, col3 double, col4 bool); @@ -86,14 +81,7 @@ Feature: LookUpTest_Vid_Int Then drop the used space Scenario: LookupTest IntVid VertexConditionScan - Given an empty graph - And create a space with following options: - | partition_num | 9 | - | replica_factor | 1 | - | vid_type | int64 | - | charset | utf8 | - | collate | utf8_bin | - And having executed: + Given having executed: """ CREATE TAG lookup_tag_2(col1 bool, col2 int, col3 double, col4 bool); CREATE TAG INDEX t_index_2 ON lookup_tag_2(col2, col3, col4); @@ -244,14 +232,7 @@ Feature: LookUpTest_Vid_Int Then drop the used space Scenario: LookupTest IntVid EdgeConditionScan - Given an empty graph - And create a space with following options: - | partition_num | 9 | - | replica_factor | 1 | - | vid_type | int64 | - | charset | utf8 | - | collate | utf8_bin | - And having executed: + Given having executed: """ CREATE EDGE lookup_edge_2(col1 bool,col2 int, col3 double, col4 bool); CREATE EDGE INDEX e_index_2 ON lookup_edge_2(col2, col3, col4); @@ -395,14 +376,7 @@ Feature: LookUpTest_Vid_Int Then drop the used space Scenario: LookupTest IntVid FunctionExprTest - Given an empty graph - And create a space with following options: - | partition_num | 9 | - | replica_factor | 1 | - | vid_type | int64 | - | charset | utf8 | - | collate | utf8_bin | - And having executed: + Given having executed: """ CREATE TAG lookup_tag_2(col1 bool, col2 int, col3 double, col4 bool); CREATE TAG INDEX t_index_2 ON lookup_tag_2(col2, col3, col4); @@ -532,13 +506,6 @@ Feature: LookUpTest_Vid_Int Then drop the used space Scenario: LookupTest IntVid YieldClauseTest - Given an empty graph - And create a space with following options: - | partition_num | 9 | - | replica_factor | 1 | - | vid_type | int64 | - | charset | utf8 | - | collate | utf8_bin | When executing query: """ CREATE TAG student(number int, age int) @@ -590,13 +557,6 @@ Feature: LookUpTest_Vid_Int Then drop the used space Scenario: LookupTest IntVid OptimizerTest - Given an empty graph - And create a space with following options: - | partition_num | 9 | - | replica_factor | 1 | - | vid_type | int64 | - | charset | utf8 | - | collate | utf8_bin | When executing query: """ CREATE TAG t1(c1 int, c2 int, c3 int, c4 int, c5 int) @@ -681,13 +641,6 @@ Feature: LookUpTest_Vid_Int Then drop the used space Scenario: LookupTest IntVid OptimizerWithStringFieldTest - Given an empty graph - And create a space with following options: - | partition_num | 9 | - | replica_factor | 1 | - | vid_type | int64 | - | charset | utf8 | - | collate | utf8_bin | When executing query: """ CREATE TAG t1_str(c1 int, c2 int, c3 string, c4 string) @@ -762,13 +715,6 @@ Feature: LookUpTest_Vid_Int Then drop the used space Scenario: LookupTest IntVid StringFieldTest - Given an empty graph - And create a space with following options: - | partition_num | 9 | - | replica_factor | 1 | - | vid_type | int64 | - | charset | utf8 | - | collate | utf8_bin | When executing query: """ CREATE TAG tag_with_str(c1 int, c2 string, c3 string) @@ -848,13 +794,6 @@ Feature: LookUpTest_Vid_Int Then drop the used space Scenario: LookupTest IntVid ConditionTest - Given an empty graph - And create a space with following options: - | partition_num | 9 | - | replica_factor | 1 | - | vid_type | int64 | - | charset | utf8 | - | collate | utf8_bin | When executing query: """ create tag identity (BIRTHDAY int, NATION string, BIRTHPLACE_CITY string) @@ -887,3 +826,21 @@ Feature: LookUpTest_Vid_Int Then the result should be, in any order: | VertexID | Then drop the used space + + Scenario: LookupTest no index to use at runtime + Given having executed: + """ + CREATE TAG player(name string, age int); + """ + And wait 6 seconds + When executing query: + """ + INSERT VERTEX player(name, age) VALUES hash('Tim'):('Tim', 20); + """ + Then the execution should be successful + When executing query: + """ + LOOKUP ON player WHERE player.name == 'Tim' + """ + Then an ExecutionError should be raised at runtime: There is no index to use at runtime + Then drop the used space diff --git a/tests/tck/features/lookup/LookUp.feature b/tests/tck/features/lookup/LookUp.feature index 5ab0caef6..6afca5f52 100644 --- a/tests/tck/features/lookup/LookUp.feature +++ b/tests/tck/features/lookup/LookUp.feature @@ -957,3 +957,21 @@ Feature: LookUpTest_Vid_String | VertexID | | '202' | Then drop the used space + + Scenario: LookupTest no index to use at runtime + Given having executed: + """ + CREATE TAG player(name string, age int); + """ + And wait 6 seconds + When executing query: + """ + INSERT VERTEX player(name, age) VALUES 'Tim':('Tim', 20); + """ + Then the execution should be successful + When executing query: + """ + LOOKUP ON player WHERE player.name == 'Tim' + """ + Then an ExecutionError should be raised at runtime: There is no index to use at runtime + Then drop the used space