diff --git a/arangod/Aql/ExecutionBlock.cpp b/arangod/Aql/ExecutionBlock.cpp index b2c7545823..9bc447ca6f 100644 --- a/arangod/Aql/ExecutionBlock.cpp +++ b/arangod/Aql/ExecutionBlock.cpp @@ -67,11 +67,6 @@ void ExecutionBlock::walk (WalkerWorker* worker) { worker->after(this); } - -//////////////////////////////////////////////////////////////////////////////// -/// @brief -//////////////////////////////////////////////////////////////////////////////// - // Local Variables: // mode: outline-minor // outline-regexp: "^\\(/// @brief\\|/// {@inheritDoc}\\|/// @addtogroup\\|// --SECTION--\\|/// @\\}\\)" diff --git a/arangod/Aql/ExecutionBlock.h b/arangod/Aql/ExecutionBlock.h index d69d395229..0c4ed97548 100644 --- a/arangod/Aql/ExecutionBlock.h +++ b/arangod/Aql/ExecutionBlock.h @@ -594,7 +594,7 @@ namespace triagens { //////////////////////////////////////////////////////////////////////////////// ~EnumerateCollectionBlock () { - if (_allDocs.size() > 0) { + if (! _allDocs.empty()) { for (auto it = _allDocs.begin(); it != _allDocs.end(); ++it) { delete *it; } @@ -627,6 +627,7 @@ namespace triagens { auto shaper = trx.documentCollection()->getShaper(); + // TODO: if _allDocs is not empty, its contents will leak _allDocs.clear(); for (size_t i = 0; i < n; ++i) { TRI_shaped_json_t shaped; @@ -637,7 +638,7 @@ namespace triagens { res = trx.finish(res); - if (_allDocs.size() == 0) { + if (_allDocs.empty()) { _done = true; } @@ -653,7 +654,7 @@ namespace triagens { if (res != TRI_ERROR_NO_ERROR) { return res; } - if (_allDocs.size() == 0) { + if (_allDocs.empty()) { _done = true; } return TRI_ERROR_NO_ERROR; @@ -773,7 +774,7 @@ namespace triagens { private: - vector _allDocs; + std::vector _allDocs; //////////////////////////////////////////////////////////////////////////////// /// @brief current position in _allDocs @@ -783,19 +784,19 @@ namespace triagens { }; // ----------------------------------------------------------------------------- -// --SECTION-- RootBlock +// --SECTION-- ReturnBlock // ----------------------------------------------------------------------------- - class RootBlock : public ExecutionBlock { + class ReturnBlock : public ExecutionBlock { public: - RootBlock (RootNode const* ep) + ReturnBlock (ReturnNode const* ep) : ExecutionBlock(ep) { } - ~RootBlock () { + ~ReturnBlock () { } virtual AqlItemBlock* getOne () { @@ -808,7 +809,7 @@ namespace triagens { // Let's steal the actual result and throw away the vars: AqlItemBlock* stripped = new AqlItemBlock(1, 1); - auto ep = static_cast(getPlanNode()); + auto ep = static_cast(getPlanNode()); auto it = _varOverview->varInfo.find(ep->_inVariable->id); TRI_ASSERT(it != _varOverview->varInfo.end()); unsigned int index = it->second.index; @@ -826,7 +827,7 @@ namespace triagens { } // Let's steal the actual result and throw away the vars: - auto ep = static_cast(getPlanNode()); + auto ep = static_cast(getPlanNode()); auto it = _varOverview->varInfo.find(ep->_inVariable->id); TRI_ASSERT(it != _varOverview->varInfo.end()); unsigned int index = it->second.index; diff --git a/arangod/Aql/ExecutionEngine.cpp b/arangod/Aql/ExecutionEngine.cpp index a69dc5b3ae..d3823ceb45 100644 --- a/arangod/Aql/ExecutionEngine.cpp +++ b/arangod/Aql/ExecutionEngine.cpp @@ -88,7 +88,7 @@ struct Instanciator : public ExecutionNode::WalkerWorker { break; } case ExecutionNode::ROOT: { - eb = new RootBlock(static_cast(en)); + eb = new ReturnBlock(static_cast(en)); root = eb; break; } @@ -138,14 +138,13 @@ ExecutionEngine* ExecutionEngine::instanciateFromPlan (ExecutionNode* plan) { root->initialize(); engine->_root = root; - + + return engine; } catch (...) { delete engine; throw; } - - return engine; } // ----------------------------------------------------------------------------- diff --git a/arangod/Aql/ExecutionNode.cpp b/arangod/Aql/ExecutionNode.cpp index 667f17b373..987251eee0 100644 --- a/arangod/Aql/ExecutionNode.cpp +++ b/arangod/Aql/ExecutionNode.cpp @@ -414,16 +414,16 @@ void AggregateOnUnsortedNode::toJsonHelper (std::map& index } // ----------------------------------------------------------------------------- -// --SECTION-- methods of RootNode +// --SECTION-- methods of ReturnNode // ----------------------------------------------------------------------------- //////////////////////////////////////////////////////////////////////////////// -/// @brief toJson, for RootNode +/// @brief toJson, for ReturnNode //////////////////////////////////////////////////////////////////////////////// -void RootNode::toJsonHelper (std::map& indexTab, - triagens::basics::Json& nodes, - TRI_memory_zone_t* zone) { +void ReturnNode::toJsonHelper (std::map& indexTab, + triagens::basics::Json& nodes, + TRI_memory_zone_t* zone) { Json json(ExecutionNode::toJsonHelperGeneric(indexTab, nodes, zone)); // call base class method if (json.isEmpty()) { return; diff --git a/arangod/Aql/ExecutionNode.h b/arangod/Aql/ExecutionNode.h index 917bd4c0cd..256ddb70b0 100644 --- a/arangod/Aql/ExecutionNode.h +++ b/arangod/Aql/ExecutionNode.h @@ -1073,17 +1073,17 @@ namespace triagens { // ----------------------------------------------------------------------------- -// --SECTION-- class RootNode +// --SECTION-- class ReturnNode // ----------------------------------------------------------------------------- //////////////////////////////////////////////////////////////////////////////// -/// @brief class RootNode, derived from ExecutionNode +/// @brief class ReturnNode, derived from ExecutionNode //////////////////////////////////////////////////////////////////////////////// - class RootNode : public ExecutionNode { + class ReturnNode : public ExecutionNode { friend class ExecutionBlock; - friend class RootBlock; + friend class ReturnBlock; //////////////////////////////////////////////////////////////////////////////// /// @brief constructors for various arguments, always with offset and limit @@ -1091,7 +1091,7 @@ namespace triagens { public: - RootNode (Variable const* inVariable) + ReturnNode (Variable const* inVariable) : ExecutionNode(), _inVariable(inVariable) { TRI_ASSERT(_inVariable != nullptr); @@ -1110,7 +1110,7 @@ namespace triagens { //////////////////////////////////////////////////////////////////////////////// virtual std::string getTypeString () const { - return std::string("RootNode"); + return std::string("ReturnNode"); } //////////////////////////////////////////////////////////////////////////////// @@ -1126,7 +1126,7 @@ namespace triagens { //////////////////////////////////////////////////////////////////////////////// virtual ExecutionNode* clone () const { - auto c = new RootNode(_inVariable); + auto c = new ReturnNode(_inVariable); cloneDependencies(c); return static_cast(c); } diff --git a/arangod/Aql/ExecutionPlan.cpp b/arangod/Aql/ExecutionPlan.cpp index 4d4365bbca..1ac7c09885 100644 --- a/arangod/Aql/ExecutionPlan.cpp +++ b/arangod/Aql/ExecutionPlan.cpp @@ -96,13 +96,20 @@ ExecutionPlan* ExecutionPlan::instanciateFromAst (Ast const* ast) { // ----------------------------------------------------------------------------- //////////////////////////////////////////////////////////////////////////////// -/// @brief add a node to the plan +/// @brief add a node to the plan, will delete node if addition fails //////////////////////////////////////////////////////////////////////////////// -void ExecutionPlan::addNode (ExecutionNode* node) { +ExecutionNode* ExecutionPlan::addNode (ExecutionNode* node) { TRI_ASSERT(node != nullptr); - _nodes.push_back(node); + try { + _nodes.push_back(node); + return node; + } + catch (...) { + delete node; + throw; + } } //////////////////////////////////////////////////////////////////////////////// @@ -119,7 +126,10 @@ CalculationNode* ExecutionPlan::createTemporaryCalculation (Ast const* ast, auto expr = new Expression(ast->query()->executor(), const_cast(expression)); try { - return new CalculationNode(expr, out); + auto en = new CalculationNode(expr, out); + + addNode(reinterpret_cast(en)); + return en; } catch (...) { // prevent memleak @@ -167,27 +177,27 @@ ExecutionNode* ExecutionPlan::fromNodeFor (Ast const* ast, auto v = static_cast(variable->getData()); TRI_ASSERT(v != nullptr); - ExecutionNode* plan = nullptr; + ExecutionNode* en = nullptr; // peek at second operand if (expression->type == NODE_TYPE_COLLECTION) { // second operand is a collection char const* collectionName = expression->getStringValue(); - plan = new EnumerateCollectionNode(ast->query()->vocbase(), std::string(collectionName), v); + en = addNode(new EnumerateCollectionNode(ast->query()->vocbase(), std::string(collectionName), v)); } else if (expression->type == NODE_TYPE_REFERENCE) { // second operand is already a variable auto inVariable = static_cast(variable->getData()); TRI_ASSERT(inVariable != nullptr); - plan = new EnumerateListNode(inVariable, v); + en = addNode(new EnumerateListNode(inVariable, v)); } else { // second operand is some misc. expression auto calc = createTemporaryCalculation(ast, expression); try { - plan = new EnumerateListNode(calc->outVariable(), v); - plan->addDependency(calc); + en = addNode(new EnumerateListNode(calc->outVariable(), v)); + en->addDependency(calc); } catch (...) { // prevent memleak @@ -195,9 +205,9 @@ ExecutionNode* ExecutionPlan::fromNodeFor (Ast const* ast, } } - TRI_ASSERT(plan != nullptr); + TRI_ASSERT(en != nullptr); - return addDependency(previous, plan); + return addDependency(previous, en); } //////////////////////////////////////////////////////////////////////////////// @@ -212,21 +222,21 @@ ExecutionNode* ExecutionPlan::fromNodeFilter (Ast const* ast, auto expression = node->getMember(0); - ExecutionNode* plan = nullptr; + ExecutionNode* en = nullptr; if (expression->type == NODE_TYPE_REFERENCE) { // operand is already a variable auto v = static_cast(expression->getData()); TRI_ASSERT(v != nullptr); - plan = new FilterNode(v); + en = addNode(new FilterNode(v)); } else { // operand is some misc expression auto calc = createTemporaryCalculation(ast, expression); try { - plan = new FilterNode(calc->outVariable()); - plan->addDependency(calc); + en = addNode(new FilterNode(calc->outVariable())); + en->addDependency(calc); } catch (...) { // prevent memleak @@ -234,7 +244,7 @@ ExecutionNode* ExecutionPlan::fromNodeFilter (Ast const* ast, } } - return addDependency(previous, plan); + return addDependency(previous, en); } //////////////////////////////////////////////////////////////////////////////// @@ -252,7 +262,7 @@ ExecutionNode* ExecutionPlan::fromNodeLet (Ast const* ast, auto v = static_cast(variable->getData()); - ExecutionNode* plan = nullptr; + ExecutionNode* en = nullptr; if (expression->type == NODE_TYPE_SUBQUERY) { // TODO: node might be a subquery. this is currently NOT handled @@ -263,7 +273,7 @@ ExecutionNode* ExecutionPlan::fromNodeLet (Ast const* ast, auto expr = new Expression(ast->query()->executor(), const_cast(expression)); try { - plan = new CalculationNode(expr, v); + en = addNode(new CalculationNode(expr, v)); } catch (...) { // prevent memleak @@ -272,7 +282,7 @@ ExecutionNode* ExecutionPlan::fromNodeLet (Ast const* ast, } } - return addDependency(previous, plan); + return addDependency(previous, en); } //////////////////////////////////////////////////////////////////////////////// @@ -331,9 +341,9 @@ ExecutionNode* ExecutionPlan::fromNodeSort (Ast const* ast, previous = (*it); } - auto plan = new SortNode(elements); + auto en = addNode(new SortNode(elements)); - return addDependency(previous, plan); + return addDependency(previous, en); } //////////////////////////////////////////////////////////////////////////////// @@ -370,9 +380,9 @@ ExecutionNode* ExecutionPlan::fromNodeLimit (Ast const* ast, TRI_ASSERT(offset->type == NODE_TYPE_VALUE); TRI_ASSERT(count->type == NODE_TYPE_VALUE); - auto plan = new LimitNode(static_cast(offset->getIntValue()), static_cast(count->getIntValue())); + auto en = addNode(new LimitNode(static_cast(offset->getIntValue()), static_cast(count->getIntValue()))); - return addDependency(previous, plan); + return addDependency(previous, en); } //////////////////////////////////////////////////////////////////////////////// @@ -387,21 +397,21 @@ ExecutionNode* ExecutionPlan::fromNodeReturn (Ast const* ast, auto expression = node->getMember(0); - ExecutionNode* plan = nullptr; + ExecutionNode* en = nullptr; if (expression->type == NODE_TYPE_REFERENCE) { // operand is already a variable auto v = static_cast(expression->getData()); TRI_ASSERT(v != nullptr); - plan = new RootNode(v); + en = addNode(new ReturnNode(v)); } else { // operand is some misc expression auto calc = createTemporaryCalculation(ast, expression); try { - plan = new RootNode(calc->outVariable()); - plan->addDependency(calc); + en = addNode(new ReturnNode(calc->outVariable())); + en->addDependency(calc); } catch (...) { // prevent memleak @@ -409,7 +419,7 @@ ExecutionNode* ExecutionPlan::fromNodeReturn (Ast const* ast, } } - return addDependency(previous, plan); + return addDependency(previous, en); } //////////////////////////////////////////////////////////////////////////////// @@ -480,7 +490,7 @@ ExecutionNode* ExecutionPlan::fromNode (Ast const* ast, AstNode const* node) { TRI_ASSERT(node != nullptr); - ExecutionNode* plan = new SingletonNode(); + ExecutionNode* en = addNode(new SingletonNode()); try { size_t const n = node->numMembers(); @@ -494,78 +504,78 @@ ExecutionNode* ExecutionPlan::fromNode (Ast const* ast, switch (member->type) { case NODE_TYPE_FOR: { - plan = fromNodeFor(ast, plan, member); + en = fromNodeFor(ast, en, member); break; } case NODE_TYPE_FILTER: { - plan = fromNodeFilter(ast, plan, member); + en = fromNodeFilter(ast, en, member); break; } case NODE_TYPE_LET: { - plan = fromNodeLet(ast, plan, member); + en = fromNodeLet(ast, en, member); break; } case NODE_TYPE_SORT: { - plan = fromNodeSort(ast, plan, member); + en = fromNodeSort(ast, en, member); break; } case NODE_TYPE_COLLECT: { - plan = fromNodeCollect(ast, plan, member); + en = fromNodeCollect(ast, en, member); break; } case NODE_TYPE_LIMIT: { - plan = fromNodeLimit(ast, plan, member); + en = fromNodeLimit(ast, en, member); break; } case NODE_TYPE_RETURN: { - plan = fromNodeReturn(ast, plan, member); + en = fromNodeReturn(ast, en, member); break; } case NODE_TYPE_REMOVE: { - plan = fromNodeRemove(ast, plan, member); + en = fromNodeRemove(ast, en, member); break; } case NODE_TYPE_INSERT: { - plan = fromNodeInsert(ast, plan, member); + en = fromNodeInsert(ast, en, member); break; } case NODE_TYPE_UPDATE: { - plan = fromNodeUpdate(ast, plan, member); + en = fromNodeUpdate(ast, en, member); break; } case NODE_TYPE_REPLACE: { - plan = fromNodeReplace(ast, plan, member); + en = fromNodeReplace(ast, en, member); break; } default: { // node type not implemented - plan = nullptr; + en = nullptr; break; } } - if (plan == nullptr) { + if (en == nullptr) { THROW_ARANGO_EXCEPTION(TRI_ERROR_INTERNAL); } } - return plan; + return en; } catch (...) { // prevent memleak - if (plan != nullptr) { - delete plan; + if (en != nullptr) { + delete en; } throw; } diff --git a/arangod/Aql/ExecutionPlan.h b/arangod/Aql/ExecutionPlan.h index 177952f460..d97289366e 100644 --- a/arangod/Aql/ExecutionPlan.h +++ b/arangod/Aql/ExecutionPlan.h @@ -94,10 +94,10 @@ namespace triagens { private: //////////////////////////////////////////////////////////////////////////////// -/// @brief add a node to the plan +/// @brief add a node to the plan, will delete node if addition fails //////////////////////////////////////////////////////////////////////////////// - void addNode (ExecutionNode*); + ExecutionNode* addNode (ExecutionNode*); //////////////////////////////////////////////////////////////////////////////// /// @brief creates a calculation node for an arbitrary expression