diff --git a/arangod/Aql/AstNode.cpp b/arangod/Aql/AstNode.cpp index 90483e2e2c..fad5f89200 100644 --- a/arangod/Aql/AstNode.cpp +++ b/arangod/Aql/AstNode.cpp @@ -2582,6 +2582,28 @@ void AstNode::stealComputedValue() { } } +/// @brief Removes all members from the current node that are also +/// members of the other node (ignoring ording) +/// Can only be applied if this and other are of type +/// n-ary-and +void AstNode::removeMembersInOtherAndNode(AstNode const* other) { + TRI_ASSERT(type == NODE_TYPE_OPERATOR_NARY_AND); + TRI_ASSERT(other->type == NODE_TYPE_OPERATOR_NARY_AND); + for (size_t i = 0; i < other->numMembers(); ++i) { + auto theirs = other->getMemberUnchecked(i); + for (size_t j = 0; j < numMembers(); ++j) { + auto ours = getMemberUnchecked(j); + // NOTE: Pointer comparison on purpose. + // We do not want to reduce equivalent but identical nodes + if (ours == theirs) { + removeMemberUnchecked(j); + break; + } + } + } + +} + /// @brief append the AstNode to an output stream std::ostream& operator<<(std::ostream& stream, arangodb::aql::AstNode const* node) { diff --git a/arangod/Aql/AstNode.h b/arangod/Aql/AstNode.h index e86041df33..705d446bf9 100644 --- a/arangod/Aql/AstNode.h +++ b/arangod/Aql/AstNode.h @@ -659,6 +659,13 @@ struct AstNode { /// @brief Steals the computed value and frees it. void stealComputedValue(); + + /// @brief Removes all members from the current node that are also + /// members of the other node (ignoring ording) + /// Can only be applied if this and other are of type + /// n-ary-and + void removeMembersInOtherAndNode(AstNode const* other); + public: /// @brief the node type AstNodeType const type; @@ -677,7 +684,7 @@ struct AstNode { std::vector members; }; -int CompareAstNodes(AstNode const*, AstNode const*, bool); +int CompareAstNodes(AstNode const* lhs, AstNode const* rhs, bool compareUtf8); /// @brief less comparator for Ast value nodes template diff --git a/arangod/Aql/Condition.cpp b/arangod/Aql/Condition.cpp index 7b418c3166..aa670e05d6 100644 --- a/arangod/Aql/Condition.cpp +++ b/arangod/Aql/Condition.cpp @@ -182,11 +182,12 @@ ConditionPart::ConditionPart( ConditionPart::~ConditionPart() {} /// @brief true if the condition is completely covered by the other condition -bool ConditionPart::isCoveredBy(ConditionPart const& other, bool isReversed) const { +bool ConditionPart::isCoveredBy(ConditionPart const& other, + bool isReversed) const { if (variable != other.variable || attributeName != other.attributeName) { return false; } - + if (!isExpanded && !other.isExpanded && other.operatorType == NODE_TYPE_OPERATOR_BINARY_IN && other.valueNode->isConstant() && isReversed) { @@ -197,12 +198,11 @@ bool ConditionPart::isCoveredBy(ConditionPart const& other, bool isReversed) con TRI_ASSERT(valueNode != nullptr); TRI_ASSERT(other.valueNode != nullptr); - + if (!valueNode->isConstant() || !other.valueNode->isConstant()) { return false; } - // special cases for IN... if (!isExpanded && !other.isExpanded && other.operatorType == NODE_TYPE_OPERATOR_BINARY_IN && @@ -225,8 +225,8 @@ bool ConditionPart::isCoveredBy(ConditionPart const& other, bool isReversed) con auto w = other.valueNode->getMemberUnchecked(j); ConditionPartCompareResult res = - ConditionPart::ResultsTable[CompareAstNodes(v, w, true) + - 1][0][0]; + ConditionPart::ResultsTable[CompareAstNodes(v, w, true) + 1][0] + [0]; if (res != CompareResult::OTHER_CONTAINED_IN_SELF && res != CompareResult::CONVERT_EQUAL && @@ -270,8 +270,8 @@ bool ConditionPart::isCoveredBy(ConditionPart const& other, bool isReversed) con // Results are -1, 0, 1, move to 0, 1, 2 for the lookup: ConditionPartCompareResult res = ConditionPart::ResultsTable - [CompareAstNodes(other.valueNode, valueNode, true) + - 1][other.whichCompareOperation()][whichCompareOperation()]; + [CompareAstNodes(other.valueNode, valueNode, true) + 1] + [other.whichCompareOperation()][whichCompareOperation()]; if (res == CompareResult::OTHER_CONTAINED_IN_SELF || res == CompareResult::CONVERT_EQUAL || res == CompareResult::IMPOSSIBLE) { @@ -283,7 +283,8 @@ bool ConditionPart::isCoveredBy(ConditionPart const& other, bool isReversed) con /// @brief clears the attribute access data static inline void clearAttributeAccess( - std::pair>& parts) { + std::pair>& + parts) { parts.first = nullptr; parts.second.clear(); } @@ -369,7 +370,7 @@ std::pair Condition::findIndexes( TRI_ASSERT(usedIndexes.empty()); Variable const* reference = node->outVariable(); std::string collectionName = node->collection()->getName(); - + transaction::Methods* trx = _ast->query()->trx(); size_t const itemsInIndex = node->collection()->count(trx); @@ -387,8 +388,8 @@ std::pair Condition::findIndexes( /// @brief get the attributes for a sub-condition that are const /// (i.e. compared with equality) -std::vector> Condition::getConstAttributes(Variable const* reference, - bool includeNull) { +std::vector> +Condition::getConstAttributes(Variable const* reference, bool includeNull) { std::vector> result; if (_root == nullptr) { @@ -401,7 +402,8 @@ std::vector> Condition::getConstAtt return result; } - std::pair> parts; + std::pair> + parts; AstNode const* node = _root->getMember(0); n = node->numMembers(); @@ -416,13 +418,16 @@ std::vector> Condition::getConstAtt if (lhs->isAttributeAccessForVariable(parts) && parts.first == reference) { - if (includeNull || ((rhs->isConstant() || rhs->type == NODE_TYPE_REFERENCE) && !rhs->isNullValue())) { + if (includeNull || + ((rhs->isConstant() || rhs->type == NODE_TYPE_REFERENCE) && + !rhs->isNullValue())) { result.emplace_back(std::move(parts.second)); } - } - else if (rhs->isAttributeAccessForVariable(parts) && - parts.first == reference) { - if (includeNull || ((lhs->isConstant() || lhs->type == NODE_TYPE_REFERENCE) && !lhs->isNullValue())) { + } else if (rhs->isAttributeAccessForVariable(parts) && + parts.first == reference) { + if (includeNull || + ((lhs->isConstant() || lhs->type == NODE_TYPE_REFERENCE) && + !lhs->isNullValue())) { result.emplace_back(std::move(parts.second)); } } @@ -474,6 +479,55 @@ void Condition::normalize() { #endif } +void Condition::CollectOverlappingMembers( + ExecutionPlan const* plan, Variable const* variable, AstNode* andNode, + AstNode* otherAndNode, std::unordered_set& toRemove) { + std::pair> + result; + + size_t const n = andNode->numMembers(); + + for (size_t i = 0; i < n; ++i) { + auto operand = andNode->getMemberUnchecked(i); + + if (operand->isComparisonOperator() && + operand->type != NODE_TYPE_OPERATOR_BINARY_NE && + operand->type != NODE_TYPE_OPERATOR_BINARY_NIN) { + auto lhs = operand->getMember(0); + auto rhs = operand->getMember(1); + + if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + clearAttributeAccess(result); + + if (lhs->isAttributeAccessForVariable(result) && + result.first == variable) { + ConditionPart current(variable, result.second, operand, + ATTRIBUTE_LEFT, nullptr); + + if (CanRemove(plan, current, otherAndNode)) { + toRemove.emplace(i); + } + } + } + + if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS || + rhs->type == NODE_TYPE_EXPANSION) { + clearAttributeAccess(result); + + if (rhs->isAttributeAccessForVariable(result) && + result.first == variable) { + ConditionPart current(variable, result.second, operand, + ATTRIBUTE_RIGHT, nullptr); + + if (CanRemove(plan, current, otherAndNode)) { + toRemove.emplace(i); + } + } + } + } + } +} + /// @brief removes condition parts from another AstNode* Condition::removeIndexCondition(ExecutionPlan const* plan, Variable const* variable, @@ -494,51 +548,13 @@ AstNode* Condition::removeIndexCondition(ExecutionPlan const* plan, auto andNode = _root->getMemberUnchecked(0); TRI_ASSERT(andNode->type == NODE_TYPE_OPERATOR_NARY_AND); + auto otherAndNode = other->getMemberUnchecked(0); + TRI_ASSERT(otherAndNode->type == NODE_TYPE_OPERATOR_NARY_AND); size_t const n = andNode->numMembers(); - std::pair> result; std::unordered_set toRemove; - for (size_t i = 0; i < n; ++i) { - auto operand = andNode->getMemberUnchecked(i); - - if (operand->isComparisonOperator() && - operand->type != NODE_TYPE_OPERATOR_BINARY_NE && - operand->type != NODE_TYPE_OPERATOR_BINARY_NIN) { - - auto lhs = operand->getMember(0); - auto rhs = operand->getMember(1); - - if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - clearAttributeAccess(result); - - if (lhs->isAttributeAccessForVariable(result) && - result.first == variable) { - ConditionPart current(variable, result.second, operand, - ATTRIBUTE_LEFT, nullptr); - - if (canRemove(plan, current, other)) { - toRemove.emplace(i); - } - } - } - - if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS || - rhs->type == NODE_TYPE_EXPANSION) { - clearAttributeAccess(result); - - if (rhs->isAttributeAccessForVariable(result) && - result.first == variable) { - ConditionPart current(variable, result.second, operand, - ATTRIBUTE_RIGHT, nullptr); - - if (canRemove(plan, current, other)) { - toRemove.emplace(i); - } - } - } - } - } + CollectOverlappingMembers(plan, variable, andNode, otherAndNode, toRemove); if (toRemove.empty()) { return _root; @@ -623,12 +639,13 @@ void Condition::optimize(ExecutionPlan* plan) { return; } - transaction::Methods* trx = plan->getAst()->query()->trx(); + transaction::Methods* trx = plan->getAst()->query()->trx(); TRI_ASSERT(_root != nullptr); TRI_ASSERT(_root->type == NODE_TYPE_OPERATOR_NARY_OR); - - std::pair> varAccess; + + std::pair> + varAccess; // handle sub nodes of top-level OR node size_t n = _root->numMembers(); @@ -662,23 +679,32 @@ void Condition::optimize(ExecutionPlan* plan) { } TRI_ASSERT(andNumMembers > 1); - + // sort AND parts of each sub-condition so > and >= come before < and <= - // we use this to some advantage when we check the conditions for a sparse index + // we use this to some advantage when we check the conditions for a sparse + // index // later. - // if a sparse index is asked whether it can supported a condition such as `attr < value1`, - // this range would include `null`, which the sparse index cannot provide. however, if we - // first check other conditions we may find a condition on the same attribute, e.g. `attr > value2`. - // this other condition may exclude `null` so we then use the full range `value2 < attr < value1` + // if a sparse index is asked whether it can supported a condition such as + // `attr < value1`, + // this range would include `null`, which the sparse index cannot provide. + // however, if we + // first check other conditions we may find a condition on the same + // attribute, e.g. `attr > value2`. + // this other condition may exclude `null` so we then use the full range + // `value2 < attr < value1` // and do not have to discard sub-conditions anymore andNode->sortMembers([](AstNode const* lhs, AstNode const* rhs) { - if ((lhs->type != NODE_TYPE_OPERATOR_BINARY_LT && lhs->type != NODE_TYPE_OPERATOR_BINARY_LE) && - (rhs->type == NODE_TYPE_OPERATOR_BINARY_LT || rhs->type == NODE_TYPE_OPERATOR_BINARY_LE)) { + if ((lhs->type != NODE_TYPE_OPERATOR_BINARY_LT && + lhs->type != NODE_TYPE_OPERATOR_BINARY_LE) && + (rhs->type == NODE_TYPE_OPERATOR_BINARY_LT || + rhs->type == NODE_TYPE_OPERATOR_BINARY_LE)) { // sort < and <= after other comparison operators return true; } - if ((lhs->type == NODE_TYPE_OPERATOR_BINARY_LT || lhs->type == NODE_TYPE_OPERATOR_BINARY_LE) && - (rhs->type != NODE_TYPE_OPERATOR_BINARY_LT && rhs->type != NODE_TYPE_OPERATOR_BINARY_LE)) { + if ((lhs->type == NODE_TYPE_OPERATOR_BINARY_LT || + lhs->type == NODE_TYPE_OPERATOR_BINARY_LE) && + (rhs->type != NODE_TYPE_OPERATOR_BINARY_LT && + rhs->type != NODE_TYPE_OPERATOR_BINARY_LE)) { // sort < and <= after other comparison operators return false; } @@ -686,7 +712,6 @@ void Condition::optimize(ExecutionPlan* plan) { return (lhs->type < rhs->type); }); - if (inComparisons > 0) { // move IN operations to the front to make comparison code below simpler std::vector stack; @@ -715,7 +740,7 @@ void Condition::optimize(ExecutionPlan* plan) { stack.pop_back(); } } - + // optimization is only necessary if an AND node has multiple members VariableUsageType variableUsage; @@ -730,14 +755,16 @@ void Condition::optimize(ExecutionPlan* plan) { if (lhs->isConstant()) { lhs = Ast::resolveConstAttributeAccess(lhs); } - storeAttributeAccess(varAccess, variableUsage, lhs, j, ATTRIBUTE_LEFT); + storeAttributeAccess(varAccess, variableUsage, lhs, j, + ATTRIBUTE_LEFT); } if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS || rhs->type == NODE_TYPE_EXPANSION) { if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS && rhs->isConstant()) { rhs = Ast::resolveConstAttributeAccess(rhs); } - storeAttributeAccess(varAccess, variableUsage, rhs, j, ATTRIBUTE_RIGHT); + storeAttributeAccess(varAccess, variableUsage, rhs, j, + ATTRIBUTE_RIGHT); } } } @@ -807,8 +834,8 @@ void Condition::optimize(ExecutionPlan* plan) { for (size_t k = 0; k < values->numMembers(); ++k) { auto value = values->getMemberUnchecked(k); ConditionPartCompareResult res = ConditionPart::ResultsTable - [CompareAstNodes(value, other.valueNode, true) + - 1][0 /*NODE_TYPE_OPERATOR_BINARY_EQ*/] + [CompareAstNodes(value, other.valueNode, true) + 1] + [0 /*NODE_TYPE_OPERATOR_BINARY_EQ*/] [other.whichCompareOperation()]; bool const keep = @@ -902,11 +929,10 @@ void Condition::optimize(ExecutionPlan* plan) { /// @brief registers an attribute access for a particular (collection) variable void Condition::storeAttributeAccess( - std::pair>& varAccess, - VariableUsageType& variableUsage, - AstNode const* node, size_t position, + std::pair>& + varAccess, + VariableUsageType& variableUsage, AstNode const* node, size_t position, AttributeSideType side) { - if (!node->isAttributeAccessForVariable(varAccess)) { return; } @@ -969,20 +995,22 @@ void Condition::validateAst(AstNode const* node, int level) { #endif /// @brief checks if the current condition is covered by the other -bool Condition::canRemove(ExecutionPlan const* plan, ConditionPart const& me, - arangodb::aql::AstNode const* otherCondition) const { - TRI_ASSERT(otherCondition != nullptr); - TRI_ASSERT(otherCondition->type == NODE_TYPE_OPERATOR_NARY_OR); - - std::pair> result; - auto andNode = otherCondition->getMemberUnchecked(0); +bool Condition::CanRemove(ExecutionPlan const* plan, ConditionPart const& me, + arangodb::aql::AstNode const* andNode) { + TRI_ASSERT(andNode != nullptr); TRI_ASSERT(andNode->type == NODE_TYPE_OPERATOR_NARY_AND); + + std::pair> + result; + size_t const n = andNode->numMembers(); auto normalize = [&plan](AstNode const* node) -> std::string { if (node->type == NODE_TYPE_REFERENCE) { - auto setter = plan->getVarSetBy(static_cast(node->getData())->id); - if (setter != nullptr && setter->getType() == ExecutionNode::CALCULATION) { + auto setter = + plan->getVarSetBy(static_cast(node->getData())->id); + if (setter != nullptr && + setter->getType() == ExecutionNode::CALCULATION) { auto cn = static_cast(setter); // use expression node instead node = cn->expression()->node(); @@ -1006,7 +1034,7 @@ bool Condition::canRemove(ExecutionPlan const* plan, ConditionPart const& me, if (lhs->isAttributeAccessForVariable(result)) { if (rhs->isConstant()) { ConditionPart indexCondition(result.first, result.second, operand, - ATTRIBUTE_LEFT, nullptr); + ATTRIBUTE_LEFT, nullptr); if (me.isCoveredBy(indexCondition, false)) { return true; @@ -1014,7 +1042,7 @@ bool Condition::canRemove(ExecutionPlan const* plan, ConditionPart const& me, } // non-constant condition else if (me.operatorType == operand->type && - normalize(me.valueNode) == normalize(rhs)) { + normalize(me.valueNode) == normalize(rhs)) { return true; } } @@ -1027,7 +1055,7 @@ bool Condition::canRemove(ExecutionPlan const* plan, ConditionPart const& me, if (rhs->isAttributeAccessForVariable(result)) { if (lhs->isConstant()) { ConditionPart indexCondition(result.first, result.second, operand, - ATTRIBUTE_RIGHT, nullptr); + ATTRIBUTE_RIGHT, nullptr); if (me.isCoveredBy(indexCondition, true)) { return true; @@ -1035,7 +1063,7 @@ bool Condition::canRemove(ExecutionPlan const* plan, ConditionPart const& me, } // non-constant condition else if (me.operatorType == operand->type && - normalize(me.valueNode) == normalize(lhs)) { + normalize(me.valueNode) == normalize(lhs)) { return true; } } @@ -1074,7 +1102,8 @@ void Condition::deduplicateInOperation(AstNode* operation) { } /// @brief merge the values from two IN operations -AstNode* Condition::mergeInOperations(transaction::Methods* trx, AstNode const* lhs, AstNode const* rhs) { +AstNode* Condition::mergeInOperations(transaction::Methods* trx, + AstNode const* lhs, AstNode const* rhs) { TRI_ASSERT(lhs->type == NODE_TYPE_OPERATOR_BINARY_IN); TRI_ASSERT(rhs->type == NODE_TYPE_OPERATOR_BINARY_IN); @@ -1099,9 +1128,11 @@ AstNode* Condition::collapse(AstNode const* node) { for (size_t i = 0; i < n; ++i) { auto sub = node->getMemberUnchecked(i); - bool const isSame = (node->type == sub->type) || - (node->type == NODE_TYPE_OPERATOR_NARY_OR && sub->type == NODE_TYPE_OPERATOR_BINARY_OR) || - (node->type == NODE_TYPE_OPERATOR_NARY_AND && sub->type == NODE_TYPE_OPERATOR_BINARY_AND); + bool const isSame = (node->type == sub->type) || + (node->type == NODE_TYPE_OPERATOR_NARY_OR && + sub->type == NODE_TYPE_OPERATOR_BINARY_OR) || + (node->type == NODE_TYPE_OPERATOR_NARY_AND && + sub->type == NODE_TYPE_OPERATOR_BINARY_AND); if (isSame) { // merge @@ -1121,7 +1152,7 @@ AstNode* Condition::transformNode(AstNode* node) { if (node == nullptr) { return nullptr; } - + if (node->type == NODE_TYPE_OPERATOR_BINARY_AND || node->type == NODE_TYPE_OPERATOR_BINARY_OR) { // convert binary AND/OR into n-ary AND/OR @@ -1142,7 +1173,7 @@ AstNode* Condition::transformNode(AstNode* node) { bool processChildren = false; bool mustCollapse = false; size_t const n = node->numMembers(); - + for (size_t i = 0; i < n; ++i) { // process subnodes first auto sub = transformNode(node->getMemberUnchecked(i)); @@ -1154,7 +1185,7 @@ AstNode* Condition::transformNode(AstNode* node) { mustCollapse = true; } } - + if (processChildren) { // we found an AND with at least one OR child, e.g. // AND @@ -1174,7 +1205,7 @@ AstNode* Condition::transformNode(AstNode* node) { for (size_t i = 0; i < n; ++i) { auto sub = node->getMemberUnchecked(i); - if (sub->type == NODE_TYPE_OPERATOR_NARY_OR) { + if (sub->type == NODE_TYPE_OPERATOR_NARY_OR) { permutationStates.emplace_back(sub, sub->numMembers()); } else { permutationStates.emplace_back(sub, 1); @@ -1228,7 +1259,7 @@ AstNode* Condition::transformNode(AstNode* node) { if (node->type == NODE_TYPE_OPERATOR_NARY_OR) { size_t const n = node->numMembers(); bool mustCollapse = false; - + for (size_t i = 0; i < n; ++i) { auto sub = transformNode(node->getMemberUnchecked(i)); node->changeMember(i, sub); @@ -1237,7 +1268,7 @@ AstNode* Condition::transformNode(AstNode* node) { mustCollapse = true; } } - + if (mustCollapse) { node = collapse(node); } diff --git a/arangod/Aql/Condition.h b/arangod/Aql/Condition.h index ab9482575b..87c02f726f 100644 --- a/arangod/Aql/Condition.h +++ b/arangod/Aql/Condition.h @@ -178,6 +178,10 @@ class Condition { ~Condition(); public: + static void CollectOverlappingMembers( + ExecutionPlan const* plan, Variable const* variable, AstNode* andNode, + AstNode* otherAndNode, std::unordered_set& toRemove); + /// @brief return the condition root inline AstNode* root() const { return _root; } @@ -235,6 +239,7 @@ class Condition { std::vector> getConstAttributes (Variable const*, bool); private: + /// @brief sort ORs for the same attribute so they are in ascending value /// order. this will only work if the condition is for a single attribute bool sortOrs(Variable const*, std::vector&); @@ -254,7 +259,7 @@ class Condition { #endif /// @brief checks if the current condition covers the other - bool canRemove(ExecutionPlan const*, ConditionPart const&, AstNode const*) const; + static bool CanRemove(ExecutionPlan const*, ConditionPart const&, AstNode const*); /// @brief deduplicate IN condition values /// this may modify the node in place diff --git a/arangod/Aql/ShortestPathNode.cpp b/arangod/Aql/ShortestPathNode.cpp index 2db4759e04..2a961b9243 100644 --- a/arangod/Aql/ShortestPathNode.cpp +++ b/arangod/Aql/ShortestPathNode.cpp @@ -322,17 +322,17 @@ void ShortestPathNode::prepareOptions() { auto dir = _directions[i]; switch (dir) { case TRI_EDGE_IN: - opts->addLookupInfo(ast, _edgeColls[i]->getName(), + opts->addLookupInfo(_plan, _edgeColls[i]->getName(), StaticStrings::ToString, _toCondition->clone(ast)); - opts->addReverseLookupInfo(ast, _edgeColls[i]->getName(), + opts->addReverseLookupInfo(_plan, _edgeColls[i]->getName(), StaticStrings::FromString, _fromCondition->clone(ast)); break; case TRI_EDGE_OUT: - opts->addLookupInfo(ast, _edgeColls[i]->getName(), + opts->addLookupInfo(_plan, _edgeColls[i]->getName(), StaticStrings::FromString, _fromCondition->clone(ast)); - opts->addReverseLookupInfo(ast, _edgeColls[i]->getName(), + opts->addReverseLookupInfo(_plan, _edgeColls[i]->getName(), StaticStrings::ToString, _toCondition->clone(ast)); break; diff --git a/arangod/Aql/TraversalNode.cpp b/arangod/Aql/TraversalNode.cpp index 6a89bffebd..13cbdd18f4 100644 --- a/arangod/Aql/TraversalNode.cpp +++ b/arangod/Aql/TraversalNode.cpp @@ -496,12 +496,12 @@ void TraversalNode::prepareOptions() { switch (dir) { case TRI_EDGE_IN: _options->addLookupInfo( - ast, _edgeColls[i]->getName(), StaticStrings::ToString, + _plan, _edgeColls[i]->getName(), StaticStrings::ToString, globalEdgeConditionBuilder.getInboundCondition()->clone(ast)); break; case TRI_EDGE_OUT: _options->addLookupInfo( - ast, _edgeColls[i]->getName(), StaticStrings::FromString, + _plan, _edgeColls[i]->getName(), StaticStrings::FromString, globalEdgeConditionBuilder.getOutboundCondition()->clone(ast)); break; case TRI_EDGE_ANY: @@ -529,12 +529,12 @@ void TraversalNode::prepareOptions() { switch (dir) { case TRI_EDGE_IN: opts->addDepthLookupInfo( - ast, _edgeColls[i]->getName(), StaticStrings::ToString, + _plan, _edgeColls[i]->getName(), StaticStrings::ToString, builder->getInboundCondition()->clone(ast), depth); break; case TRI_EDGE_OUT: opts->addDepthLookupInfo( - ast, _edgeColls[i]->getName(), StaticStrings::FromString, + _plan, _edgeColls[i]->getName(), StaticStrings::FromString, builder->getOutboundCondition()->clone(ast), depth); break; case TRI_EDGE_ANY: diff --git a/arangod/Graph/BaseOptions.cpp b/arangod/Graph/BaseOptions.cpp index 66e94ea1e6..d1bd227ab5 100644 --- a/arangod/Graph/BaseOptions.cpp +++ b/arangod/Graph/BaseOptions.cpp @@ -23,10 +23,12 @@ #include "BaseOptions.h" #include "Aql/Ast.h" +#include "Aql/Condition.h" +#include "Aql/ExecutionPlan.h" #include "Aql/Expression.h" #include "Aql/Query.h" -#include "Graph/SingleServerEdgeCursor.h" #include "Graph/ShortestPathOptions.h" +#include "Graph/SingleServerEdgeCursor.h" #include "Graph/TraverserCache.h" #include "Graph/TraverserCacheFactory.h" #include "Indexes/Index.h" @@ -91,13 +93,12 @@ BaseOptions::LookupInfo::LookupInfo(arangodb::aql::Query* query, } read = info.get("expression"); - if (!read.isObject()) { - THROW_ARANGO_EXCEPTION_MESSAGE( - TRI_ERROR_BAD_PARAMETER, - "Each lookup requires expression to be an object"); + if (read.isObject()) { + expression = new aql::Expression(query->ast(), read); + } else { + expression = nullptr; } - expression = new aql::Expression(query->ast(), read); read = info.get("condition"); if (!read.isObject()) { @@ -114,7 +115,9 @@ BaseOptions::LookupInfo::LookupInfo(LookupInfo const& other) indexCondition(other.indexCondition), conditionNeedUpdate(other.conditionNeedUpdate), conditionMemberToUpdate(other.conditionMemberToUpdate) { - expression = other.expression->clone(nullptr); + if (other.expression != nullptr) { + expression = other.expression->clone(nullptr); + } } void BaseOptions::LookupInfo::buildEngineInfo(VPackBuilder& result) const { @@ -122,15 +125,17 @@ void BaseOptions::LookupInfo::buildEngineInfo(VPackBuilder& result) const { result.add(VPackValue("handle")); // We only run toVelocyPack on Coordinator. TRI_ASSERT(idxHandles.size() == 1); - // result.openObject(); + idxHandles[0].toVelocyPack(result, false); - // result.close(); - result.add(VPackValue("expression")); - result.openObject(); // We need to encapsulate the expression into an - // expression object - result.add(VPackValue("expression")); - expression->toVelocyPack(result, true); - result.close(); + + if (expression != nullptr) { + result.add(VPackValue("expression")); + result.openObject(); // We need to encapsulate the expression into an + // expression object + result.add(VPackValue("expression")); + expression->toVelocyPack(result, true); + result.close(); + } result.add(VPackValue("condition")); indexCondition->toVelocyPack(result, true); result.add("condNeedUpdate", VPackValue(conditionNeedUpdate)); @@ -153,7 +158,6 @@ double BaseOptions::LookupInfo::estimateCost(size_t& nrItems) const { return 1000.0; } - std::unique_ptr BaseOptions::createOptionsFromSlice( transaction::Methods* trx, VPackSlice const& definition) { VPackSlice type = definition.get("type"); @@ -226,22 +230,21 @@ void BaseOptions::setVariable(aql::Variable const* variable) { _tmpVar = variable; } -void BaseOptions::addLookupInfo(aql::Ast* ast, +void BaseOptions::addLookupInfo(aql::ExecutionPlan* plan, std::string const& collectionName, std::string const& attributeName, aql::AstNode* condition) { - injectLookupInfoInList(_baseLookupInfos, ast, collectionName, attributeName, + injectLookupInfoInList(_baseLookupInfos, plan, collectionName, attributeName, condition); } void BaseOptions::injectLookupInfoInList(std::vector& list, - aql::Ast* ast, + aql::ExecutionPlan* plan, std::string const& collectionName, std::string const& attributeName, aql::AstNode* condition) { LookupInfo info; - info.indexCondition = condition; - info.expression = new aql::Expression(ast, condition->clone(ast)); + info.indexCondition = condition->clone(plan->getAst()); bool res = _trx->getBestIndexHandleForFilterCondition( collectionName, info.indexCondition, _tmpVar, 1000, info.idxHandles[0]); TRI_ASSERT(res); // Right now we have an enforced edge index which will @@ -282,6 +285,23 @@ void BaseOptions::injectLookupInfoInList(std::vector& list, continue; } } + std::unordered_set toRemove; + aql::Condition::CollectOverlappingMembers(plan, _tmpVar, condition, info.indexCondition, toRemove); + size_t n = condition->numMembers(); + if (n == toRemove.size()) { + // FastPath, all covered. + info.expression = nullptr; + } else { + // Slow path need to explicitly remove nodes. + for (; n > 0; --n) { + // Now n is one more than the idx we actually check + if (toRemove.find(n - 1) != toRemove.end()) { + // This index has to be removed. + condition->removeMemberUnchecked(n - 1); + } + } + info.expression = new aql::Expression(plan->getAst(), condition); + } list.emplace_back(std::move(info)); } @@ -332,9 +352,10 @@ void BaseOptions::injectEngineInfo(VPackBuilder& result) const { } arangodb::aql::Expression* BaseOptions::getEdgeExpression( - size_t cursorId) const { + size_t cursorId, bool& needToInjectVertex) const { TRI_ASSERT(!_baseLookupInfos.empty()); TRI_ASSERT(_baseLookupInfos.size() > cursorId); + needToInjectVertex = !_baseLookupInfos[cursorId].conditionNeedUpdate; return _baseLookupInfos[cursorId].expression; } @@ -417,7 +438,9 @@ TraverserCache* BaseOptions::cache() { return _cache.get(); } -void BaseOptions::activateCache(bool enableDocumentCache, std::unordered_map const* engines) { +void BaseOptions::activateCache( + bool enableDocumentCache, + std::unordered_map const* engines) { // Do not call this twice. TRI_ASSERT(_cache == nullptr); _cache.reset(cacheFactory::CreateCache(_trx, enableDocumentCache, engines)); diff --git a/arangod/Graph/BaseOptions.h b/arangod/Graph/BaseOptions.h index e3a81a57ae..cf415e5b24 100644 --- a/arangod/Graph/BaseOptions.h +++ b/arangod/Graph/BaseOptions.h @@ -35,6 +35,7 @@ namespace arangodb { namespace aql { struct AstNode; +class ExecutionPlan; class Expression; class Query; } @@ -98,7 +99,7 @@ struct BaseOptions { void setVariable(aql::Variable const*); - void addLookupInfo(aql::Ast* ast, std::string const& collectionName, + void addLookupInfo(aql::ExecutionPlan* plan, std::string const& collectionName, std::string const& attributeName, aql::AstNode* condition); void clearVariableValues(); @@ -142,12 +143,12 @@ struct BaseOptions { // Does not close the builder. void injectEngineInfo(arangodb::velocypack::Builder&) const; - aql::Expression* getEdgeExpression(size_t cursorId) const; + aql::Expression* getEdgeExpression(size_t cursorId, bool& needToInjectVertex) const; bool evaluateExpression(aql::Expression*, arangodb::velocypack::Slice varValue) const; - void injectLookupInfoInList(std::vector&, aql::Ast* ast, + void injectLookupInfoInList(std::vector&, aql::ExecutionPlan* plan, std::string const& collectionName, std::string const& attributeName, aql::AstNode* condition); diff --git a/arangod/Graph/ShortestPathOptions.cpp b/arangod/Graph/ShortestPathOptions.cpp index a6f5585ae4..70ffe67dc3 100644 --- a/arangod/Graph/ShortestPathOptions.cpp +++ b/arangod/Graph/ShortestPathOptions.cpp @@ -169,9 +169,9 @@ double ShortestPathOptions::estimateCost(size_t& nrItems) const { } void ShortestPathOptions::addReverseLookupInfo( - aql::Ast* ast, std::string const& collectionName, + aql::ExecutionPlan* plan, std::string const& collectionName, std::string const& attributeName, aql::AstNode* condition) { - injectLookupInfoInList(_reverseLookupInfos, ast, collectionName, + injectLookupInfoInList(_reverseLookupInfos, plan, collectionName, attributeName, condition); } diff --git a/arangod/Graph/ShortestPathOptions.h b/arangod/Graph/ShortestPathOptions.h index e41060eaca..3b6a5b5ab4 100644 --- a/arangod/Graph/ShortestPathOptions.h +++ b/arangod/Graph/ShortestPathOptions.h @@ -29,6 +29,7 @@ namespace arangodb { namespace aql { +class ExecutionPlan; class Query; } @@ -90,7 +91,7 @@ struct ShortestPathOptions : public BaseOptions { // Creates a complete Object containing all EngineInfo // in the given builder. - void addReverseLookupInfo(aql::Ast* ast, std::string const& collectionName, + void addReverseLookupInfo(aql::ExecutionPlan* plan, std::string const& collectionName, std::string const& attributeName, aql::AstNode* condition); diff --git a/arangod/VocBase/TraverserOptions.cpp b/arangod/VocBase/TraverserOptions.cpp index 75f7fc4293..246b13936a 100644 --- a/arangod/VocBase/TraverserOptions.cpp +++ b/arangod/VocBase/TraverserOptions.cpp @@ -398,13 +398,13 @@ void TraverserOptions::buildEngineInfo(VPackBuilder& result) const { result.close(); } -void TraverserOptions::addDepthLookupInfo(aql::Ast* ast, +void TraverserOptions::addDepthLookupInfo(aql::ExecutionPlan* plan, std::string const& collectionName, std::string const& attributeName, aql::AstNode* condition, uint64_t depth) { auto& list = _depthLookupInfo[depth]; - injectLookupInfoInList(list, ast, collectionName, attributeName, condition); + injectLookupInfoInList(list, plan, collectionName, attributeName, condition); } bool TraverserOptions::vertexHasFilter(uint64_t depth) const { @@ -428,7 +428,8 @@ bool TraverserOptions::hasEdgeFilter(int64_t depth, size_t cursorId) const { TRI_ASSERT(specific->second.size() > cursorId); expression = specific->second[cursorId].expression; } else { - expression = getEdgeExpression(cursorId); + bool unused; + expression = getEdgeExpression(cursorId, unused); } return expression != nullptr; } @@ -445,32 +446,40 @@ bool TraverserOptions::evaluateEdgeExpression(arangodb::velocypack::Slice edge, arangodb::aql::Expression* expression = nullptr; auto specific = _depthLookupInfo.find(depth); + auto needToInjectVertex = false; if (specific != _depthLookupInfo.end()) { TRI_ASSERT(!specific->second.empty()); TRI_ASSERT(specific->second.size() > cursorId); expression = specific->second[cursorId].expression; + needToInjectVertex = !specific->second[cursorId].conditionNeedUpdate; } else { - expression = getEdgeExpression(cursorId); + expression = getEdgeExpression(cursorId, needToInjectVertex); + } if (expression == nullptr) { return true; } - // inject _from/_to value - auto node = expression->nodeForModification(); + if (needToInjectVertex) { + // If we have to inject the vertex value it has to be within + // the last member of the condition. + // We only get into this case iff the index used does + // not cover _from resp. _to. + // inject _from/_to value + auto node = expression->nodeForModification(); - TRI_ASSERT(node->numMembers() > 0); - auto dirCmp = node->getMemberUnchecked(node->numMembers() - 1); - TRI_ASSERT(dirCmp->type == aql::NODE_TYPE_OPERATOR_BINARY_EQ); - TRI_ASSERT(dirCmp->numMembers() == 2); - - auto idNode = dirCmp->getMemberUnchecked(1); - TRI_ASSERT(idNode->type == aql::NODE_TYPE_VALUE); - TRI_ASSERT(idNode->isValueType(aql::VALUE_TYPE_STRING)); - idNode->stealComputedValue(); - idNode->setStringValue(vertexId.data(), vertexId.length()); + TRI_ASSERT(node->numMembers() > 0); + auto dirCmp = node->getMemberUnchecked(node->numMembers() - 1); + TRI_ASSERT(dirCmp->type == aql::NODE_TYPE_OPERATOR_BINARY_EQ); + TRI_ASSERT(dirCmp->numMembers() == 2); + auto idNode = dirCmp->getMemberUnchecked(1); + TRI_ASSERT(idNode->type == aql::NODE_TYPE_VALUE); + TRI_ASSERT(idNode->isValueType(aql::VALUE_TYPE_STRING)); + idNode->stealComputedValue(); + idNode->setStringValue(vertexId.data(), vertexId.length()); + } return evaluateExpression(expression, edge); } diff --git a/arangod/VocBase/TraverserOptions.h b/arangod/VocBase/TraverserOptions.h index db21338d55..cae24d468b 100644 --- a/arangod/VocBase/TraverserOptions.h +++ b/arangod/VocBase/TraverserOptions.h @@ -106,7 +106,7 @@ struct TraverserOptions : public graph::BaseOptions { void buildEngineInfo(arangodb::velocypack::Builder&) const; /// @brief Add a lookup info for specific depth - void addDepthLookupInfo(aql::Ast* ast, std::string const& collectionName, + void addDepthLookupInfo(aql::ExecutionPlan* plan, std::string const& collectionName, std::string const& attributeName, aql::AstNode* condition, uint64_t depth);