arangodb · KVS85 · Sep 29, 2021 · Sep 27, 2021 · Sep 27, 2021 · Sep 29, 2021
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,3 +1,11 @@
+v3.7.16 (XXXX-XX-XX)
+--------------------
+
+* Fix issue #14807: Fix crash during optimization of certain AQL queries during
+  the remove-collect-variables optimizer rule, when a COLLECT node without
+  output variables (this includes RETURN DISTINCT) occured in the plan.
+
+
 v3.7.15 (2021-09-22)
 --------------------
 

diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp
@@ -1250,7 +1250,7 @@ void arangodb::aql::removeCollectVariablesRule(Optimizer* opt,
     auto collectNode = ExecutionNode::castTo<CollectNode*>(n);
     TRI_ASSERT(collectNode != nullptr);
 
-    auto const& varsUsedLater = n->getVarsUsedLater();
+    auto const& varsUsedLater = collectNode->getVarsUsedLater();
     auto outVariable = collectNode->outVariable();
 
     if (outVariable != nullptr &&
@@ -1294,27 +1294,17 @@ void arangodb::aql::removeCollectVariablesRule(Optimizer* opt,
             }
 
           }  // end - expression exists
-        } else if (planNode->getType() == EN::COLLECT) {
-          auto innerCollectNode = ExecutionNode::castTo<CollectNode const*>(planNode);
-          if (innerCollectNode->hasOutVariable()) {
-            // We have the following situation:
-            //
-            // COLLECT v1 = doc._id INTO g1
-            // COLLECT v2 = doc._id INTO g2
-            //
-            searchVariables.push_back(innerCollectNode->outVariable());
-          } else {
-            // when we find another COLLECT, it will invalidate all
-            // previous variables in the scope
-            searchVariables.clear();
-          }
         } else {
           auto here = planNode->getVariableIdsUsedHere();
+          TRI_ASSERT(!searchVariables.empty());
           if (here.find(searchVariables.back()->id) != here.end()) {
             // the outVariable of the last collect should not be used by any following node directly
             doOptimize = false;
             break;
           }
+          if (planNode->getType() == EN::COLLECT) {
+            break;
+          }
         }
 
         planNode = planNode->getFirstParent();

diff --git a/tests/js/server/aql/aql-optimizer-rule-remove-collect-variables.js b/tests/js/server/aql/aql-optimizer-rule-remove-collect-variables.js
@@ -357,6 +357,42 @@ function optimizerRuleTestSuite () {
       assertNotEqual(-1, explain.plan.rules.indexOf(ruleName));
     },
 
+    // Regression test for https://github.com/arangodb/arangodb/issues/14807.
+    // This resulted in an invalid memory access in the removeCollectVariablesRule.
+    testIssue14807 : function () {
+      const query = `
+        for u in union([],[])
+        collect test = u.v into g
+        return Distinct {
+          Test: test,
+            Prop1: MIN(g[*].u.someProp),
+            Prop2: MAX(g[*].u.someProp)
+        }
+      `;
+
+      const expectedResults = [];
+
+      const results = AQL_EXECUTE(query);
+      assertEqual(expectedResults, results.json);
+    },
+
+    testCollectIntoBeforeCollectWithoutInto : function () {
+      const query = `
+         LET items = [{_id: 'ID'}]
+         FOR item1 IN items
+           COLLECT unused1 = item1._id INTO first
+           COLLECT unused2 = first[0].item1._id
+           RETURN unused2
+         `;
+      const expected = [ "ID" ];
+
+      let resultEnabled = AQL_EXECUTE(query, { }, paramEnabled).json;
+      assertEqual(expected, resultEnabled);
+
+      let explain =  AQL_EXPLAIN(query, { }, paramEnabled);
+      assertNotEqual(-1, explain.plan.rules.indexOf(ruleName));
+    },
+
   };
 }