From 93d5c6076d51384cf294bd5e322c046d98d6bc28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Mon, 27 Sep 2021 10:46:20 +0200 Subject: [PATCH 1/3] Fixed issue #14807; added regression tests and CHANGELOG entry --- CHANGELOG | 4 +++ arangod/Aql/OptimizerRules.cpp | 20 +++-------- ...optimizer-rule-remove-collect-variables.js | 36 +++++++++++++++++++ 3 files changed, 45 insertions(+), 15 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 812cabe4a51d..7ddd212e41f5 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,10 @@ v3.7.15 (XXXX-XX-XX) -------------------- +* Fix issue #14807: Fix crash during optimization of certain AQL queries during + the removeCollectVariables optimizer rule, when a COLLECT node without output + variables (this includes RETURN DISTINCT) occured in the plan. + * Fixed a bug for array indexes on update of documents (BTS-548). * Fix active failover, so that the new host actually has working foxx services diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 6807c5c96399..5f871be2e36f 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -1250,7 +1250,7 @@ void arangodb::aql::removeCollectVariablesRule(Optimizer* opt, auto collectNode = ExecutionNode::castTo(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(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 index e914990bf77b..a744ba0ee13b 100644 --- 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)); + }, + }; } From 17987743ea1a76d7ac669e3b03a823ad0083a3c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20G=C3=B6dderz?= Date: Mon, 27 Sep 2021 10:56:30 +0200 Subject: [PATCH 2/3] Update CHANGELOG --- CHANGELOG | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 7ddd212e41f5..43b38c3d8c2f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,7 +2,7 @@ v3.7.15 (XXXX-XX-XX) -------------------- * Fix issue #14807: Fix crash during optimization of certain AQL queries during - the removeCollectVariables optimizer rule, when a COLLECT node without output + the remove-collect-variables optimizer rule, when a COLLECT node without output variables (this includes RETURN DISTINCT) occured in the plan. * Fixed a bug for array indexes on update of documents (BTS-548). From cb9eed6c612334dfa47b21b877b1bf4c01aa99da Mon Sep 17 00:00:00 2001 From: Vadim Date: Wed, 29 Sep 2021 12:22:06 +0300 Subject: [PATCH 3/3] Update CHANGELOG --- CHANGELOG | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 43b38c3d8c2f..d8c5e2b3f226 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,9 +1,13 @@ -v3.7.15 (XXXX-XX-XX) +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. + the remove-collect-variables optimizer rule, when a COLLECT node without + output variables (this includes RETURN DISTINCT) occured in the plan. + + +v3.7.15 (XXXX-XX-XX) +-------------------- * Fixed a bug for array indexes on update of documents (BTS-548).