8000 [3.8] Fix crash in optimizer rule remove-collect-variables (#14825) · arangodb/arangodb@8a6b2e4 · GitHub
[go: up one dir, main page]
Skip to content

Commit 8a6b2e4

Browse files
goedderzKVS85
goedderz
and
KVS85
authored
[3.8] Fix crash in optimizer rule remove-collect-variables (#14825)
* Fixed issue #14807; added regression tests and CHANGELOG entry * Update CHANGELOG Co-authored-by: Vadim <vadim@arangodb.com>
1 parent 289664f commit 8a6b2e4

File tree

3 files changed

+45
-15
lines changed

3 files changed

+45
-15
lines changed

‎CHANGELOG

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
v3.8.2 (XXXX-XX-XX)
22
-------------------
33

4+
* Fix issue #14807: Fix crash during optimization of certain AQL queries during
5+
the remove-collect-variables optimizer rule, when a COLLECT node without
6+
output variables (this includes RETURN DISTINCT) occured in the plan.
7+
48
* Prevent some possible deadlocks under high load regarding transactions and
59
document operations, and also improve performance slightly.
610

‎arangod/Aql/OptimizerRules.cpp

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,7 +1330,7 @@ void arangodb::aql::removeCollectVariablesRule(Optimizer* opt,
13301330
auto collectNode = ExecutionNode::castTo<CollectNode*>(n);
13311331
TRI_ASSERT(collectNode != nullptr);
13321332

1333-
auto const& varsUsedLater = n->getVarsUsedLater();
1333+
auto const& varsUsedLater = collectNode->getVarsUsedLater();
13341334
auto outVariable = collectNode->outVariable();
13351335

13361336
if (outVariable != nullptr &&
@@ -1371,27 +1371,17 @@ void arangodb::aql::removeCollectVariablesRule(Optimizer* opt,
13711371
}
13721372

13731373
} // end - expression exists
1374-
} else if (planNode->getType() == EN::COLLECT) {
1375-
auto innerCollectNode = ExecutionNode::castTo<CollectNode const*>(planNode);
1376-
if (innerCollectNode->hasOutVariable()) {
1377-
// We have the following situation:
1378-
//
1379-
// COLLECT v1 = doc._id INTO g1
1380-
// COLLECT v2 = doc._id INTO g2
1381-
//
1382-
searchVariables.push_back(innerCollectNode->outVariable());
1383-
} else {
1384-
// when we find another COLLECT, it will invalidate all
1385-
// previous variables in the scope
1386-
searchVariables.clear();
1387-
}
13881374
} else {
13891375
auto here = planNode->getVariableIdsUsedHere();
1376+
TRI_ASSERT(!searchVariables.empty());
13901377
if (here.find(searchVariables.back()->id) != here.end()) {
13911378
// the outVariable of the last collect should not be used by any following node directly
13921379
doOptimize = false;
13931380
break;
13941381
}
1382+
if (planNode->getType() == EN::COLLECT) {
1383+
break;
1384+
}
13951385
}
13961386

13971387
planNode = planNode->getFirstParent();

‎tests/js/server/aql/aql-optimizer-rule-remove-collect-variables.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,42 @@ function optimizerRuleTestSuite () {
357357
assertNotEqual(-1, explain.plan.rules.indexOf(ruleName));
358358
},
359359

360+
// Regression test for https://github.com/arangodb/arangodb/issues/14807.
361+
// This resulted in an invalid memory access in the removeCollectVariablesRule.
362+
testIssue14807 : function () {
363+
const query = `
364+
for u in union([],[])
365+
collect test = u.v into g
366+
return Distinct {
367+
Test: test,
368+
Prop1: MIN(g[*].u.someProp),
369+
Prop2: MAX(g[*].u.someProp)
370+
}
371+
`;
372+
373+
const expectedResults = [];
374+
375+
const results = AQL_EXECUTE(query);
376+
assertEqual(expectedResults, results.json);
377+
},
378+
379+
testCollectIntoBeforeCollectWithoutInto : function () {
380+
const query = `
381+
LET items = [{_id: 'ID'}]
382+
FOR item1 IN items
383+
COLLECT unused1 = item1._id INTO first
384+
COLLECT unused2 = first[0].item1._id
385+
RETURN unused2
386+
`;
387+
const expected = [ "ID" ];
388+
389+
let resultEnabled = AQL_EXECUTE(query, { }, paramEnabled).json;
390+
assertEqual(expected, resultEnabled);
391+
392+
let explain = AQL_EXPLAIN(query, { }, paramEnabled);
393+
assertNotEqual(-1, explain.plan.rules.indexOf(ruleName));
394+
},
395+
360396
};
361397
}
362398

0 commit comments

Comments
 (0)
0