diff --git a/CHANGELOG b/CHANGELOG index 0439ebac00..b3ac87c043 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,9 @@ +v3.3.5 (XXXX-XX-XX) +------------------- + +* fixed issue #4827: COLLECT on edge _to field doesn't group distinct values as expected (MMFiles) + + v3.3.4 (2018-03-01) ------------------- diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 393f03c5dc..fa36653473 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -1858,9 +1858,55 @@ struct SortToIndexNode final : public WalkerWorker { auto condNode = root->getMember(0); if (condNode->isOnlyEqualityMatch()) { - // now check if the index fields are the same as the sort condition - // fields + // now check if the index fields are the same as the sort condition fields // e.g. FILTER c.value1 == 1 && c.value2 == 42 SORT c.value1, c.value2 + auto i = index.getIndex(); + // some special handling for the MMFiles edge index here, which to the outside + // world is an index on attributes _from and _to at the same time, but only one + // can be queried at a time + // this special handling is required in order to prevent lookups by one of the index + // attributes (e.g. _from) and a sort clause on the other index attribte (e.g. _to) + // to be treated as the same index attribute, e.g. + // FOR doc IN edgeCol FILTER doc._from == ... SORT doc._to ... + // can use the index either for lookup or for sorting, but not for both at the same + // time. this is because if we do the lookup by _from, the results will be sorted + // by _from, and not by _to. + if (i->type() == arangodb::Index::IndexType::TRI_IDX_TYPE_EDGE_INDEX && fields.size() == 2) { + // looks like MMFiles edge index + if (condNode->type == NODE_TYPE_OPERATOR_NARY_AND) { + // check all conditions of the index node, and check if we can find _from or _to + for (size_t j = 0; j < condNode->numMembers(); ++j) { + auto sub = condNode->getMemberUnchecked(j); + if (sub->type != NODE_TYPE_OPERATOR_BINARY_EQ) { + continue; + } + auto lhs = sub->getMember(0); + if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS && + lhs->getMember(0)->type == NODE_TYPE_REFERENCE && + lhs->getMember(0)->getData() == outVariable) { + // check if this is either _from or _to + std::string attr = lhs->getString(); + if (attr == StaticStrings::FromString || attr == StaticStrings::ToString) { + // reduce index fields to just the attribute we found in the index lookup condition + fields = {{arangodb::basics::AttributeName(attr, false)} }; + } + } + + auto rhs = sub->getMember(1); + if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS && + rhs->getMember(0)->type == NODE_TYPE_REFERENCE && + rhs->getMember(0)->getData() == outVariable) { + // check if this is either _from or _to + std::string attr = rhs->getString(); + if (attr == StaticStrings::FromString || attr == StaticStrings::ToString) { + // reduce index fields to just the attribute we found in the index lookup condition + fields = {{arangodb::basics::AttributeName(attr, false)} }; + } + } + } + } + } + size_t const numCovered = sortCondition.coveredAttributes(outVariable, fields); diff --git a/js/server/tests/aql/aql-optimizer-edge-index.js b/js/server/tests/aql/aql-optimizer-edge-index.js index bdc09425fb..f5eb952ce0 100644 --- a/js/server/tests/aql/aql-optimizer-edge-index.js +++ b/js/server/tests/aql/aql-optimizer-edge-index.js @@ -1,5 +1,5 @@ /* jshint globalstrict:false, strict:false, maxlen: 500 */ -/* global assertEqual, AQL_EXECUTE */ +/* global assertEqual, assertTrue, AQL_EXECUTE, AQL_EXPLAIN */ // ////////////////////////////////////////////////////////////////////////////// // / @brief tests for index usage @@ -166,6 +166,110 @@ function optimizerEdgeIndexTestSuite () { assertEqual(0, results.stats.scannedFull); assertEqual(query[1], results.stats.scannedIndex); }); + }, + + testLookupOnFromSortOnToAttribute: function () { + let query = "FOR doc IN " + e.name() + " FILTER doc._from == 'UnitTestsCollection/nono' COLLECT to = doc._to RETURN to"; + let results = AQL_EXECUTE(query); + assertEqual(19, results.json.length); + + let node = AQL_EXPLAIN(query).plan.nodes.filter(function(n) { return n.type === 'SortNode'; }); + assertEqual(1, node.length); + node = node[0]; + assertEqual(1, node.elements.length); + assertEqual("to", node.elements[0].inVariable.name); + assertTrue(node.elements[0].ascending); + }, + + testLookupOnFromSortOnFromAttribute: function () { + let query = "FOR doc IN " + e.name() + " FILTER doc._from == 'UnitTestsCollection/nono' COLLECT from = doc._from RETURN from"; + let results = AQL_EXECUTE(query); + assertEqual(1, results.json.length); + + let node = AQL_EXPLAIN(query).plan.nodes.filter(function(n) { return n.type === 'SortNode'; }); + assertEqual(0, node.length); + }, + + testLookupOnFromSortOnFromToAttribute: function () { + let query = "FOR doc IN " + e.name() + " FILTER doc._from == 'UnitTestsCollection/nono' COLLECT from = doc._from, to = doc._to RETURN { from, to }"; + let results = AQL_EXECUTE(query); + assertEqual(19, results.json.length); + + let node = AQL_EXPLAIN(query).plan.nodes.filter(function(n) { return n.type === 'SortNode'; }); + assertEqual(1, node.length); + node = node[0]; + assertEqual(2, node.elements.length); + assertEqual("from", node.elements[0].inVariable.name); + assertEqual("to", node.elements[1].inVariable.name); + assertTrue(node.elements[0].ascending); + assertTrue(node.elements[1].ascending); + }, + + testLookupOnFromSortOnToFromAttribute: function () { + let query = "FOR doc IN " + e.name() + " FILTER doc._from == 'UnitTestsCollection/nono' COLLECT to = doc._to, from = doc._from RETURN { from, to }"; + let results = AQL_EXECUTE(query); + assertEqual(19, results.json.length); + + let node = AQL_EXPLAIN(query).plan.nodes.filter(function(n) { return n.type === 'SortNode'; }); + assertEqual(1, node.length); + node = node[0]; + assertEqual(2, node.elements.length); + assertEqual("to", node.elements[0].inVariable.name); + assertEqual("from", node.elements[1].inVariable.name); + assertTrue(node.elements[0].ascending); + assertTrue(node.elements[1].ascending); + }, + + testLookupOnToSortOnFromAttribute: function () { + let query = "FOR doc IN " + e.name() + " FILTER doc._to == 'UnitTestsCollection/nono' COLLECT from = doc._from RETURN from"; + let results = AQL_EXECUTE(query); + assertEqual(19, results.json.length); + + let node = AQL_EXPLAIN(query).plan.nodes.filter(function(n) { return n.type === 'SortNode'; }); + assertEqual(1, node.length); + node = node[0]; + assertEqual(1, node.elements.length); + assertEqual("from", node.elements[0].inVariable.name); + assertTrue(node.elements[0].ascending); + }, + + testLookupOnToSortOnToAttribute: function () { + let query = "FOR doc IN " + e.name() + " FILTER doc._to == 'UnitTestsCollection/nono' COLLECT to = doc._to RETURN to"; + let results = AQL_EXECUTE(query); + assertEqual(1, results.json.length); + + let node = AQL_EXPLAIN(query).plan.nodes.filter(function(n) { return n.type === 'SortNode'; }); + assertEqual(0, node.length); + }, + + testLookupOnToSortOnToFromAttribute: function () { + let query = "FOR doc IN " + e.name() + " FILTER doc._to == 'UnitTestsCollection/nono' COLLECT to = doc._to, from = doc._from RETURN { from, to }"; + let results = AQL_EXECUTE(query); + assertEqual(19, results.json.length); + + let node = AQL_EXPLAIN(query).plan.nodes.filter(function(n) { return n.type === 'SortNode'; }); + assertEqual(1, node.length); + node = node[0]; + assertEqual(2, node.elements.length); + assertEqual("to", node.elements[0].inVariable.name); + assertEqual("from", node.elements[1].inVariable.name); + assertTrue(node.elements[0].ascending); + assertTrue(node.elements[1].ascending); + }, + + testLookupOnToSortOnFromToAttribute: function () { + let query = "FOR doc IN " + e.name() + " FILTER doc._to == 'UnitTestsCollection/nono' COLLECT from = doc._from, to = doc._to RETURN { from, to }"; + let results = AQL_EXECUTE(query); + assertEqual(19, results.json.length); + + let node = AQL_EXPLAIN(query).plan.nodes.filter(function(n) { return n.type === 'SortNode'; }); + assertEqual(1, node.length); + node = node[0]; + assertEqual(2, node.elements.length); + assertEqual("from", node.elements[0].inVariable.name); + assertEqual("to", node.elements[1].inVariable.name); + assertTrue(node.elements[0].ascending); + assertTrue(node.elements[1].ascending); } }; @@ -174,4 +278,3 @@ function optimizerEdgeIndexTestSuite () { jsunity.run(optimizerEdgeIndexTestSuite); return jsunity.done(); -