8000 fixed issue #10440: Incorrect sorting with sort criteria partially co… · arangodb/arangodb@dd52f0f · GitHub
[go: up one dir, main page]

Skip to content

Commit dd52f0f

Browse files
jsteemannKVS85
authored andcommitted
fixed issue #10440: Incorrect sorting with sort criteria partially covered by index (#10443)
* fixed issue #10440: Incorrect sorting with sort criteria partially covered by index * Update CHANGELOG
1 parent 343c9ac commit dd52f0f

File tree

4 files changed

+13
-23
lines changed

4 files changed

+13
-23
lines changed

CHANGELOG

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

4+
* Fixed issue #10440: Incorrect sorting with sort criteria partially covered
5+
by index.
6+
47
* Make the timeouts for replication requests (for active failover and master-slave
58
replication configurable via startup options:
69

arangod/Aql/OptimizerRules.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3235,12 +3235,6 @@ struct SortToIndexNode final : public WalkerWorker<ExecutionNode> {
32353235
// sorted GatherNode in the cluster
32363236
indexNode->needsGatherNodeSort(true);
32373237
_modified = true;
3238-
} else if (numCovered > 0 && sortCondition.isUnidirectional()) {
3239-
// remove the first few attributes if they are constant
3240-
SortNode* sortNode =
3241-
ExecutionNode::castTo<SortNode*>(_plan->getNodeById(_sortNode->id()));
3242-
sortNode->removeConditions(numCovered);
3243-
_modified = true;
32443238
}
32453239
}
32463240
}

arangod/Aql/SortCondition.cpp

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -187,25 +187,21 @@ size_t SortCondition::coveredAttributes(
187187
}
188188

189189
// no match
190-
bool isConstant = false;
191-
192190
if (isContained(indexAttributes, field.attributes) &&
193191
isContained(_constAttributes, field.attributes)) {
194192
// no field match, but a constant attribute
195-
isConstant = true;
196193
++fieldsPosition;
197194
++numCovered;
195+
continue;
198196
}
199197

200-
if (!isConstant && isContained(_constAttributes, indexAttributes[i])) {
198+
if (isContained(_constAttributes, indexAttributes[i])) {
201199
// no field match, but a constant attribute
202-
isConstant = true;
203200
++i; // next index field
201+
continue;
204202
}
205203

206-
if (!isConstant) {
207-
break;
208-
}
204+
break;
209205
}
210206

211207
TRI_ASSERT(numCovered <= _fields.size());

tests/js/server/aql/aql-optimizer-rule-use-index-for-sort.js

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -189,17 +189,16 @@ function optimizerRuleTestSuite() {
189189
[ "FOR v IN " + colName + " FILTER v.b == 1 SORT v.b, v.a RETURN 1", false, true ],
190190
[ "FOR v IN " + colName + " FILTER v.a == 1 && v.b == 1 SORT v.b, v.a RETURN 1", true, false ],
191191
[ "FOR v IN " + colName + " FILTER v.a == 1 && v.b == 1 SORT v.a, v.b RETURN 1", true, false ],
192-
[ "FOR v IN " + colName + " FILTER v.a == 1 && v.b == 1 SORT v.a, v.c RETURN 1", true, true ],
192+
[ "FOR v IN " + colName + " FILTER v.a == 1 && v.b == 1 SORT v.a, v.c RETURN 1", false, true ],
193193
[ "FOR v IN " + colName + " FILTER v.a == 1 && v.b == 1 SORT v.b, v.a RETURN 1", true, false ],
194-
[ "FOR v IN " + colName + " FILTER v.a == 1 && v.b == 1 SORT v.a, v.b, v.c RETURN 1", true, true ]
194+
[ "FOR v IN " + colName + " FILTER v.a == 1 && v.b == 1 SORT v.a, v.b, v.c RETURN 1", false, true ]
195195
];
196196

197197
queries.forEach(function(query) {
198198
var result = AQL_EXPLAIN(query[0]);
199199
if (query[1]) {
200200
assertNotEqual(-1, removeAlwaysOnClusterRules(result.plan.rules).indexOf(ruleName), query[0]);
201-
}
202-
else {
201+
} else {
203202
assertEqual(-1, removeAlwaysOnClusterRules(result.plan.rules).indexOf(ruleName), query[0]);
204203
}
205204
if (query[2]) {
@@ -402,7 +401,6 @@ function optimizerRuleTestSuite() {
402401
// place since the index can't fullfill all of the sorting criteria.
403402
////////////////////////////////////////////////////////////////////////////////
404403
testSortMoreThanIndexed: function () {
405-
406404
var query = "FOR v IN " + colName + " FILTER v.a == 1 SORT v.a, v.c RETURN [v.a, v.b, v.c]";
407405
// no index can be used for v.c -> sort has to remain in place!
408406
var XPresult;
@@ -431,7 +429,7 @@ function optimizerRuleTestSuite() {
431429
QResults[2] = AQL_EXECUTE(query, { }, paramIndexFromSort_IndexRange).json;
432430
XPresult = AQL_EXPLAIN(query, { }, paramIndexFromSort_IndexRange);
433431

434-
assertEqual([ ruleName, secondRuleName ], removeAlwaysOnClusterRules(XPresult.plan.rules).sort());
432+
assertEqual([ secondRuleName ], removeAlwaysOnClusterRules(XPresult.plan.rules).sort());
435433
// The sortnode and its calculation node should not have been removed.
436434
hasSortNode(XPresult);
437435
hasCalculationNodes(XPresult, 4);
@@ -1124,7 +1122,7 @@ function optimizerRuleTestSuite() {
11241122
testSortModifyFilterCondition : function () {
11251123
var query = "FOR v IN " + colName + " FILTER v.a == 123 SORT v.a, v.xxx RETURN v";
11261124
var rules = AQL_EXPLAIN(query).plan.rules;
1127-
assertNotEqual(-1, rules.indexOf(ruleName));
1125+
assertEqual(-1, rules.indexOf(ruleName));
11281126
assertNotEqual(-1, rules.indexOf(secondRuleName));
11291127
assertNotEqual(-1, rules.indexOf("remove-filter-covered-by-index"));
11301128

@@ -1135,9 +1133,8 @@ function optimizerRuleTestSuite() {
11351133
++seen;
11361134
assertFalse(node.reverse);
11371135
} else if (node.type === "SortNode") {
1138-
// first sort condition (v.a) should have been removed because it is const
11391136
++seen;
1140-
assertEqual(1, node.elements.length);
1137+
assertEqual(2, node.elements.length);
11411138
}
11421139
});
11431140
assertEqual(2, seen);

0 commit comments

Comments
 (0)
0