fixed issue #4827: COLLECT on edge _to field doesn't group distinct values as expected (#4829)

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)
 -------------------
 

arangod/Aql/OptimizerRules.cpp

@@ -1858,9 +1858,55 @@ struct SortToIndexNode final : public WalkerWorker<ExecutionNode> {
         auto condNode = root->getMember(0);
 
         if (condNode->isOnlyEqualityMatch()) {
-          // now check if the index fields are the same as the sort condition
+          // now check if the index fields are the same as the sort condition fields
-          // 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);
 

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();
-