8000 Fixed issue #15476: FATAL {crash} occurs on a simple query. (#15480) · strogo/arangodb@9734ddb · GitHub
[go: up one dir, main page]
Skip to content

Commit 9734ddb

Browse files
jsteemannjsteemann
authored
Fixed issue arangodb#15476: FATAL {crash} occurs on a simple query. (arangodb#15480)
1 parent fbdd9bd commit 9734ddb

File tree

3 files changed

+96
-39
lines changed

3 files changed

+96
-39
lines changed

‎CHANGELOG

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
devel
22
-----
33

4+
* Fixed issue #15476: FATAL {crash} occurs on a simple query.
5+
46
* Added `disableIndex` index hint for AQL FOR loops. This index hint disables
57
the usage of any index (except geo or full text indexes) and will cause a
68
full scan over the collection.

‎arangod/Aql/OptimizerRules.cpp

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1503,16 +1503,31 @@ void arangodb::aql::removeCollectVariablesRule(
15031503
} // end - if doOptimize
15041504
} // end - if collectNode has outVariable
15051505

1506+
size_t numGroupVariables = collectNode->groupVariables().size();
1507+
size_t numAggregateVariables = collectNode->aggregateVariables().size();
1508+
15061509
collectNode->clearAggregates(
1507-
[&varsUsedLater, &modified](AggregateVarInfo const& aggregate) -> bool {
1510+
[&varsUsedLater, &numGroupVariables, &numAggregateVariables,
1511+
&modified](AggregateVarInfo const& aggregate) -> bool {
1512+
// it is ok to remove unused aggregations if we have at least one
1513+
// aggregate variable remaining, or if we have a group variable left.
1514+
// it is not ok to have 0 aggregate variables and 0 group variables
1515+
// left, because the different COLLECT executors require some
1516+
// variables to be present.
15081517
if (varsUsedLater.find(aggregate.outVar) == varsUsedLater.end()) {
15091518
// result of aggregate function not used later
1510-
modified = true;
1511-
return true;
1519+
if (numGroupVariables > 0 || numAggregateVariables > 1) {
1520+
--numAggregateVariables;
1521+
modified = true;
1522+
return true;
1523+
}
15121524
}
15131525
return false;
15141526
});
15151527

1528+
TRI_ASSERT(!collectNode->groupVariables().empty() ||
1529+
!collectNode->aggregateVariables().empty());
1530+
15161531
} // for node in nodes
15171532
opt->addPlan(std::move(plan), rule, modified);
15181533
}

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

Lines changed: 76 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
////////////////////////////////////////////////////////////////////////////////
55
/// @brief tests for optimizer rules
66
///
7-
/// @file
8-
///
97
/// DISCLAIMER
108
///
119
/// Copyright 2010-2012 triagens GmbH, Cologne, Germany
@@ -28,20 +26,16 @@
2826
/// @author Copyright 2012, triAGENS GmbH, Cologne, Germany
2927
////////////////////////////////////////////////////////////////////////////////
3028

31-
var jsunity = require("jsunity");
32-
var helper = require("@arangodb/aql-helper");
33-
var isEqual = helper.isEqual;
34-
35-
////////////////////////////////////////////////////////////////////////////////
36-
/// @brief test suite
37-
////////////////////////////////////////////////////////////////////////////////
29+
const jsunity = require("jsunity");
30+
const helper = require("@arangodb/aql-helper");
31+
const isEqual = helper.isEqual;
3832

3933
function optimizerRuleTestSuite () {
40-
var ruleName = "remove-collect-variables";
34+
const ruleName = "remove-collect-variables";
4135
// various choices to control the optimizer:
42-
var paramNone = { optimizer: { rules: [ "-all" ] } };
43-
var paramEnabled = { optimizer: { rules: [ "-all", "+" + ruleName ] } };
44-
var paramDisabled = { optimizer: { rules: [ "+all", "-" + ruleName ] } };
36+
const paramNone = { optimizer: { rules: [ "-all" ] } };
37+
const paramEnabled = { optimizer: { rules: [ "-all", "+" + ruleName ] } };
38+
const paramDisabled = { optimizer: { rules: [ "+all", "-" + ruleName ] } };
4539

4640
return {
4741

@@ -50,14 +44,18 @@ function optimizerRuleTestSuite () {
5044
////////////////////////////////////////////////////////////////////////////////
5145

5246
testRuleDisabled : function () {
53-
var queries = [
47+
const queries = [
5448
"FOR i IN 1..10 COLLECT a = i INTO group RETURN a",
5549
"FOR i IN 1..10 FOR j IN 1..10 COLLECT a = i, b = j INTO group RETURN a",
56-
"FOR i IN 1..10 FOR j IN 1..10 COLLECT a = i, b = j INTO group RETURN { a: a, b : b }"
50+
"FOR i IN 1..10 FOR j IN 1..10 COLLECT a = i, b = j INTO group RETURN { a: a, b : b }",
51+
"FOR i IN 1..10 COLLECT WITH COUNT INTO cnt RETURN 1",
52+
"FOR i IN 1..10 COLLECT AGGREGATE cnt = COUNT() RETURN 1",
53+
"FOR i IN 1..10 COLLECT AGGREGATE sum = SUM(i) RETURN 1",
54+
"FOR i IN 1..10 COLLECT AGGREGATE cnt = COUNT(), sum = SUM(i) RETURN 1",
5755
];
5856

5957
queries.forEach(function(query) {
60-
var result = AQL_EXPLAIN(query, { }, paramNone);
58+
let result = AQL_EXPLAIN(query, { }, paramNone);
6159
assertEqual([ ], result.plan.rules);
6260
});
6361
},
@@ -67,15 +65,23 @@ function optimizerRuleTestSuite () {
6765
////////////////////////////////////////////////////////////////////////////////
6866

6967
testRuleNoEffect : function () {
70-
var queries = [
68+
const queries = [
7169
"FOR i IN 1..10 COLLECT a = i RETURN a",
7270
"FOR i IN 1..10 FOR j IN 1..10 COLLECT a = i, b = j RETURN a",
7371
"FOR i IN 1..10 FOR j IN 1..10 COLLECT a = i, b = j INTO group RETURN { a: a, b : b, group: group }",
72+
"FOR i IN 1..10 COLLECT AGGREGATE cnt = COUNT() RETURN cnt",
73+
"FOR i IN 1..10 COLLECT AGGREGATE cnt = COUNT() RETURN 1",
74+
"FOR i IN 1..10 COLLECT AGGREGATE sum = SUM(i) RETURN sum",
75+
"FOR i IN 1..10 COLLECT AGGREGATE sum = SUM(i) RETURN 1",
76+
"FOR i IN 1..10 COLLECT AGGREGATE cnt = COUNT(), sum = SUM(i) RETURN [cnt, sum]",
77+
"FOR i IN 1..10 COLLECT v = i WITH COUNT INTO cnt RETURN cnt",
78+
"FOR i IN 1..10 COLLECT v = i AGGREGATE cnt = COUNT() RETURN cnt",
79+
"FOR i IN 1..10 COLLECT v = i AGGREGATE sum = SUM(i) RETURN sum",
7480
];
7581

7682
queries.forEach(function(query) {
77-
var result = AQL_EXPLAIN(query, { }, paramEnabled);
78-
assertTrue(result.plan.rules.indexOf(ruleName) === -1, query);
83+
let result = AQL_EXPLAIN(query, { }, paramEnabled);
84+
assertEqual(-1, result.plan.rules.indexOf(ruleName), query);
7985
});
8086
},
8187

@@ -84,14 +90,19 @@ function optimizerRuleTestSuite () {
8490
////////////////////////////////////////////////////////////////////////////////
8591

8692
testRuleHasEffect : function () {
87-
var queries = [
93+
const queries = [
8894
"FOR i IN 1..10 COLLECT a = i INTO group RETURN a",
8995
"FOR i IN 1..10 FOR j IN 1..10 COLLECT a = i, b = j INTO group RETURN a",
9096
"FOR i IN 1..10 FOR j IN 1..10 COLLECT a = i, b = j INTO group RETURN { a: a, b : b }",
97+
"FOR i IN 1..10 COLLECT AGGREGATE cnt = COUNT(), sum = SUM(i) RETURN 1",
98+
"FOR i IN 1..10 COLLECT v = i AGGREGATE cnt = COUNT() RETURN 1",
99+
"FOR i IN 1..10 COLLECT v = i AGGREGATE sum = SUM(i) RETURN 1",
100+
"FOR i IN 1..10 COLLECT v = i AGGREGATE cnt = COUNT(), sum = SUM(i) RETURN 1",
101+
"FOR i IN 1..10 COLLECT v = i WITH COUNT INTO cnt RETURN 1",
91102
];
92103

93104
queries.forEach(function(query) {
94-
var result = AQL_EXPLAIN(query, { }, paramEnabled);
105+
let result = AQL_EXPLAIN(query, { }, paramEnabled);
95106
assertNotEqual(-1, result.plan.rules.indexOf(ruleName), query);
96107
});
97108
},
@@ -102,14 +113,16 @@ function optimizerRuleTestSuite () {
102113

103114

104115
testPlans : function () {
105-
var plans = [
116+
const plans = [
106117
[ "FOR i IN 1..10 COLLECT a = i INTO group RETURN a", [ "SingletonNode", "CalculationNode", "EnumerateListNode", "SortNode", "CollectNode", "ReturnNode" ] ],
107118
[ "FOR i IN 1..10 FOR j IN 1..10 COLLECT a = i, b = j INTO group RETURN a", [ "SingletonNode", "CalculationNode", "EnumerateListNode", "CalculationNode", "EnumerateListNode", "SortNode", "CollectNode", "ReturnNode" ] ],
108-
[ "FOR i IN 1..10 FOR j IN 1..10 COLLECT a = i, b = j INTO group RETURN { a: a, b : b }", [ "SingletonNode", "CalculationNode", "EnumerateListNode", "CalculationNode", "EnumerateListNode", "SortNode", "CollectNode", "CalculationNode", "ReturnNode" ] ]
119+
[ "FOR i IN 1..10 FOR j IN 1..10 COLLECT a = i, b = j INTO group RETURN { a: a, b : b }", [ "SingletonNode", "CalculationNode", "EnumerateListNode", "CalculationNode", "EnumerateListNode", "SortNode", "CollectNode", "CalculationNode", "ReturnNode" ] ],
120+
[ "FOR i IN 1..10 COLLECT v = i WITH COUNT INTO cnt RETURN 1", [ "SingletonNode", "CalculationNode", "EnumerateListNode", "CollectNode", "SortNode", "CalculationNode", "ReturnNode" ] ],
121+
[ "FOR i IN 1..10 COLLECT v = i AGGREGATE cnt = COUNT() RETURN 1", [ "SingletonNode", "CalculationNode", "EnumerateListNode", "CollectNode", "SortNode", "CalculationNode", "ReturnNode" ] ],
109122
];
110123

111124
plans.forEach(function(plan) {
112-
var result = AQL_EXPLAIN(plan[0], { }, paramEnabled);
125+
let result = AQL_EXPLAIN(plan[0], { }, paramEnabled);
113126
assertNotEqual(-1, result.plan.rules.indexOf(ruleName), plan[0]);
114127
assertEqual(plan[1], helper.getCompactPlan(result).map(function(node) { return node.type; }), plan[0]);
115128
});
@@ -120,25 +133,57 @@ function optimizerRuleTestSuite () {
120133
////////////////////////////////////////////////////////////////////////////////
121134

122135
testResults : function () {
123-
var queries = [
136+
const queries = [
124137
[ "FOR i IN 1..10 COLLECT a = i INTO group RETURN a", [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ] ],
125138
[ "FOR i IN 1..2 FOR j IN 1..2 COLLECT a = i, b = j INTO group RETURN [ a, b ]", [ [ 1, 1 ], [ 1, 2 ], [ 2, 1 ], [ 2, 2 ] ] ],
139+
[ "FOR i IN [] COLLECT v = i AGGREGATE cnt = COUNT() RETURN 1", [ ] ],
140+
[ "FOR i IN [] COLLECT v = i WITH COUNT INTO cnt RETURN 1", [ ] ],
141+
[ "FOR i IN 1..10 COLLECT v = i AGGREGATE cnt = COUNT() RETURN 1", [ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 ] ],
142+
[ "FOR i IN 1..10 COLLECT v = i WITH COUNT INTO cnt RETURN 1", [ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 ] ],
143+
[ "FOR i IN 1..10 COLLECT v = i AGGREGATE cnt = COUNT(), sum = SUM(i) RETURN 1", [ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 ] ],
144+
[ "FOR i IN 1..10 COLLECT v = i AGGREGATE cnt = COUNT(), sum = SUM(i) RETURN cnt", [ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 ] ],
145+
[ "FOR i IN 1..10 COLLECT v = i AGGREGATE cnt = COUNT(), sum = SUM(i) RETURN sum", [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ] ],
126146
];
127147

128148
queries.forEach(function(query) {
129-
var planDisabled = AQL_EXPLAIN(query[0], { }, paramDisabled);
130-
var planEnabled = AQL_EXPLAIN(query[0], { }, paramEnabled);
149+
let planDisabled = AQL_EXPLAIN(query[0], { }, paramDisabled);
150+
let planEnabled = AQL_EXPLAIN(query[0], { }, paramEnabled);
131151

132-
var resultDisabled = AQL_EXECUTE(query[0], { }, paramDisabled).json;
133-
var resultEnabled = AQL_EXECUTE(query[0], { }, paramEnabled).json;
152+
let resultDisabled = AQL_EXECUTE(query[0], { }, paramDisabled).json;
153+
let resultEnabled = AQL_EXECUTE(query[0], { }, paramEnabled).json;
134154

135155
assertTrue(isEqual(resultDisabled, resultEnabled), query[0]);
136156

137157
assertEqual(-1, planDisabled.plan.rules.indexOf(ruleName), query[0]);
138158
assertNotEqual(-1, planEnabled.plan.rules.indexOf(ruleName), query[0]);
139159

140-
assertEqual(resultDisabled, query[1]);
141-
assertEqual(resultEnabled, query[1]);
160+
assertEqual(resultDisabled, query[1], query);
161+
assertEqual(resultEnabled, query[1], query);
162+
});
163+
},
164+
165+
////////////////////////////////////////////////////////////////////////////////
166+
/// @brief test results
167+
////////////////////////////////////////////////////////////////////////////////
168+
169+
testResultsWhenRuleCannotFire : function () {
170+
const queries = [
171+
[ "FOR i IN [] COLLECT AGGREGATE cnt = COUNT() RETURN 1", [ 1 ] ],
172+
[ "FOR i IN [] COLLECT WITH COUNT INTO cnt RETURN 1", [ 1 ] ],
173+
[ "FOR i IN 1..10 COLLECT AGGREGATE cnt = COUNT() RETURN 1", [ 1 ] ],
174+
[ "FOR i IN 1..10 COLLECT WITH COUNT INTO cnt RETURN 1", [ 1 ] ],
175+
[ "FOR i IN [] COLLECT AGGREGATE cnt = COUNT() RETURN cnt", [ 0 ] ],
176+
[ "FOR i IN [] COLLECT WITH COUNT INTO cnt RETURN cnt", [ 0 ] ],
177+
[ "FOR i IN 1..10 COLLECT AGGREGATE cnt = COUNT() RETURN cnt", [ 10 ] ],
178+
[ "FOR i IN 1..10 COLLECT WITH COUNT INTO cnt RETURN cnt", [ 10 ] ],
179+
];
180+
181+
queries.forEach(function(query) {
182+
let plan = AQL_EXPLAIN(query[0], { }, paramEnabled);
183+
let result = AQL_EXECUTE(query[0], { }, paramEnabled).json;
184+
185+
assertEqual(-1, plan.plan.rules.indexOf(ruleName), query[0]);
186+
assertEqual(result, query[1], query);
142187
});
143188
},
144189

@@ -396,11 +441,6 @@ function optimizerRuleTestSuite () {
396441
};
397442
}
398443

399-
////////////////////////////////////////////////////////////////////////////////
400-
/// @brief executes the test suite
401-
////////////////////////////////////////////////////////////////////////////////
402-
403444
jsunity.run(optimizerRuleTestSuite);
404445

405446
return jsunity.done();
406-

0 commit comments

Comments
 (0)
0