From 3a95244599dea8536b277fa688dd842f999d83cb Mon Sep 17 00:00:00 2001 From: Michael Hackstein Date: Wed, 10 Aug 2016 17:10:25 +0200 Subject: [PATCH] Fixed ALL== and NONE== tests for graph traversals. SingleServer and cluster --- arangod/Aql/TraversalNode.cpp | 98 +++++++++++++++------- arangod/Aql/TraversalNode.h | 4 +- arangod/VocBase/TraverserOptions.cpp | 54 ++++++++++-- arangod/VocBase/TraverserOptions.h | 2 + js/server/tests/aql/aql-graph-traverser.js | 44 ++++++++-- 5 files changed, 151 insertions(+), 51 deletions(-) diff --git a/arangod/Aql/TraversalNode.cpp b/arangod/Aql/TraversalNode.cpp index 679be26f0c..8e7985c6b9 100644 --- a/arangod/Aql/TraversalNode.cpp +++ b/arangod/Aql/TraversalNode.cpp @@ -60,8 +60,8 @@ AstNode* TraversalNode::EdgeConditionBuilder::getOutboundCondition() { if (_containsCondition) { _modCondition->changeMember(_modCondition->numMembers() - 1, _tn->_fromCondition); } else { - if (_tn->_globalEdgeCondition != nullptr) { - _modCondition->addMember(_tn->_globalEdgeCondition); + for (auto& it : _tn->_globalEdgeConditions) { + _modCondition->addMember(it); } TRI_ASSERT(_tn->_fromCondition != nullptr); TRI_ASSERT(_tn->_fromCondition->type == NODE_TYPE_OPERATOR_BINARY_EQ); @@ -76,8 +76,8 @@ AstNode* TraversalNode::EdgeConditionBuilder::getInboundCondition() { if (_containsCondition) { _modCondition->changeMember(_modCondition->numMembers() - 1, _tn->_toCondition); } else { - if (_tn->_globalEdgeCondition != nullptr) { - _modCondition->addMember(_tn->_globalEdgeCondition); + for (auto& it : _tn->_globalEdgeConditions) { + _modCondition->addMember(it); } TRI_ASSERT(_tn->_toCondition != nullptr); TRI_ASSERT(_tn->_toCondition->type == NODE_TYPE_OPERATOR_BINARY_EQ); @@ -131,8 +131,6 @@ TraversalNode::TraversalNode(ExecutionPlan* plan, size_t id, _tmpIdNode(_plan->getAst()->createNodeValueString("", 0)), _fromCondition(nullptr), _toCondition(nullptr), - _globalEdgeCondition(nullptr), - _globalVertexCondition(nullptr), _optionsBuild(false) { TRI_ASSERT(_vocbase != nullptr); TRI_ASSERT(direction != nullptr); @@ -369,8 +367,6 @@ TraversalNode::TraversalNode(ExecutionPlan* plan, _tmpIdNode(nullptr), _fromCondition(nullptr), _toCondition(nullptr), - _globalEdgeCondition(nullptr), - _globalVertexCondition(nullptr), _optionsBuild(false) { _options = std::make_unique( _plan->getAst()->query()->trx(), base); @@ -523,12 +519,30 @@ TraversalNode::TraversalNode(ExecutionPlan* plan, TRI_ASSERT(base.has("toCondition")); _toCondition = new AstNode(plan->getAst(), base.get("toCondition")); - if (base.has("globalEdgeCondition")) { - _globalEdgeCondition = new AstNode(plan->getAst(), base.get("globalEdgeCondition")); + if (base.has("globalEdgeConditions")) { + TRI_json_t const* list = + JsonHelper::checkAndGetArrayValue(base.json(), "globalEdgeConditions"); + size_t count = TRI_LengthVector(&list->_value._objects); + // List of edge collection names + for (size_t i = 0; i < count; ++i) { + Json cond(TRI_UNKNOWN_MEM_ZONE, + static_cast( + TRI_AtVector(&list->_value._objects, i))); + _globalEdgeConditions.emplace_back(new AstNode(plan->getAst(), cond)); + } } - if (base.has("globalVertexCondition")) { - _globalVertexCondition = new AstNode(plan->getAst(), base.get("globalVertexCondition")); + if (base.has("globalVertexConditions")) { + TRI_json_t const* list = + JsonHelper::checkAndGetArrayValue(base.json(), "globalVertexConditions"); + size_t count = TRI_LengthVector(&list->_value._objects); + // List of edge collection names + for (size_t i = 0; i < count; ++i) { + Json cond(TRI_UNKNOWN_MEM_ZONE, + static_cast( + TRI_AtVector(&list->_value._objects, i))); + _globalVertexConditions.emplace_back(new AstNode(plan->getAst(), cond)); + } } if (base.has("vertexConditions")) { @@ -745,14 +759,21 @@ void TraversalNode::toVelocyPackHelper(arangodb::velocypack::Builder& nodes, nodes.add(VPackValue("toCondition")); _toCondition->toVelocyPack(nodes, verbose); - if (_globalEdgeCondition) { - nodes.add(VPackValue("globalEdgeCondition")); - _globalEdgeCondition->toVelocyPack(nodes, verbose); + if (!_globalEdgeConditions.empty()) { + nodes.add(VPackValue("globalEdgeConditions")); + nodes.openArray(); + for (auto const& it : _globalEdgeConditions) { + it->toVelocyPack(nodes, verbose); + } + nodes.close(); } - if (_globalVertexCondition) { - nodes.add(VPackValue("globalVertexCondition")); - _globalVertexCondition->toVelocyPack(nodes, verbose); + if (!_globalVertexConditions.empty()) { + nodes.add(VPackValue("globalVertexConditions")); + nodes.openArray(); + for (auto const& it : _globalVertexConditions) { + it->toVelocyPack(nodes, verbose); + } } if (!_vertexConditions.empty()) { @@ -840,13 +861,12 @@ ExecutionNode* TraversalNode::clone(ExecutionPlan* plan, bool withDependencies, // Filter Condition Parts c->_fromCondition = _fromCondition->clone(_plan->getAst()); c->_toCondition = _toCondition->clone(_plan->getAst()); - if (c->_globalEdgeCondition != nullptr) { - c->_globalEdgeCondition = _globalEdgeCondition; - } - - if (c->_globalVertexCondition != nullptr) { - c->_globalVertexCondition = _globalVertexCondition; - } + c->_globalEdgeConditions.insert(c->_globalEdgeConditions.end(), + _globalEdgeConditions.begin(), + _globalEdgeConditions.end()); + c->_globalVertexConditions.insert(c->_globalVertexConditions.end(), + _globalVertexConditions.begin(), + _globalVertexConditions.end()); for (auto const& it : _edgeConditions) { c->_edgeConditions.emplace(it.first, it.second); @@ -952,6 +972,10 @@ void TraversalNode::prepareOptions() { for (std::pair it : _edgeConditions) { auto ins = _options->_depthLookupInfo.emplace( it.first, std::vector()); + // We probably have to adopt minDepth. We cannot fulfill a condition of larger depth anyway + if (_options->minDepth < it.first + 1) { + _options->minDepth = it.first + 1; + } TRI_ASSERT(ins.second); auto& infos = ins.first->second; infos.reserve(numEdgeColls); @@ -975,7 +999,6 @@ void TraversalNode::prepareOptions() { } info.expression = new Expression(ast, info.indexCondition->clone(ast)); -#warning hard-coded nrItems. res = trx->getBestIndexHandleForFilterCondition( _edgeColls[i]->getName(), info.indexCondition, _tmpObjVariable, 1000, info.idxHandles[0]); @@ -986,9 +1009,25 @@ void TraversalNode::prepareOptions() { } for (auto& it : _vertexConditions) { + // We inject the base conditions as well here. + for (auto const& jt : _globalVertexConditions) { + it.second->addMember(jt); + } _options->_vertexExpressions.emplace(it.first, new Expression(ast, it.second)); + if (_options->minDepth < it.first) { + _options->minDepth = it.first; + } TRI_ASSERT(!_options->_vertexExpressions[it.first]->isV8()); } + if (!_globalVertexConditions.empty()) { + auto cond = _plan->getAst()->createNodeNaryOperator(NODE_TYPE_OPERATOR_NARY_AND); + for (auto const& it : _globalVertexConditions) { + cond->addMember(it); + } + _options->_baseVertexExpression = new Expression(ast, cond); + TRI_ASSERT(!_options->_baseVertexExpression->isV8()); + + } _optionsBuild = true; } @@ -1034,9 +1073,6 @@ void TraversalNode::registerCondition(bool isConditionOnEdge, auto const& it = _vertexConditions.find(conditionLevel); if (it == _vertexConditions.end()) { auto cond = _plan->getAst()->createNodeNaryOperator(NODE_TYPE_OPERATOR_NARY_AND); - if (_globalVertexCondition != nullptr) { - cond->addMember(_globalVertexCondition); - } cond->addMember(condition); _vertexConditions.emplace(conditionLevel, cond); } else { @@ -1049,9 +1085,9 @@ void TraversalNode::registerGlobalCondition(bool isConditionOnEdge, AstNode const* condition) { Ast::getReferencedVariables(condition, _conditionVariables); if (isConditionOnEdge) { - _globalEdgeCondition = condition; + _globalEdgeConditions.emplace_back(condition); } else { - _globalVertexCondition = condition; + _globalVertexConditions.emplace_back(condition); } } diff --git a/arangod/Aql/TraversalNode.h b/arangod/Aql/TraversalNode.h index 4ca14557cf..5c48801194 100644 --- a/arangod/Aql/TraversalNode.h +++ b/arangod/Aql/TraversalNode.h @@ -324,10 +324,10 @@ class TraversalNode : public ExecutionNode { /// @brief The global edge condition. Does not contain /// _from and _to checks - AstNode const* _globalEdgeCondition; + std::vector _globalEdgeConditions; /// @brief The global vertex condition - AstNode const* _globalVertexCondition; + std::vector _globalVertexConditions; /// @brief List of all depth specific conditions for edges std::unordered_map _edgeConditions; diff --git a/arangod/VocBase/TraverserOptions.cpp b/arangod/VocBase/TraverserOptions.cpp index ba87d74f24..ef148472c1 100644 --- a/arangod/VocBase/TraverserOptions.cpp +++ b/arangod/VocBase/TraverserOptions.cpp @@ -124,6 +124,7 @@ void arangodb::traverser::TraverserOptions::LookupInfo::buildEngineInfo( arangodb::traverser::TraverserOptions::TraverserOptions( arangodb::Transaction* trx, Json const& json) : _trx(trx), + _baseVertexExpression(nullptr), _tmpVar(nullptr), _ctx(new aql::FixedVarExpressionContext()), minDepth(1), @@ -165,6 +166,7 @@ arangodb::traverser::TraverserOptions::TraverserOptions( arangodb::traverser::TraverserOptions::TraverserOptions( arangodb::aql::Query* query, VPackSlice info, VPackSlice collections) : _trx(query->trx()), + _baseVertexExpression(nullptr), _tmpVar(nullptr), _ctx(new aql::FixedVarExpressionContext()), minDepth(1), @@ -296,6 +298,18 @@ arangodb::traverser::TraverserOptions::TraverserOptions( TRI_ASSERT(it.second); } } + + read = info.get("baseVertexExpression"); + if (!read.isNone()) { + if (!read.isObject()) { + THROW_ARANGO_EXCEPTION_MESSAGE( + TRI_ERROR_BAD_PARAMETER, + "The options require vertexExpressions to be an object"); + } + arangodb::basics::Json infoJson(TRI_UNKNOWN_MEM_ZONE, + arangodb::basics::VelocyPackHelper::velocyPackToJson(read)); + _baseVertexExpression = new aql::Expression(query->ast(), infoJson); + } } arangodb::traverser::TraverserOptions::TraverserOptions( @@ -312,6 +326,7 @@ arangodb::traverser::TraverserOptions::TraverserOptions( TRI_ASSERT(other._depthLookupInfo.empty()); TRI_ASSERT(other._vertexExpressions.empty()); TRI_ASSERT(other._tmpVar == nullptr); + TRI_ASSERT(other._baseVertexExpression == nullptr); } arangodb::traverser::TraverserOptions::~TraverserOptions() { @@ -320,6 +335,9 @@ arangodb::traverser::TraverserOptions::~TraverserOptions() { delete pair.second; } } + if (_baseVertexExpression != nullptr) { + delete _baseVertexExpression; + } if (_ctx != nullptr) { delete _ctx; } @@ -420,11 +438,18 @@ void arangodb::traverser::TraverserOptions::buildEngineInfo(VPackBuilder& result result.add(VPackValue("expression")); pair.second->toVelocyPack(result, true); result.close(); - } result.close(); } + if (_baseVertexExpression != nullptr) { + result.add(VPackValue("baseVertexExpression")); + result.openObject(); + result.add(VPackValue("expression")); + _baseVertexExpression->toVelocyPack(result, true); + result.close(); + } + result.add(VPackValue("tmpVar")); _tmpVar->toVelocyPack(result); @@ -433,6 +458,9 @@ void arangodb::traverser::TraverserOptions::buildEngineInfo(VPackBuilder& result bool arangodb::traverser::TraverserOptions::vertexHasFilter( size_t depth) const { + if (_baseVertexExpression != nullptr) { + return true; + } return _vertexExpressions.find(depth) != _vertexExpressions.end(); } @@ -488,17 +516,25 @@ bool arangodb::traverser::TraverserOptions::evaluateEdgeExpression( bool arangodb::traverser::TraverserOptions::evaluateVertexExpression( arangodb::velocypack::Slice vertex, size_t depth) const { bool mustDestroy = false; + arangodb::aql::Expression* expression = nullptr; + auto specific = _vertexExpressions.find(depth); if (specific != _vertexExpressions.end()) { - arangodb::aql::Expression* expression = specific->second; - TRI_ASSERT(!expression->isV8()); - expression->setVariable(_tmpVar, vertex); - aql::AqlValue res = expression->execute(_trx, _ctx, mustDestroy); - TRI_ASSERT(res.isBoolean()); - expression->clearVariable(_tmpVar); - return res.toBoolean(); + expression = specific->second; + } else { + expression = _baseVertexExpression; } - return true; + + if (expression == nullptr) { + return true; + } + + TRI_ASSERT(!expression->isV8()); + expression->setVariable(_tmpVar, vertex); + aql::AqlValue res = expression->execute(_trx, _ctx, mustDestroy); + TRI_ASSERT(res.isBoolean()); + expression->clearVariable(_tmpVar); + return res.toBoolean(); } arangodb::traverser::EdgeCursor* diff --git a/arangod/VocBase/TraverserOptions.h b/arangod/VocBase/TraverserOptions.h index d38c5a2408..0b7d885021 100644 --- a/arangod/VocBase/TraverserOptions.h +++ b/arangod/VocBase/TraverserOptions.h @@ -94,6 +94,7 @@ struct TraverserOptions { std::vector _baseLookupInfos; std::unordered_map> _depthLookupInfo; std::unordered_map _vertexExpressions; + aql::Expression* _baseVertexExpression; aql::Variable const* _tmpVar; aql::FixedVarExpressionContext* _ctx; arangodb::traverser::ClusterTraverser* _traverser; @@ -111,6 +112,7 @@ struct TraverserOptions { explicit TraverserOptions(arangodb::Transaction* trx) : _trx(trx), + _baseVertexExpression(nullptr), _tmpVar(nullptr), _ctx(new aql::FixedVarExpressionContext()), minDepth(1), diff --git a/js/server/tests/aql/aql-graph-traverser.js b/js/server/tests/aql/aql-graph-traverser.js index f55ba440cd..c6c7fd14ad 100644 --- a/js/server/tests/aql/aql-graph-traverser.js +++ b/js/server/tests/aql/aql-graph-traverser.js @@ -2754,7 +2754,11 @@ function optimizeQuantifierSuite() { let stats = cursor.getExtra().stats; assertEqual(stats.scannedFull, 0); - assertEqual(stats.scannedIndex, 8); + if (isCluster) { + assertEqual(stats.scannedIndex, 7); + } else { + assertEqual(stats.scannedIndex, 8); + } assertEqual(stats.filtered, 1); query = ` @@ -2770,7 +2774,11 @@ function optimizeQuantifierSuite() { stats = cursor.getExtra().stats; assertEqual(stats.scannedFull, 0); - assertEqual(stats.scannedIndex, 8); + if (isCluster) { + assertEqual(stats.scannedIndex, 7); + } else { + assertEqual(stats.scannedIndex, 8); + } assertEqual(stats.filtered, 1); }, @@ -2820,7 +2828,11 @@ function optimizeQuantifierSuite() { let stats = cursor.getExtra().stats; assertEqual(stats.scannedFull, 0); - assertEqual(stats.scannedIndex, 8); + if (isCluster) { + assertEqual(stats.scannedIndex, 7); + } else { + assertEqual(stats.scannedIndex, 8); + } assertEqual(stats.filtered, 1); query = ` @@ -2836,7 +2848,11 @@ function optimizeQuantifierSuite() { stats = cursor.getExtra().stats; assertEqual(stats.scannedFull, 0); - assertEqual(stats.scannedIndex, 8); + if (isCluster) { + assertEqual(stats.scannedIndex, 7); + } else { + assertEqual(stats.scannedIndex, 8); + } assertEqual(stats.filtered, 1); }, @@ -2874,7 +2890,11 @@ function optimizeQuantifierSuite() { let stats = cursor.getExtra().stats; assertEqual(stats.scannedFull, 0); - assertEqual(stats.scannedIndex, 9); + if (isCluster) { + assertEqual(stats.scannedIndex, 5); + } else { + assertEqual(stats.scannedIndex, 7); + } assertEqual(stats.filtered, 2); }, @@ -2912,7 +2932,11 @@ function optimizeQuantifierSuite() { let stats = cursor.getExtra().stats; assertEqual(stats.scannedFull, 0); - assertEqual(stats.scannedIndex, 9); + if (isCluster) { + assertEqual(stats.scannedIndex, 5); + } else { + assertEqual(stats.scannedIndex, 7); + } assertEqual(stats.filtered, 2); }, @@ -2950,12 +2974,14 @@ function optimizeQuantifierSuite() { let stats = cursor.getExtra().stats; assertEqual(stats.scannedFull, 0); - assertEqual(stats.scannedIndex, 9); + if (isCluster) { + assertEqual(stats.scannedIndex, 5); + } else { + assertEqual(stats.scannedIndex, 7); + } assertEqual(stats.filtered, 2); } - }; - }; jsunity.run(namedGraphSuite);