From 36e398f3f4c3bcc54ac6b3ce76be4edc6eebe668 Mon Sep 17 00:00:00 2001
From: Michael Hackstein <michael@arangodb.com>
Date: Mon, 23 Jan 2017 09:45:22 +0100
Subject: [PATCH] Fixed a bug with sort optimization in indexes. Also adapted
 tests.

---
 arangod/Aql/ClusterBlocks.cpp                 |  2 +-
 arangod/Aql/ExecutionNode.h                   |  4 +++
 arangod/Aql/OptimizerRules.cpp                | 30 +++++++++++++++++--
 .../aql-optimizer-rule-use-index-for-sort.js  |  4 +--
 4 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/arangod/Aql/ClusterBlocks.cpp b/arangod/Aql/ClusterBlocks.cpp
index c11a6b9592..00780f7727 100644
--- a/arangod/Aql/ClusterBlocks.cpp
+++ b/arangod/Aql/ClusterBlocks.cpp
@@ -458,7 +458,7 @@ bool GatherBlock::OurLessThan::operator()(std::pair<size_t, size_t> const& a,
       // Take attributePath into consideration:
       AqlValue topA = _gatherBlockBuffer.at(a.first).front()->getValue(a.second,
                                                                        reg.reg);
-      AqlValue topB = _gatherBlockBuffer.at(a.first).front()->getValue(b.second,
+      AqlValue topB = _gatherBlockBuffer.at(b.first).front()->getValue(b.second,
                                                                        reg.reg);
       bool mustDestroyA;
       AqlValue aa = topA.get(_trx, reg.attributePath, mustDestroyA, false);
diff --git a/arangod/Aql/ExecutionNode.h b/arangod/Aql/ExecutionNode.h
index 72363d4d0b..6a53a82a32 100644
--- a/arangod/Aql/ExecutionNode.h
+++ b/arangod/Aql/ExecutionNode.h
@@ -85,6 +85,10 @@ struct SortElement {
   SortElement(Variable const* v, bool asc)
     : var(v), ascending(asc) {
   }
+
+  SortElement(Variable const* v, bool asc, std::vector<std::string> const& path)
+    : var(v), ascending(asc), attributePath(path) {
+  }
 };
 
 typedef std::vector<SortElement> SortElementVector;
diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp
index b614a18d0a..bf4e10d9d0 100644
--- a/arangod/Aql/OptimizerRules.cpp
+++ b/arangod/Aql/OptimizerRules.cpp
@@ -2332,12 +2332,33 @@ void arangodb::aql::scatterInClusterRule(Optimizer* opt, ExecutionPlan* plan,
       TRI_vocbase_t* vocbase = nullptr;
       Collection const* collection = nullptr;
 
+      SortElementVector elements;
+
       if (nodeType == ExecutionNode::ENUMERATE_COLLECTION) {
         vocbase = static_cast<EnumerateCollectionNode*>(node)->vocbase();
         collection = static_cast<EnumerateCollectionNode*>(node)->collection();
       } else if (nodeType == ExecutionNode::INDEX) {
-        vocbase = static_cast<IndexNode*>(node)->vocbase();
-        collection = static_cast<IndexNode*>(node)->collection();
+        auto idxNode = static_cast<IndexNode*>(node);
+        vocbase = idxNode->vocbase();
+        collection = idxNode->collection();
+        auto outVars = idxNode->getVariablesSetHere();
+        TRI_ASSERT(outVars.size() == 1);
+        Variable const* sortVariable = outVars[0];
+        bool isSortReverse = idxNode->reverse();
+        auto allIndexes = idxNode->getIndexes();
+        TRI_ASSERT(!allIndexes.empty());
+
+        // Using Index for sort only works if all indexes are equal.
+        auto first = allIndexes[0].getIndex();
+        for (auto const& path : first->fieldNames()) {
+          elements.emplace_back(sortVariable, !isSortReverse, path);
+        }
+        for (auto const& it : allIndexes) {
+          if (first != it.getIndex()) {
+            elements.clear();
+            break;
+          }
+        }
       } else if (nodeType == ExecutionNode::INSERT ||
                  nodeType == ExecutionNode::UPDATE ||
                  nodeType == ExecutionNode::REPLACE ||
@@ -2383,11 +2404,14 @@ void arangodb::aql::scatterInClusterRule(Optimizer* opt, ExecutionPlan* plan,
       remoteNode->addDependency(node);
 
       // insert a gather node
-      ExecutionNode* gatherNode =
+      GatherNode* gatherNode =
           new GatherNode(plan, plan->nextId(), vocbase, collection);
       plan->registerNode(gatherNode);
       TRI_ASSERT(remoteNode);
       gatherNode->addDependency(remoteNode);
+      if (!elements.empty()) {
+        gatherNode->setElements(elements);
+      }
 
       // and now link the gather node with the rest of the plan
       if (parents.size() == 1) {
diff --git a/js/server/tests/aql/aql-optimizer-rule-use-index-for-sort.js b/js/server/tests/aql/aql-optimizer-rule-use-index-for-sort.js
index ad302bf6b4..305b6fe468 100644
--- a/js/server/tests/aql/aql-optimizer-rule-use-index-for-sort.js
+++ b/js/server/tests/aql/aql-optimizer-rule-use-index-for-sort.js
@@ -112,7 +112,7 @@ function optimizerRuleTestSuite() {
       var loopto = 10;
 
       internal.db._drop(colName);
-      skiplist = internal.db._create(colName);
+      skiplist = internal.db._create(colName, {numberOfShards: 4});
       var i, j;
       for (j = 1; j <= loopto; ++j) {
         for (i = 1; i <= loopto; ++i) {
@@ -127,7 +127,7 @@ function optimizerRuleTestSuite() {
       skiplist.ensureIndex({ type: "hash", fields: [ "c" ], unique: false });
 
       internal.db._drop(colNameOther);
-      skiplist2 = internal.db._create(colNameOther);
+      skiplist2 = internal.db._create(colNameOther, {numberOfShards: 4});
       for (j = 1; j <= loopto; ++j) {
         for (i = 1; i <= loopto; ++i) {
           skiplist2.save({ "f" : i, "g": j , "h": j, "i": i, "j": i, "joinme" : "aoeu " + j});