Skip to content

Commit 7319562

Browse files
NotBobTheBuilderflovilmart
authored andcommitted
Alleviate SERVER-13732 on all top level filters (#3564)
In a prior commit, improvements were made to the addition of `_rperm` in the case of `$or` queries, to avoid MongoDB bug SERVER-13732. As the vast majority of $or queries previously hit this bug due to the presence of `_rperm` on most Parse queries), the present solution avoids the bug and improves query performance in most cases. However, it's still possible for clients to supply their own queries which hit that bug, such as those with `_created_at` or `_updated_at` filters, or their own properties from their data model. This commit makes the logic currently present for `_rperm` available to all top level filters that exist alongside an $or query, meaning SERVER-13732 should be avoided in all cases where keys at the top and inner levels do not have name clashes. - #3476 - https://jira.mongodb.org/browse/SERVER-13732
1 parent 053bbef commit 7319562

File tree

2 files changed

+55
-8
lines changed

2 files changed

+55
-8
lines changed

spec/DatabaseController.spec.js

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
var DatabaseController = require('../src/Controllers/DatabaseController.js');
2+
var validateQuery = DatabaseController._validateQuery;
3+
4+
describe('DatabaseController', function() {
5+
6+
describe('validateQuery', function() {
7+
8+
it('should restructure simple cases of SERVER-13732', (done) => {
9+
var query = {$or: [{a: 1}, {a: 2}], _rperm: {$in: ['a', 'b']}, foo: 3};
10+
validateQuery(query);
11+
expect(query).toEqual({$or: [{a: 1, _rperm: {$in: ['a', 'b']}, foo: 3},
12+
{a: 2, _rperm: {$in: ['a', 'b']}, foo: 3}]});
13+
done();
14+
});
15+
16+
it('should reject invalid queries', (done) => {
17+
expect(() => validateQuery({$or: {'a': 1}})).toThrow();
18+
done();
19+
});
20+
21+
it('should accept valid queries', (done) => {
22+
expect(() => validateQuery({$or: [{'a': 1}, {'b': 2}]})).not.toThrow();
23+
done();
24+
});
25+
26+
});
27+
28+
});

src/Controllers/DatabaseController.js

+27-8
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,7 @@ function addWriteACL(query, acl) {
1818
function addReadACL(query, acl) {
1919
const newQuery = _.cloneDeep(query);
2020
//Can't be any existing '_rperm' query, we don't allow client queries on that, no need to $and
21-
if (newQuery.hasOwnProperty('$or')) {
22-
newQuery.$or = newQuery.$or.map(function(qobj) {
23-
qobj._rperm = {'$in' : [null, '*', ...acl]};
24-
return qobj;
25-
});
26-
} else {
27-
newQuery._rperm = { "$in" : [null, "*", ...acl]};
28-
}
21+
newQuery._rperm = {"$in": [null, "*", ...acl]};
2922
return newQuery;
3023
}
3124

@@ -63,6 +56,30 @@ const validateQuery = query => {
6356
if (query.$or) {
6457
if (query.$or instanceof Array) {
6558
query.$or.forEach(validateQuery);
59+
60+
/* In MongoDB, $or queries which are not alone at the top level of the
61+
* query can not make efficient use of indexes due to a long standing
62+
* bug known as SERVER-13732.
63+
*
64+
* This block restructures queries in which $or is not the sole top
65+
* level element by moving all other top-level predicates inside every
66+
* subdocument of the $or predicate, allowing MongoDB's query planner
67+
* to make full use of the most relevant indexes.
68+
*
69+
* EG: {$or: [{a: 1}, {a: 2}], b: 2}
70+
* Becomes: {$or: [{a: 1, b: 2}, {a: 2, b: 2}]}
71+
*
72+
* https://jira.mongodb.org/browse/SERVER-13732
73+
*/
74+
Object.keys(query).forEach(key => {
75+
const noCollisions = !query.$or.some(subq => subq.hasOwnProperty(key))
76+
if (key != '$or' && noCollisions) {
77+
query.$or.forEach(subquery => {
78+
subquery[key] = query[key];
79+
});
80+
delete query[key];
81+
}
82+
});
6683
} else {
6784
throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Bad $or format - use an array value.');
6885
}
@@ -919,4 +936,6 @@ function joinTableName(className, key) {
919936
return `_Join:${key}:${className}`;
920937
}
921938

939+
// Expose validateQuery for tests
940+
DatabaseController._validateQuery = validateQuery;
922941
module.exports = DatabaseController;

0 commit comments

Comments
 (0)