diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index f86a8093f4..24242fcb86 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -1675,14 +1675,14 @@ static RangeInfoMapVec* BuildRangeInfo (ExecutionPlan* plan, if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { FindVarAndAttr(plan, rhs, enumCollVar, attr); - if (enumCollVar != nullptr) { - foundSomething = true; + if (enumCollVar != nullptr) { 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: + foundSomething = true; rim->insert(enumCollVar->name, attr.substr(0, attr.size() - 1), @@ -1698,13 +1698,15 @@ static RangeInfoMapVec* BuildRangeInfo (ExecutionPlan* plan, 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 (enumCollVar != nullptr) { + 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: + foundSomething = true; + rim->insert(enumCollVar->name, attr.substr(0, attr.size() - 1), RangeInfoBound(rhs, true), @@ -1830,13 +1832,13 @@ static RangeInfoMapVec* BuildRangeInfo (ExecutionPlan* plan, 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: + foundSomething = true; + if (rhs->type == NODE_TYPE_ARRAY) { size_t const n = rhs->numMembers(); rimv->reserve(n); @@ -1890,7 +1892,7 @@ static RangeInfoMapVec* BuildRangeInfo (ExecutionPlan* plan, auto lhs = BuildRangeInfo(plan, node->getMember(0), enumCollVar, attr, lhsMustNotUseRange, node->type); auto rhs = BuildRangeInfo(plan, node->getMember(1), enumCollVar, attr, rhsMustNotUseRange, node->type); - + if (lhsMustNotUseRange || rhsMustNotUseRange) { mustNotUseRanges = true; } @@ -2007,7 +2009,7 @@ class FilterToEnumCollFinder : public WalkerWorker { case EN::SUBQUERY: break; case EN::FILTER: { - std::vector inVar = en->getVariablesUsedHere(); + std::vector&& inVar = en->getVariablesUsedHere(); TRI_ASSERT(inVar.size() == 1); _varIds.emplace(inVar[0]->id); break; @@ -2042,6 +2044,7 @@ class FilterToEnumCollFinder : public WalkerWorker { case EN::ENUMERATE_COLLECTION: { auto node = static_cast(en); auto var = node->getVariablesSetHere()[0]; // should only be 1 + if (_rangeInfoMapVec == nullptr) { break; } @@ -2054,7 +2057,7 @@ class FilterToEnumCollFinder : public WalkerWorker { std::unordered_set varsDefined = node->getVarsValid(); // Take out the variable we define only here, because we are // not allowed to use it in a variable bound expression: - std::vector varsSetHere = node->getVariablesSetHere(); + std::vector&& varsSetHere = node->getVariablesSetHere(); for (auto const& v : varsSetHere) { varsDefined.erase(v); } @@ -2089,7 +2092,7 @@ class FilterToEnumCollFinder : public WalkerWorker { } map = _rangeInfoMapVec->find(var->name, ++pos); } - while (map !=nullptr); + while (map != nullptr); // Now remove empty conditions: _rangeInfoMapVec->eraseEmptyOrUndefined(var->name); @@ -2203,10 +2206,10 @@ class FilterToEnumCollFinder : public WalkerWorker { // each valid orCondition should match every field of the given index for (size_t k = 0; k < validPos.size() && ! indexOrCondition.empty(); k++) { auto const map = _rangeInfoMapVec->find(var->name, validPos[k]); - + for (size_t j = 0; j < idx->fields.size(); j++) { auto range = map->find(idx->fields[j]); - + if (range == map->end() || ! range->second.is1ValueRangeInfo()) { indexOrCondition.clear(); // not usable break; diff --git a/arangod/Aql/RangeInfo.cpp b/arangod/Aql/RangeInfo.cpp index ef35ba28d2..d478b572d8 100644 --- a/arangod/Aql/RangeInfo.cpp +++ b/arangod/Aql/RangeInfo.cpp @@ -191,13 +191,13 @@ triagens::basics::Json RangeInfo::toJson () const { ("highConst", _highConst.toJson()); triagens::basics::Json lowList(triagens::basics::Json::Array, _lows.size()); - for (auto l : _lows) { + for (auto const& l : _lows) { lowList(l.toJson()); } item("lows", lowList); triagens::basics::Json highList(triagens::basics::Json::Array, _highs.size()); - for (auto h : _highs) { + for (auto const& h : _highs) { highList(h.toJson()); } item("highs", highList); @@ -239,7 +239,7 @@ void RangeInfo::fuse (RangeInfo const& that) { // First sort out the constant low bounds: _lowConst.andCombineLowerBounds(that._lowConst); - + // Simply append the variable ones: for (auto const& l : that._lows) { _lows.emplace_back(l); @@ -346,6 +346,7 @@ void RangeInfoMap::erase (RangeInfo* ri) { if (it != nullptr) { auto it2 = it->find(ri->_attr); + if (it2 != (*it).end()) { it->erase(it2); } @@ -404,7 +405,7 @@ bool RangeInfoMap::isValid (std::string const& var) const { auto map = find(var); if (map != nullptr) { - for (auto x : *map) { + for (auto const& x : *map) { if (! x.second.isValid()) { return false; } @@ -466,7 +467,7 @@ RangeInfoMapVec::RangeInfoMapVec (RangeInfoMap* rim) //////////////////////////////////////////////////////////////////////////////// RangeInfoMapVec::~RangeInfoMapVec () { - for (auto x: _rangeInfoMapVec) { + for (auto& x: _rangeInfoMapVec) { delete x; } _rangeInfoMapVec.clear(); @@ -494,7 +495,7 @@ void RangeInfoMapVec::emplace_back (RangeInfoMap* rim) { //////////////////////////////////////////////////////////////////////////////// void RangeInfoMapVec::eraseEmptyOrUndefined (std::string const& var) { - for (RangeInfoMap* x: _rangeInfoMapVec) { + for (auto& x: _rangeInfoMapVec) { x->eraseEmptyOrUndefined(var); } } @@ -566,11 +567,12 @@ std::unordered_set RangeInfoMapVec::attributes (std::string const& //////////////////////////////////////////////////////////////////////////////// void RangeInfoMapVec::differenceRangeInfo (RangeInfo& newRi) { - for (auto rim: _rangeInfoMapVec) { + for (auto const& rim: _rangeInfoMapVec) { RangeInfo* oldRi = rim->find(newRi._var, newRi._attr); if (oldRi != nullptr) { differenceRangeInfos(*oldRi, newRi); + if (! newRi.isValid() || (newRi._lowConst.bound().isEmpty() && newRi._highConst.bound().isEmpty())) { break; @@ -614,8 +616,8 @@ RangeInfoMapVec* triagens::aql::orCombineRangeInfoMapVecs (RangeInfoMapVec* lhs, for (size_t i = 0; i < rhs->size(); i++) { std::unique_ptr rim(new RangeInfoMap()); - for (auto x: (*rhs)[i]->_ranges) { - for (auto y: x.second) { + for (auto const& x: (*rhs)[i]->_ranges) { + for (auto const& y: x.second) { RangeInfo ri = y.second.clone(); rim->insert(ri); } diff --git a/js/server/tests/aql-optimizer-indexes.js b/js/server/tests/aql-optimizer-indexes.js index 64642de38a..51107378c1 100644 --- a/js/server/tests/aql-optimizer-indexes.js +++ b/js/server/tests/aql-optimizer-indexes.js @@ -806,6 +806,98 @@ function optimizerIndexesTestSuite () { assertTrue(results.stats.scannedFull > 0); }, +//////////////////////////////////////////////////////////////////////////////// +/// @brief test index usage +//////////////////////////////////////////////////////////////////////////////// + + testIndexOrNoIndexComplexConditionEq : function () { + c.ensureSkiplist("value2"); + AQL_EXECUTE("FOR i IN " + c.name() + " UPDATE i WITH { value2: [ i.value ] } IN " + c.name()); + + var query = "FOR i IN " + c.name() + " FILTER i.value == -1 || [ i.value ] == i.value2 RETURN i.value2"; + + var plan = AQL_EXPLAIN(query).plan; + var nodeTypes = plan.nodes.map(function(node) { + return node.type; + }); + + assertEqual(-1, nodeTypes.indexOf("IndexRangeNode"), query); + + var results = AQL_EXECUTE(query); + assertEqual(2000, results.json.length); + assertEqual(0, results.stats.scannedIndex); + assertTrue(results.stats.scannedFull > 0); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test index usage +//////////////////////////////////////////////////////////////////////////////// + + testIndexOrNoIndexComplexConditionIn : function () { + c.ensureSkiplist("value2"); + AQL_EXECUTE("FOR i IN " + c.name() + " UPDATE i WITH { value2: [ i.value ] } IN " + c.name()); + + var query = "FOR i IN " + c.name() + " FILTER i.value == -1 || i.value IN i.value2 RETURN i.value2"; + + var plan = AQL_EXPLAIN(query).plan; + var nodeTypes = plan.nodes.map(function(node) { + return node.type; + }); + + assertEqual(-1, nodeTypes.indexOf("IndexRangeNode"), query); + + var results = AQL_EXECUTE(query); + assertEqual(2000, results.json.length); + assertEqual(0, results.stats.scannedIndex); + assertTrue(results.stats.scannedFull > 0); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test index usage +//////////////////////////////////////////////////////////////////////////////// + + testIndexOrNoIndexComplexConditionLt : function () { + c.ensureSkiplist("value2"); + AQL_EXECUTE("FOR i IN " + c.name() + " UPDATE i WITH { value2: i.value - 1 } IN " + c.name()); + + var query = "FOR i IN " + c.name() + " FILTER i.value == -1 || i.value2 < i.value RETURN i.value2"; + + var plan = AQL_EXPLAIN(query).plan; + var nodeTypes = plan.nodes.map(function(node) { + return node.type; + }); + + assertEqual(-1, nodeTypes.indexOf("IndexRangeNode"), query); + + var results = AQL_EXECUTE(query); + assertEqual(2000, results.json.length); + assertEqual(0, results.stats.scannedIndex); + assertTrue(results.stats.scannedFull > 0); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test index usage +//////////////////////////////////////////////////////////////////////////////// + + testIndexOrNoIndexComplexConditionGt : function () { + c.ensureSkiplist("value2"); + AQL_EXECUTE("FOR i IN " + c.name() + " UPDATE i WITH { value2: i.value + 1 } IN " + c.name()); + + var query = "FOR i IN " + c.name() + " FILTER i.value == -1 || i.value2 > i.value RETURN i.value2"; + + var plan = AQL_EXPLAIN(query).plan; + var nodeTypes = plan.nodes.map(function(node) { + return node.type; + }); + + assertEqual(-1, nodeTypes.indexOf("IndexRangeNode"), query); + + var results = AQL_EXECUTE(query); + assertEqual(2000, results.json.length); + assertEqual(0, results.stats.scannedIndex); + assertTrue(results.stats.scannedFull > 0); + }, + //////////////////////////////////////////////////////////////////////////////// /// @brief test index usage ////////////////////////////////////////////////////////////////////////////////