From 14e82da2b03fe308de98dbdc198f76e17166abd9 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Mon, 19 Jan 2015 22:25:16 +0100 Subject: [PATCH] fixed leak --- arangod/Aql/AstNode.cpp | 2 +- arangod/Aql/ExecutionBlock.cpp | 6 +- arangod/Aql/OptimizerRules.cpp | 577 +++++++++++++++++---------------- arangod/Aql/RangeInfo.cpp | 16 +- arangod/Aql/RangeInfo.h | 11 +- 5 files changed, 322 insertions(+), 290 deletions(-) diff --git a/arangod/Aql/AstNode.cpp b/arangod/Aql/AstNode.cpp index ec87989662..99f91cc992 100644 --- a/arangod/Aql/AstNode.cpp +++ b/arangod/Aql/AstNode.cpp @@ -526,7 +526,7 @@ AstNode::AstNode (Ast* ast, case NODE_TYPE_RANGE: case NODE_TYPE_NOP: break; - } + } Json subNodes = json.get("subNodes"); if (subNodes.isArray()) { diff --git a/arangod/Aql/ExecutionBlock.cpp b/arangod/Aql/ExecutionBlock.cpp index dd646195e9..e1ae4464ad 100644 --- a/arangod/Aql/ExecutionBlock.cpp +++ b/arangod/Aql/ExecutionBlock.cpp @@ -5504,9 +5504,9 @@ int RemoteBlock::initializeCursor (AqlItemBlock* items, size_t pos) { } else { body("pos", Json(static_cast(pos))) - ("items", items->toJson(_engine->getQuery()->trx())) - ("exhausted", Json(false)) - ("error", Json(false)); + ("items", items->toJson(_engine->getQuery()->trx())) + ("exhausted", Json(false)) + ("error", Json(false)); } std::string bodyString(body.toString()); diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 02fb56f3d6..3a0c5b9d60 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -1160,6 +1160,296 @@ int triagens::aql::removeUnnecessaryCalculationsRule (Optimizer* opt, return TRI_ERROR_NO_ERROR; } +////////////////////////////////////////////////////////////////////////////////////////////////// +/// @brief helper function to find variable and attribute names from a node (if any) +////////////////////////////////////////////////////////////////////////////////////////////////// + +static void FindVarAndAttr (ExecutionPlan const* plan, + AstNode const* node, + Variable const*& enumCollVar, + std::string& attr) { + if (node->type == NODE_TYPE_REFERENCE) { + auto x = static_cast(node->getData()); + auto setter = plan->getVarSetBy(x->id); + if (setter != nullptr && + setter->getType() == EN::ENUMERATE_COLLECTION) { + enumCollVar = x; + } + return; + } + + if (node->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + FindVarAndAttr(plan, node->getMember(0), enumCollVar, attr); + + if (enumCollVar != nullptr) { + char const* attributeName = node->getStringValue(); + attr.append(attributeName); + attr.push_back('.'); + } + return; + } + + attr.clear(); + enumCollVar = nullptr; + return; +} + +//////////////////////////////////////////////////////////////////////////////// +/// @brief builds a range info from the expression node +//////////////////////////////////////////////////////////////////////////////// + +static RangeInfoMapVec* BuildRangeInfo (ExecutionPlan* plan, + AstNode const* node, + Variable const*& enumCollVar, + std::string& attr, + bool& mustNotUseRanges, + AstNodeType combineType = NODE_TYPE_OPERATOR_BINARY_AND) { + TRI_ASSERT(combineType == NODE_TYPE_OPERATOR_BINARY_AND || + combineType == NODE_TYPE_OPERATOR_BINARY_OR); + + bool foundSomething = false; + + + if (node->type == NODE_TYPE_OPERATOR_BINARY_EQ) { + auto lhs = node->getMember(0); + auto rhs = node->getMember(1); + std::unique_ptr rim(new RangeInfoMap()); + + if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + FindVarAndAttr(plan, rhs, enumCollVar, attr); + if (enumCollVar != nullptr) { + foundSomething = true; + + std::unordered_set varsUsed + = Ast::getReferencedVariables(lhs); + if (varsUsed.find(const_cast(enumCollVar)) + == varsUsed.end()) { + // Found a multiple attribute access of a variable and an + // expression which does not involve that variable: + + rim->insert(enumCollVar->name, + attr.substr(0, attr.size() - 1), + RangeInfoBound(lhs, true), + RangeInfoBound(lhs, true), + true); + + enumCollVar = nullptr; + attr.clear(); + } + } + } + + if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + FindVarAndAttr(plan, lhs, enumCollVar, attr); + if (enumCollVar != nullptr) { + foundSomething = true; + + std::unordered_set varsUsed + = Ast::getReferencedVariables(rhs); + if (varsUsed.find(const_cast(enumCollVar)) + == varsUsed.end()) { + // Found a multiple attribute access of a variable and an + // expression which does not involve that variable: + rim->insert(enumCollVar->name, + attr.substr(0, attr.size() - 1), + RangeInfoBound(rhs, true), + RangeInfoBound(rhs, true), + true); + enumCollVar = nullptr; + attr.clear(); + } + } + } + + if (combineType == NODE_TYPE_OPERATOR_BINARY_OR && ! foundSomething) { + // disable the use of the range because we may have found something like this, + // which makes using an index for a.x invalid: + // a.x == 1 || RAND() > 0 + mustNotUseRanges = true; + } + + return new RangeInfoMapVec(rim.release()); + } + + if (node->type == NODE_TYPE_OPERATOR_BINARY_LT || + node->type == NODE_TYPE_OPERATOR_BINARY_GT || + node->type == NODE_TYPE_OPERATOR_BINARY_LE || + node->type == NODE_TYPE_OPERATOR_BINARY_GE) { + + std::unique_ptr rim(new RangeInfoMap()); + bool include = (node->type == NODE_TYPE_OPERATOR_BINARY_LE || + node->type == NODE_TYPE_OPERATOR_BINARY_GE); + + auto lhs = node->getMember(0); + auto rhs = node->getMember(1); + + if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + // Attribute access on the right: + // First find out whether there is a multiple attribute access + // of a variable on the right: + FindVarAndAttr(plan, rhs, enumCollVar, attr); + + if (enumCollVar != nullptr) { + foundSomething = true; + RangeInfoBound low; + RangeInfoBound high; + + // Constant value on the left, so insert a constant condition: + if (node->type == NODE_TYPE_OPERATOR_BINARY_GE || + node->type == NODE_TYPE_OPERATOR_BINARY_GT) { + high.assign(lhs, include); + } + else { + low.assign(lhs, include); + } + + rim->insert(enumCollVar->name, + attr.substr(0, attr.size() - 1), + low, + high, + false); + + enumCollVar = nullptr; + attr.clear(); + } + } + + if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + // Attribute access on the left: + // First find out whether there is a multiple attribute access + // of a variable on the left: + FindVarAndAttr(plan, lhs, enumCollVar, attr); + + if (enumCollVar != nullptr) { + foundSomething = true; + RangeInfoBound low; + RangeInfoBound high; + + // Constant value on the right, so insert a constant condition: + if (node->type == NODE_TYPE_OPERATOR_BINARY_GE || + node->type == NODE_TYPE_OPERATOR_BINARY_GT) { + low.assign(rhs, include); + } + else { + high.assign(rhs, include); + } + + rim->insert(enumCollVar->name, + attr.substr(0, attr.size() - 1), + low, + high, + false); + + enumCollVar = nullptr; + attr.clear(); + } + } + + if (combineType == NODE_TYPE_OPERATOR_BINARY_OR && ! foundSomething) { + // disable the use of the range because we may have found something like this, + // which makes using an index for a.x invalid: + // a.x == 1 || RAND() > 0 + mustNotUseRanges = true; + } + + return new RangeInfoMapVec(rim.release()); + } + + if (node->type == NODE_TYPE_OPERATOR_BINARY_AND) { + auto lhs = BuildRangeInfo(plan, node->getMember(0), enumCollVar, attr, mustNotUseRanges, node->type); + auto rhs = BuildRangeInfo(plan, node->getMember(1), enumCollVar, attr, mustNotUseRanges, node->type); + + if ((lhs == nullptr || lhs->empty()) && rhs != nullptr) { + delete lhs; + return rhs; + } + else if (lhs != nullptr && (rhs == nullptr || rhs->empty())) { + delete rhs; + return lhs; + } + + // distribute AND into OR + return andCombineRangeInfoMapVecs(lhs, rhs); + } + + if (node->type == NODE_TYPE_OPERATOR_BINARY_IN) { + auto lhs = node->getMember(0); // enumCollVar + auto rhs = node->getMember(1); // value + + std::unique_ptr rimv(new RangeInfoMapVec()); + + if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + FindVarAndAttr(plan, lhs, enumCollVar, attr); + + if (enumCollVar != nullptr) { + foundSomething = true; + + std::unordered_set varsUsed + = Ast::getReferencedVariables(rhs); + if (varsUsed.find(const_cast(enumCollVar)) + == varsUsed.end()) { + // Found a multiple attribute access of a variable and an + // expression which does not involve that variable: + if (rhs->type == NODE_TYPE_ARRAY) { + for (size_t i = 0; i < rhs->numMembers(); i++) { + RangeInfo ri(enumCollVar->name, + attr.substr(0, attr.size() - 1), + RangeInfoBound(rhs->getMember(i), true), + RangeInfoBound(rhs->getMember(i), true), + true); + rimv->differenceRangeInfo(ri); + if (ri.isValid()) { + rimv->emplace_back(new RangeInfoMap(ri)); + } + } + } + else { + RangeInfo ri(enumCollVar->name, + attr.substr(0, attr.size() - 1), + RangeInfoBound(rhs, true), + RangeInfoBound(rhs, true), + true); + rimv->differenceRangeInfo(ri); + if (ri.isValid()) { + rimv->emplace_back(new RangeInfoMap(ri)); + } + } + enumCollVar = nullptr; + attr.clear(); + } + } + } + + if (combineType == NODE_TYPE_OPERATOR_BINARY_OR && ! foundSomething) { + // disable the use of the range because we may have found something like this, + // which makes using an index for a.x invalid: + // a.x == 1 || RAND() > 0 + mustNotUseRanges = true; + } + + return rimv.release(); + } + + if (node->type == NODE_TYPE_OPERATOR_BINARY_OR) { + return orCombineRangeInfoMapVecs( + BuildRangeInfo(plan, node->getMember(0), enumCollVar, attr, mustNotUseRanges, node->type), + BuildRangeInfo(plan, node->getMember(1), enumCollVar, attr, mustNotUseRanges, node->type) + ); + } + + if (combineType == NODE_TYPE_OPERATOR_BINARY_AND) { + attr.clear(); + enumCollVar = nullptr; + return nullptr; + } + + // default case + mustNotUseRanges = true; + attr.clear(); + enumCollVar = nullptr; + return nullptr; +} + //////////////////////////////////////////////////////////////////////////////// /// @brief prefer IndexRange nodes over EnumerateCollection nodes //////////////////////////////////////////////////////////////////////////////// @@ -1221,13 +1511,18 @@ class FilterToEnumCollFinder : public WalkerWorker { auto node = static_cast(en); std::string attr; Variable const* enumCollVar = nullptr; + auto expression = node->expression()->node(); // there is an implicit AND between FILTER statements if (_rangeInfoMapVec == nullptr) { - _rangeInfoMapVec = buildRangeInfo(node->expression()->node(), enumCollVar, attr, NODE_TYPE_OPERATOR_BINARY_AND); + // don't yet have anything to AND-combine + _rangeInfoMapVec = BuildRangeInfo(_plan, expression, enumCollVar, attr, _mustNotUseRanges); } else { - _rangeInfoMapVec = andCombineRangeInfoMapVecs(_rangeInfoMapVec, - buildRangeInfo(node->expression()->node(), enumCollVar, attr, NODE_TYPE_OPERATOR_BINARY_AND)); + // AND-combine with previous ranges + _rangeInfoMapVec = andCombineRangeInfoMapVecs( + _rangeInfoMapVec, + BuildRangeInfo(_plan, expression, enumCollVar, attr, _mustNotUseRanges) + ); } if (_rangeInfoMapVec != nullptr && _mustNotUseRanges) { @@ -1533,282 +1828,6 @@ class FilterToEnumCollFinder : public WalkerWorker { return false; } -////////////////////////////////////////////////////////////////////////////////////////////////// - - void findVarAndAttr (AstNode const* node, - Variable const*& enumCollVar, - std::string& attr) { - if (node->type == NODE_TYPE_REFERENCE) { - auto x = static_cast(node->getData()); - auto setter = _plan->getVarSetBy(x->id); - if (setter != nullptr && - setter->getType() == EN::ENUMERATE_COLLECTION) { - enumCollVar = x; - } - return; - } - - if (node->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - findVarAndAttr(node->getMember(0), enumCollVar, attr); - - if (enumCollVar != nullptr) { - char const* attributeName = node->getStringValue(); - attr.append(attributeName); - attr.push_back('.'); - } - return; - } - - attr.clear(); - enumCollVar = nullptr; - return; - } - - RangeInfoMapVec* buildRangeInfo (AstNode const* node, - Variable const*& enumCollVar, - std::string& attr, - AstNodeType previousNode) { - bool foundSomething = false; - - if (node->type == NODE_TYPE_OPERATOR_BINARY_EQ) { - auto lhs = node->getMember(0); - auto rhs = node->getMember(1); - std::unique_ptr rim(new RangeInfoMap()); - - if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - findVarAndAttr(rhs, enumCollVar, attr); - if (enumCollVar != nullptr) { - foundSomething = true; - - std::unordered_set varsUsed - = Ast::getReferencedVariables(lhs); - if (varsUsed.find(const_cast(enumCollVar)) - == varsUsed.end()) { - // Found a multiple attribute access of a variable and an - // expression which does not involve that variable: - - rim->insert(enumCollVar->name, - attr.substr(0, attr.size() - 1), - RangeInfoBound(lhs, true), - RangeInfoBound(lhs, true), - true); - - enumCollVar = nullptr; - attr.clear(); - } - } - } - - if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - findVarAndAttr(lhs, enumCollVar, attr); - if (enumCollVar != nullptr) { - foundSomething = true; - - std::unordered_set varsUsed - = Ast::getReferencedVariables(rhs); - if (varsUsed.find(const_cast(enumCollVar)) - == varsUsed.end()) { - // Found a multiple attribute access of a variable and an - // expression which does not involve that variable: - rim->insert(enumCollVar->name, - attr.substr(0, attr.size() - 1), - RangeInfoBound(rhs, true), - RangeInfoBound(rhs, true), - true); - enumCollVar = nullptr; - attr.clear(); - } - } - } - - if (previousNode == NODE_TYPE_OPERATOR_BINARY_OR && ! foundSomething) { - // disable the use of the range because we may have found something like this, - // which makes using an index for a.x invalid: - // a.x == 1 || RAND() > 0 - _mustNotUseRanges = true; - } - - return new RangeInfoMapVec(rim.release()); - } - - if (node->type == NODE_TYPE_OPERATOR_BINARY_LT || - node->type == NODE_TYPE_OPERATOR_BINARY_GT || - node->type == NODE_TYPE_OPERATOR_BINARY_LE || - node->type == NODE_TYPE_OPERATOR_BINARY_GE) { - - std::unique_ptr rim(new RangeInfoMap()); - bool include = (node->type == NODE_TYPE_OPERATOR_BINARY_LE || - node->type == NODE_TYPE_OPERATOR_BINARY_GE); - - auto lhs = node->getMember(0); - auto rhs = node->getMember(1); - - if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - // Attribute access on the right: - // First find out whether there is a multiple attribute access - // of a variable on the right: - findVarAndAttr(rhs, enumCollVar, attr); - - if (enumCollVar != nullptr) { - foundSomething = true; - RangeInfoBound low; - RangeInfoBound high; - - // Constant value on the left, so insert a constant condition: - if (node->type == NODE_TYPE_OPERATOR_BINARY_GE || - node->type == NODE_TYPE_OPERATOR_BINARY_GT) { - high.assign(lhs, include); - } - else { - low.assign(lhs, include); - } - - rim->insert(enumCollVar->name, - attr.substr(0, attr.size() - 1), - low, - high, - false); - - enumCollVar = nullptr; - attr.clear(); - } - } - - if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - // Attribute access on the left: - // First find out whether there is a multiple attribute access - // of a variable on the left: - findVarAndAttr(lhs, enumCollVar, attr); - - if (enumCollVar != nullptr) { - foundSomething = true; - RangeInfoBound low; - RangeInfoBound high; - - // Constant value on the right, so insert a constant condition: - if (node->type == NODE_TYPE_OPERATOR_BINARY_GE || - node->type == NODE_TYPE_OPERATOR_BINARY_GT) { - low.assign(rhs, include); - } - else { - high.assign(rhs, include); - } - - rim->insert(enumCollVar->name, - attr.substr(0, attr.size() - 1), - low, - high, - false); - - enumCollVar = nullptr; - attr.clear(); - } - } - - if (previousNode == NODE_TYPE_OPERATOR_BINARY_OR && ! foundSomething) { - // disable the use of the range because we may have found something like this, - // which makes using an index for a.x invalid: - // a.x == 1 || RAND() > 0 - _mustNotUseRanges = true; - } - - return new RangeInfoMapVec(rim.release()); - } - - if (node->type == NODE_TYPE_OPERATOR_BINARY_AND) { - auto lhs = buildRangeInfo(node->getMember(0), enumCollVar, attr, node->type); - auto rhs = buildRangeInfo(node->getMember(1), enumCollVar, attr, node->type); - - if ((lhs == nullptr || lhs->empty()) && rhs != nullptr) { - delete lhs; - return rhs; - } - else if (lhs != nullptr && (rhs == nullptr || rhs->empty())) { - delete rhs; - return lhs; - } - - // distribute AND into OR - return andCombineRangeInfoMapVecs(buildRangeInfo(node->getMember(0), enumCollVar, attr, node->type), - buildRangeInfo(node->getMember(1), enumCollVar, attr, node->type)); - } - - if (node->type == NODE_TYPE_OPERATOR_BINARY_IN) { - auto lhs = node->getMember(0); // enumCollVar - auto rhs = node->getMember(1); // value - - std::unique_ptr rimv(new RangeInfoMapVec()); - - if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - findVarAndAttr(lhs, enumCollVar, attr); - - if (enumCollVar != nullptr) { - foundSomething = true; - - std::unordered_set varsUsed - = Ast::getReferencedVariables(rhs); - if (varsUsed.find(const_cast(enumCollVar)) - == varsUsed.end()) { - // Found a multiple attribute access of a variable and an - // expression which does not involve that variable: - if (rhs->type == NODE_TYPE_ARRAY) { - for (size_t i = 0; i < rhs->numMembers(); i++) { - RangeInfo ri(enumCollVar->name, - attr.substr(0, attr.size() - 1), - RangeInfoBound(rhs->getMember(i), true), - RangeInfoBound(rhs->getMember(i), true), - true); - rimv->differenceRangeInfo(ri); - if (ri.isValid()) { - rimv->emplace_back(new RangeInfoMap(ri)); - } - } - } - else { - RangeInfo ri(enumCollVar->name, - attr.substr(0, attr.size() - 1), - RangeInfoBound(rhs, true), - RangeInfoBound(rhs, true), - true); - rimv->differenceRangeInfo(ri); - if (ri.isValid()) { - rimv->emplace_back(new RangeInfoMap(ri)); - } - } - enumCollVar = nullptr; - attr.clear(); - } - } - } - - if (previousNode == NODE_TYPE_OPERATOR_BINARY_OR && ! foundSomething) { - // disable the use of the range because we may have found something like this, - // which makes using an index for a.x invalid: - // a.x == 1 || RAND() > 0 - _mustNotUseRanges = true; - } - - return rimv.release(); - } - - if (node->type == NODE_TYPE_OPERATOR_BINARY_OR) { - return orCombineRangeInfoMapVecs(buildRangeInfo(node->getMember(0), enumCollVar, attr, node->type), - buildRangeInfo(node->getMember(1), enumCollVar, attr, node->type)); - } - - if (previousNode == NODE_TYPE_OPERATOR_BINARY_AND) { - attr.clear(); - enumCollVar = nullptr; - return nullptr; - } - - // default case - _mustNotUseRanges = true; - attr.clear(); - enumCollVar = nullptr; - return nullptr; - } - bool enterSubquery (ExecutionNode* super, ExecutionNode* sub) final { return false; } diff --git a/arangod/Aql/RangeInfo.cpp b/arangod/Aql/RangeInfo.cpp index 793dd9149f..267183d230 100644 --- a/arangod/Aql/RangeInfo.cpp +++ b/arangod/Aql/RangeInfo.cpp @@ -130,7 +130,10 @@ AstNode const* RangeInfoBound::getExpressionAst (Ast* ast) const { if (_isConstant) { return nullptr; } + + // TODO: check who is going to free this node... _expressionAst = new AstNode(ast, _bound); + return _expressionAst; } @@ -408,6 +411,7 @@ bool RangeInfoMap::isValid (std::string const& var) { void RangeInfoMap::attributes (std::unordered_set& set, std::string const& var) { std::unordered_map* map = find(var); + if (map != nullptr) { for(auto x: *map) { set.insert(x.first); @@ -415,11 +419,17 @@ void RangeInfoMap::attributes (std::unordered_set& set, } } -std::unordered_set RangeInfoMap::variables () { +//////////////////////////////////////////////////////////////////////////////// +/// @brief return the names of variables contained in the RangeInfoMap +//////////////////////////////////////////////////////////////////////////////// + +std::unordered_set RangeInfoMap::variables () const { std::unordered_set vars; + for(auto x: _ranges) { vars.insert(x.first); } + return vars; } @@ -432,8 +442,8 @@ std::unordered_set RangeInfoMap::variables () { /// RangeInfoMap containing a single RangeInfo. //////////////////////////////////////////////////////////////////////////////// -RangeInfoMapVec::RangeInfoMapVec (RangeInfoMap* rim) : - _rangeInfoMapVec() { +RangeInfoMapVec::RangeInfoMapVec (RangeInfoMap* rim) + : _rangeInfoMapVec() { _rangeInfoMapVec.emplace_back(rim); } diff --git a/arangod/Aql/RangeInfo.h b/arangod/Aql/RangeInfo.h index 4513884176..ef752b3972 100644 --- a/arangod/Aql/RangeInfo.h +++ b/arangod/Aql/RangeInfo.h @@ -587,7 +587,7 @@ namespace triagens { public: - RangeInfoMap (const RangeInfoMap& copy) = delete; + RangeInfoMap (RangeInfoMap const& copy) = delete; RangeInfoMap& operator= (RangeInfoMap const& copy) = delete; //////////////////////////////////////////////////////////////////////////////// @@ -742,9 +742,11 @@ namespace triagens { void attributes (std::unordered_set& set, std::string const& var); +//////////////////////////////////////////////////////////////////////////////// +/// @brief return the names of variables contained in the RangeInfoMap +//////////////////////////////////////////////////////////////////////////////// - //TODO write @brief - std::unordered_set variables(); + std::unordered_set variables() const; //////////////////////////////////////////////////////////////////////////////// /// @brief private data @@ -766,7 +768,7 @@ namespace triagens { public: - RangeInfoMapVec (const RangeInfoMapVec& copy) = delete; + RangeInfoMapVec (RangeInfoMapVec const& copy) = delete; RangeInfoMapVec& operator= (RangeInfoMapVec const& copy) = delete; //////////////////////////////////////////////////////////////////////////////// @@ -888,6 +890,7 @@ namespace triagens { //////////////////////////////////////////////////////////////////////////////// private: + std::vector _rangeInfoMapVec; };