Skip to content

Commit

Permalink
Alleviate SERVER-13732 on all top level filters (#3564)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
NotBobTheBuilder authored and flovilmart committed Feb 26, 2017
1 parent 053bbef commit 7319562
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 8 deletions.
28 changes: 28 additions & 0 deletions spec/DatabaseController.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
var DatabaseController = require('../src/Controllers/DatabaseController.js');
var validateQuery = DatabaseController._validateQuery;

describe('DatabaseController', function() {

describe('validateQuery', function() {

it('should restructure simple cases of SERVER-13732', (done) => {
var query = {$or: [{a: 1}, {a: 2}], _rperm: {$in: ['a', 'b']}, foo: 3};
validateQuery(query);
expect(query).toEqual({$or: [{a: 1, _rperm: {$in: ['a', 'b']}, foo: 3},
{a: 2, _rperm: {$in: ['a', 'b']}, foo: 3}]});
done();
});

it('should reject invalid queries', (done) => {
expect(() => validateQuery({$or: {'a': 1}})).toThrow();
done();
});

it('should accept valid queries', (done) => {
expect(() => validateQuery({$or: [{'a': 1}, {'b': 2}]})).not.toThrow();
done();
});

});

});
35 changes: 27 additions & 8 deletions src/Controllers/DatabaseController.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,7 @@ function addWriteACL(query, acl) {
function addReadACL(query, acl) {
const newQuery = _.cloneDeep(query);
//Can't be any existing '_rperm' query, we don't allow client queries on that, no need to $and
if (newQuery.hasOwnProperty('$or')) {
newQuery.$or = newQuery.$or.map(function(qobj) {
qobj._rperm = {'$in' : [null, '*', ...acl]};
return qobj;
});
} else {
newQuery._rperm = { "$in" : [null, "*", ...acl]};
}
newQuery._rperm = {"$in": [null, "*", ...acl]};
return newQuery;
}

Expand Down Expand Up @@ -63,6 +56,30 @@ const validateQuery = query => {
if (query.$or) {
if (query.$or instanceof Array) {
query.$or.forEach(validateQuery);

/* In MongoDB, $or queries which are not alone at the top level of the
* query can not make efficient use of indexes due to a long standing
* bug known as SERVER-13732.
*
* This block restructures queries in which $or is not the sole top
* level element by moving all other top-level predicates inside every
* subdocument of the $or predicate, allowing MongoDB's query planner
* to make full use of the most relevant indexes.
*
* EG: {$or: [{a: 1}, {a: 2}], b: 2}
* Becomes: {$or: [{a: 1, b: 2}, {a: 2, b: 2}]}
*
* https://jira.mongodb.org/browse/SERVER-13732
*/
Object.keys(query).forEach(key => {
const noCollisions = !query.$or.some(subq => subq.hasOwnProperty(key))
if (key != '$or' && noCollisions) {
query.$or.forEach(subquery => {
subquery[key] = query[key];
});
delete query[key];
}
});
} else {
throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Bad $or format - use an array value.');
}
Expand Down Expand Up @@ -919,4 +936,6 @@ function joinTableName(className, key) {
return `_Join:${key}:${className}`;
}

// Expose validateQuery for tests
DatabaseController._validateQuery = validateQuery;
module.exports = DatabaseController;

0 comments on commit 7319562

Please sign in to comment.