diff --git a/arangod/Aql/AqlItemBlock.cpp b/arangod/Aql/AqlItemBlock.cpp index 5a148641c4..6f1568033d 100644 --- a/arangod/Aql/AqlItemBlock.cpp +++ b/arangod/Aql/AqlItemBlock.cpp @@ -231,7 +231,7 @@ void AqlItemBlock::shrink (size_t nrItems) { if (nrItems > _nrItems) { // cannot use shrink() to increase the size of the block - THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, "cannout use shrink() to increase block"); + THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, "cannot use shrink() to increase block"); } // erase all stored values in the region that we freed diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index db51169742..10e36fece5 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -1912,7 +1912,7 @@ struct SortToIndexNode final : public WalkerWorker { return true; } - if (! seen.empty() && triagens::basics::AttributeName::isIdentical(index->fields, seen)) { + if (! seen.empty() && triagens::basics::AttributeName::isIdentical(index->fields, seen, true)) { // different attributes return true; } diff --git a/arangod/Indexes/HashIndex.cpp b/arangod/Indexes/HashIndex.cpp index ad5bbd5bd7..cff8bb099f 100644 --- a/arangod/Indexes/HashIndex.cpp +++ b/arangod/Indexes/HashIndex.cpp @@ -856,7 +856,7 @@ IndexIterator* HashIndex::iteratorForCondition (IndexIteratorContext* context, size_t attributePosition = SIZE_MAX; for (size_t j = 0; j < _fields.size(); ++j) { - if (_fields[j] == paramPair.second) { + if (triagens::basics::AttributeName::isIdentical(_fields[j], paramPair.second, true)) { attributePosition = j; break; } diff --git a/arangod/Indexes/Index.cpp b/arangod/Indexes/Index.cpp index e575cccbf6..b43f7e1011 100644 --- a/arangod/Indexes/Index.cpp +++ b/arangod/Indexes/Index.cpp @@ -562,8 +562,9 @@ bool Index::canUseConditionPart (triagens::aql::AstNode const* access, } if (op->type == triagens::aql::NODE_TYPE_OPERATOR_BINARY_IN && - other->type == triagens::aql::NODE_TYPE_EXPANSION) { - // value IN a.b + (other->type == triagens::aql::NODE_TYPE_EXPANSION || + other->type == triagens::aql::NODE_TYPE_ATTRIBUTE_ACCESS)) { + // value IN a.b OR value IN a.b[*] if (! access->isConstant()) { return false; } @@ -576,7 +577,7 @@ bool Index::canUseConditionPart (triagens::aql::AstNode const* access, } else if (op->type == triagens::aql::NODE_TYPE_OPERATOR_BINARY_IN && access->type == triagens::aql::NODE_TYPE_EXPANSION) { - // value IN a.b + // value[*] IN a.b if (! other->isConstant()) { return false; } @@ -627,8 +628,10 @@ bool Index::canUseConditionPart (triagens::aql::AstNode const* access, // test if the reference variable is contained on both side of the expression std::unordered_set variables; if (op->type == triagens::aql::NODE_TYPE_OPERATOR_BINARY_IN && - other->type == triagens::aql::NODE_TYPE_EXPANSION) { - // value IN a.b + (other->type == triagens::aql::NODE_TYPE_EXPANSION || + other->type == triagens::aql::NODE_TYPE_ATTRIBUTE_ACCESS)) { + + // value IN a.b OR value IN a.b[*] triagens::aql::Ast::getReferencedVariables(access, variables); } else { diff --git a/arangod/Indexes/SimpleAttributeEqualityMatcher.cpp b/arangod/Indexes/SimpleAttributeEqualityMatcher.cpp index 96bfd596bf..a11aadc2e3 100644 --- a/arangod/Indexes/SimpleAttributeEqualityMatcher.cpp +++ b/arangod/Indexes/SimpleAttributeEqualityMatcher.cpp @@ -36,6 +36,7 @@ using namespace triagens::arango; +#include // ----------------------------------------------------------------------------- // --SECTION-- class SimpleAttributeEqualityMatcher // ----------------------------------------------------------------------------- @@ -120,10 +121,6 @@ bool SimpleAttributeEqualityMatcher::matchAll (triagens::arango::Index const* in if (accessFitsIndex(index, op->getMember(0), op->getMember(1), op, reference, false) || accessFitsIndex(index, op->getMember(1), op->getMember(0), op, reference, false)) { - if (_found.size() == _attributes.size()) { - // got enough attributes - break; - } } } else if (op->type == triagens::aql::NODE_TYPE_OPERATOR_BINARY_IN) { @@ -131,17 +128,18 @@ bool SimpleAttributeEqualityMatcher::matchAll (triagens::arango::Index const* in if (accessFitsIndex(index, op->getMember(0), op->getMember(1), op, reference, false)) { auto m = op->getMember(1); - if (m->type != triagens::aql::NODE_TYPE_EXPANSION && m->numMembers() > 1) { + + if (m->isArray() && m->numMembers() > 1) { // attr IN [ a, b, c ] => this will produce multiple items, so count them! values += m->numMembers() - 1; } - - if (_found.size() == _attributes.size()) { - // got enough attributes - break; - } } } + + if (_found.size() == _attributes.size()) { + // got enough attributes + break; + } } if (_found.size() == _attributes.size()) { @@ -346,22 +344,38 @@ bool SimpleAttributeEqualityMatcher::accessFitsIndex (triagens::arango::Index co return false; } - triagens::aql::AstNode const* what; - - if (op->type == triagens::aql::NODE_TYPE_OPERATOR_BINARY_IN && - other->type == triagens::aql::NODE_TYPE_EXPANSION) { - what = other; - } - else { - what = access; - } - + triagens::aql::AstNode const* what = access; std::pair> attributeData; - if (! what->isAttributeAccessForVariable(attributeData) || - attributeData.first != reference) { - // this access is not referencing this collection - return false; + if (op->type != triagens::aql::NODE_TYPE_OPERATOR_BINARY_IN) { + if (! what->isAttributeAccessForVariable(attributeData) || + attributeData.first != reference) { + // this access is not referencing this collection + return false; + } + if (triagens::basics::TRI_AttributeNamesHaveExpansion(attributeData.second)) { + // doc.value[*] IN 'value' + return false; + } + } + else { + // ok, we do have an IN here... check if it's something like 'value' IN doc.value[*] + bool canUse = false; + if (what->isAttributeAccessForVariable(attributeData) && + attributeData.first == reference && + ! triagens::basics::TRI_AttributeNamesHaveExpansion(attributeData.second)) { + // doc.value[*] IN 'value' + canUse = true; + } + if (! canUse) { + // check for doc.value[*] IN 'value' + what = other; + if (! what->isAttributeAccessForVariable(attributeData) || + attributeData.first != reference) { + // this access is not referencing this collection + return false; + } + } } std::vector const& fieldNames = attributeData.second; @@ -380,8 +394,15 @@ bool SimpleAttributeEqualityMatcher::accessFitsIndex (triagens::arango::Index co bool match = true; for (size_t j = 0; j < _attributes[i].size(); ++j) { if (_attributes[i][j] != fieldNames[j]) { - match = false; - break; + // special case: a[*] is identical to a, and a.b[*] is identical to a.b + // general rule: if index attribute is expanded and last part in index, then it can + // be used in a query without expansion operator + bool const isLast = (j == _attributes[i].size() - 1); + + if (! isLast || (! _attributes[i][j].shouldExpand) || _attributes[i][j].name != fieldNames[j].name) { + match = false; + break; + } } } diff --git a/arangod/Indexes/SkiplistIndex.cpp b/arangod/Indexes/SkiplistIndex.cpp index b3122461d0..3d2629d570 100644 --- a/arangod/Indexes/SkiplistIndex.cpp +++ b/arangod/Indexes/SkiplistIndex.cpp @@ -1006,6 +1006,7 @@ int SkiplistIndex::ElementElementComparator::operator() (TRI_index_element_t con // .......................................................................... // The document could be the same -- so no further comparison is required. // .......................................................................... + if (leftElement == rightElement || (! _idx->_skiplistIndex->isArray() && leftElement->document() == rightElement->document())) { return 0; @@ -1059,14 +1060,40 @@ bool SkiplistIndex::accessFitsIndex (triagens::aql::AstNode const* access, return false; } + triagens::aql::AstNode const* what = access; std::pair> attributeData; - - if (! access->isAttributeAccessForVariable(attributeData) || - attributeData.first != reference) { - // this access is not referencing this collection - return false; + + if (op->type != triagens::aql::NODE_TYPE_OPERATOR_BINARY_IN) { + if (! what->isAttributeAccessForVariable(attributeData) || + attributeData.first != reference) { + // this access is not referencing this collection + return false; + } + if (triagens::basics::TRI_AttributeNamesHaveExpansion(attributeData.second)) { + // doc.value[*] IN 'value' + return false; + } } - + else { + // ok, we do have an IN here... check if it's something like 'value' IN doc.value[*] + bool canUse = false; + if (what->isAttributeAccessForVariable(attributeData) && + attributeData.first == reference && + ! triagens::basics::TRI_AttributeNamesHaveExpansion(attributeData.second)) { + // doc.value[*] IN 'value' + canUse = true; + } + if (! canUse) { + // check for doc.value[*] IN 'value' + what = other; + if (! what->isAttributeAccessForVariable(attributeData) || + attributeData.first != reference) { + // this access is not referencing this collection + return false; + } + } + } + std::vector const& fieldNames = attributeData.second; for (size_t i = 0; i < _fields.size(); ++i) { @@ -1074,15 +1101,15 @@ bool SkiplistIndex::accessFitsIndex (triagens::aql::AstNode const* access, // attribute path length differs continue; } - - bool match = true; - for (size_t j = 0; j < _fields[i].size(); ++j) { - if (_fields[i][j] != fieldNames[j]) { - match = false; - break; - } + + if (this->isAttributeExpanded(i) && + op->type != triagens::aql::NODE_TYPE_OPERATOR_BINARY_IN) { + // If this attribute is correct or not, it could only serve for IN + continue; } + bool match = triagens::basics::AttributeName::isIdentical(_fields[i], fieldNames, true); + if (match) { // mark ith attribute as being covered auto it = found.find(i); @@ -1126,14 +1153,11 @@ void SkiplistIndex::matchAttributes (triagens::aql::AstNode const* node, case triagens::aql::NODE_TYPE_OPERATOR_BINARY_IN: if (accessFitsIndex(op->getMember(0), op->getMember(1), op, reference, found, isExecution)) { auto m = op->getMember(1); - if (m->type != triagens::aql::NODE_TYPE_EXPANSION && m->numMembers() > 1) { + if (m->isArray() && m->numMembers() > 1) { // attr IN [ a, b, c ] => this will produce multiple items, so count them! values += m->numMembers() - 1; } } - else { - accessFitsIndex(op->getMember(1), op->getMember(0), op, reference, found, isExecution); - } break; default: @@ -1301,6 +1325,7 @@ IndexIterator* SkiplistIndex::iteratorForCondition (IndexIteratorContext* contex } return new SkiplistIndexIterator(this, searchValues, reverse); } + std::unordered_map> found; size_t unused = 0; matchAttributes(node, reference, found, unused, true); diff --git a/js/server/tests/aql-queries-array-nested.js b/js/server/tests/aql-queries-array-nested.js index 79629724c4..9b28b3a141 100644 --- a/js/server/tests/aql-queries-array-nested.js +++ b/js/server/tests/aql-queries-array-nested.js @@ -58,6 +58,118 @@ function nestedArrayIndexSuite () { db._drop(cn); }, + testIndexUsageHashFlat : function () { + c.ensureIndex({ type: "hash", fields: [ "tags[*]" ] }); + c.insert({ tags: [ "foo", "bar", "baz" ] }); + c.insert({ tags: [ "foobar", "quetzalcoatl", "bark" ] }); + c.insert({ tags: [ "b0rk", "bar", "bark" ] }); + + [ + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags[*] RETURN doc", { value: "bar" }, 2 ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags RETURN doc", { value: "bar" }, 2 ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags[*] RETURN doc", { value: "bark" }, 2 ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags RETURN doc", { value: "bark" }, 2 ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags[*] RETURN doc", { value: "quetzal" }, 0 ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags RETURN doc", { value: "quetzal" }, 0 ] + ].forEach(function(query) { + var result = AQL_EXECUTE(query[0], query[1]).json; + assertEqual(query[2], result.length); + assertTrue(indexUsed(query[0], query[1])); + }); + }, + + testIndexUsageHashNested : function () { + c.ensureIndex({ type: "hash", fields: [ "tags[*].name" ] }); + c.insert({ tags: [ { name: "foo" }, { name: "bar" }, { name: "baz" } ] }); + c.insert({ tags: [ { name: "foobar" }, { name: "quetzalcoatl" }, { name: "bark" } ] }); + c.insert({ tags: [ { name: "b0rk" }, { name: "bar" }, { name: "bark" } ] }); + + var result; + + [ + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags[*].name RETURN doc", { value: "bar" }, 2 ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags[*].name RETURN doc", { value: "bark" }, 2 ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags[*].name RETURN doc", { value: "quetzal" }, 0 ] + ].forEach(function(query) { + result = AQL_EXECUTE(query[0], query[1]).json; + assertEqual(query[2], result.length); + assertTrue(indexUsed(query[0], query[1])); + }); + + [ + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags.name RETURN doc", { value: "bar" } ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags.name RETURN doc", { value: "bark" } ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags.name RETURN doc", { value: "quetzal" } ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags.name[*] RETURN doc", { value: "bar" } ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags.name[*] RETURN doc", { value: "bark" } ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags.name[*] RETURN doc", { value: "quetzal" } ], + [ "FOR doc IN " + cn + " FILTER doc.tags[*].name IN [ @value ] RETURN doc", { value: "bar" } ], + [ "FOR doc IN " + cn + " FILTER doc.tags.name[*] IN [ @value ] RETURN doc", { value: "bar" } ], + [ "FOR doc IN " + cn + " FILTER doc.tags[*].name IN NOOPT(PASSTHRU(@value)) RETURN doc", { value: "bar" } ], + [ "FOR doc IN " + cn + " FILTER doc.tags.name[*] IN NOOPT(PASSTHRU(@value)) RETURN doc", { value: "bar" } ] + ].forEach(function(query) { + result = AQL_EXECUTE(query[0], query[1]).json; + assertFalse(indexUsed(query[0], query[1]), query[0]); + }); + }, + + testIndexUsageSkiplistFlat : function () { + c.ensureIndex({ type: "skiplist", fields: [ "tags[*]" ] }); + c.insert({ tags: [ "foo", "bar", "baz" ] }); + c.insert({ tags: [ "foobar", "quetzalcoatl", "bark" ] }); + c.insert({ tags: [ "b0rk", "bar", "bark" ] }); + + [ + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags[*] RETURN doc", { value: "bar" }, 2 ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags RETURN doc", { value: "bar" }, 2 ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags[*] RETURN doc", { value: "bark" }, 2 ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags RETURN doc", { value: "bark" }, 2 ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags[*] RETURN doc", { value: "quetzal" }, 0 ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags RETURN doc", { value: "quetzal" }, 0 ] + ].forEach(function(query) { + var result = AQL_EXECUTE(query[0], query[1]).json; + assertEqual(query[2], result.length); + assertTrue(indexUsed(query[0], query[1])); + }); + }, + + testIndexUsageSkiplistNested : function () { + c.ensureIndex({ type: "skiplist", fields: [ "tags[*].name" ] }); + c.insert({ tags: [ { name: "foo" }, { name: "bar" }, { name: "baz" } ] }); + c.insert({ tags: [ { name: "foobar" }, { name: "quetzalcoatl" }, { name: "bark" } ] }); + c.insert({ tags: [ { name: "b0rk" }, { name: "bar" }, { name: "bark" } ] }); + + var result; + + [ + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags[*].name RETURN doc", { value: "bar" }, 2 ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags[*].name RETURN doc", { value: "bark" }, 2 ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags[*].name RETURN doc", { value: "quetzal" }, 0 ] + ].forEach(function(query) { + result = AQL_EXECUTE(query[0], query[1]).json; + assertEqual(query[2], result.length); + assertTrue(indexUsed(query[0], query[1])); + }); + + [ + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags.name RETURN doc", { value: "bar" } ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags.name RETURN doc", { value: "bark" } ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags.name RETURN doc", { value: "quetzal" } ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags.name[*] RETURN doc", { value: "bar" } ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags.name[*] RETURN doc", { value: "bark" } ], + [ "FOR doc IN " + cn + " FILTER @value IN doc.tags.name[*] RETURN doc", { value: "quetzal" } ], + [ "FOR doc IN " + cn + " FILTER doc.tags[*].name IN @value RETURN doc", { value: "bar" } ], + [ "FOR doc IN " + cn + " FILTER doc.tags.name[*] IN @value RETURN doc", { value: "bar" } ], + [ "FOR doc IN " + cn + " FILTER doc.tags[*].name IN [ @value ] RETURN doc", { value: "bar" } ], + [ "FOR doc IN " + cn + " FILTER doc.tags.name[*] IN [ @value ] RETURN doc", { value: "bar" } ], + [ "FOR doc IN " + cn + " FILTER doc.tags[*].name IN NOOPT(PASSTHRU(@value)) RETURN doc", { value: "bar" } ], + [ "FOR doc IN " + cn + " FILTER doc.tags.name[*] IN NOOPT(PASSTHRU(@value)) RETURN doc", { value: "bar" } ] + ].forEach(function(query) { + result = AQL_EXECUTE(query[0], query[1]).json; + assertFalse(indexUsed(query[0], query[1]), query[0]); + }); + }, + testNestedSubAttribute : function () { c.insert({ tags: [ { name: "foo" }, { name: "bar" }, { name: "baz" } ] }); c.insert({ tags: [ { name: "quux" } ] }); diff --git a/js/server/tests/aql-queries-array.js b/js/server/tests/aql-queries-array.js index 9f03a73e1a..957d9e279c 100644 --- a/js/server/tests/aql-queries-array.js +++ b/js/server/tests/aql-queries-array.js @@ -297,7 +297,7 @@ function arrayIndexSuite () { var query = `FOR x IN ${cName} FILTER x.a[*] IN [1] RETURN x`; validateIndexNotUsed(query); query = `FOR x IN ${cName} FILTER 1 IN x.a RETURN x`; - validateIndexNotUsed(query); + checkIsOptimizedQuery(query); query = `FOR x IN ${cName} FILTER x.a IN [1] RETURN x`; validateIndexNotUsed(query); }, @@ -410,7 +410,7 @@ function arrayIndexSuite () { var query = `FOR x IN ${cName} FILTER x.a[*] IN [1] RETURN x`; validateIndexNotUsed(query); query = `FOR x IN ${cName} FILTER 1 IN x.a RETURN x`; - validateIndexNotUsed(query); + checkIsOptimizedQuery(query); query = `FOR x IN ${cName} FILTER x.a IN [1] RETURN x`; validateIndexNotUsed(query); }, diff --git a/lib/Basics/AttributeNameParser.cpp b/lib/Basics/AttributeNameParser.cpp index d0e555eadb..ffa6130453 100644 --- a/lib/Basics/AttributeNameParser.cpp +++ b/lib/Basics/AttributeNameParser.cpp @@ -38,14 +38,23 @@ using AttributeName = triagens::basics::AttributeName; //////////////////////////////////////////////////////////////////////////////// bool triagens::basics::AttributeName::isIdentical (std::vector const& lhs, - std::vector const& rhs) { + std::vector const& rhs, + bool ignoreExpansionInLast) { if (lhs.size() != rhs.size()) { return false; } for (size_t i = 0; i < lhs.size(); ++i) { - if (lhs[i] != rhs[i]) { + if (lhs[i].name != rhs[i].name) { return false; + } + if (lhs[i].shouldExpand != rhs[i].shouldExpand) { + if (! ignoreExpansionInLast) { + return false; + } + if (i != lhs.size() - 1) { + return false; + } } } @@ -57,13 +66,14 @@ bool triagens::basics::AttributeName::isIdentical (std::vector co //////////////////////////////////////////////////////////////////////////////// bool triagens::basics::AttributeName::isIdentical (std::vector> const& lhs, - std::vector> const& rhs) { + std::vector> const& rhs, + bool ignoreExpansionInLast) { if (lhs.size() != rhs.size()) { return false; } for (size_t i = 0; i < lhs.size(); ++i) { - if (! isIdentical(lhs[i], rhs[i])) { + if (! isIdentical(lhs[i], rhs[i], ignoreExpansionInLast && (i == lhs.size() - 1))) { return false; } } diff --git a/lib/Basics/AttributeNameParser.h b/lib/Basics/AttributeNameParser.h index f9551eb74e..06963dc0f0 100644 --- a/lib/Basics/AttributeNameParser.h +++ b/lib/Basics/AttributeNameParser.h @@ -77,14 +77,16 @@ namespace triagens { //////////////////////////////////////////////////////////////////////////////// static bool isIdentical (std::vector const&, - std::vector const&); + std::vector const&, + bool); //////////////////////////////////////////////////////////////////////////////// /// @brief compare two attribute name vectors //////////////////////////////////////////////////////////////////////////////// static bool isIdentical (std::vector> const&, - std::vector> const&); + std::vector> const&, + bool); }; // -----------------------------------------------------------------------------