From 03e998c0daf5a1734699c5a442634df5b6d5caed Mon Sep 17 00:00:00 2001 From: mutdmour Date: Sat, 22 Apr 2017 09:15:30 +0300 Subject: [PATCH 1/4] sort overrides $near sort - minimongo (#3599) --- packages/minimongo/minimongo_tests.js | 5 +++-- packages/minimongo/sort.js | 16 +++++++++------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index 21ae3169cc..c03e885995 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -3207,13 +3207,14 @@ Tinytest.add("minimongo - $near operator tests", function (test) { // 'y'. testNear([2, 2], 1000, ['x', 'y']); - // Ensure that distance is used as a tie-breaker for sort. + // issue #3599 + // Ensure that distance are not used as a tie-breaker for sort. test.equal( _.pluck(coll.find({'a.b': {$near: [1, 1]}}, {sort: {k: 1}}).fetch(), '_id'), ['x', 'y']); test.equal( _.pluck(coll.find({'a.b': {$near: [5, 5]}}, {sort: {k: 1}}).fetch(), '_id'), - ['y', 'x']); + ['x', 'y']); var operations = []; var cbs = log_callbacks(operations); diff --git a/packages/minimongo/sort.js b/packages/minimongo/sort.js index 7c90b8f650..aebbcc2d4f 100644 --- a/packages/minimongo/sort.js +++ b/packages/minimongo/sort.js @@ -81,23 +81,25 @@ _.extend(Minimongo.Sorter.prototype, { getComparator: function (options) { var self = this; - // If we have no distances, just use the comparator from the source - // specification (which defaults to "everything is equal". - if (!options || !options.distances) { + // If sort is specified or have no distances, just use the comparator from + // the source specification (which defaults to "everything is equal". + // issue #3599 + // https://docs.mongodb.com/manual/reference/operator/query/near/#sort-operation + // sort effectively overrides $near + if (self._sortSpecParts.length || !options || !options.distances) { return self._getBaseComparator(); } var distances = options.distances; - // Return a comparator which first tries the sort specification, and if that - // says "it's equal", breaks ties using $near distances. - return composeComparators([self._getBaseComparator(), function (a, b) { + // Return a comparator which compares using $near distances. + return function (a, b) { if (!distances.has(a._id)) throw Error("Missing distance for " + a._id); if (!distances.has(b._id)) throw Error("Missing distance for " + b._id); return distances.get(a._id) - distances.get(b._id); - }]); + }; }, _getPaths: function () { From 3ad95d4deff13329846c78fb01528415053293a7 Mon Sep 17 00:00:00 2001 From: mutdmour Date: Wed, 26 Apr 2017 08:35:55 +0300 Subject: [PATCH 2/4] Using $near to query with an update (#3599) --- packages/minimongo/minimongo.js | 2 +- packages/minimongo/minimongo_tests.js | 40 ++++++++++++++++++++++----- packages/minimongo/selector.js | 10 +++++-- 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js index dd2017ebce..92265e6a90 100644 --- a/packages/minimongo/minimongo.js +++ b/packages/minimongo/minimongo.js @@ -699,7 +699,7 @@ LocalCollection.prototype.update = function (selector, mod, options, callback) { } if (!options) options = {}; - var matcher = new Minimongo.Matcher(selector); + var matcher = new Minimongo.Matcher(selector, true); // Save the original results of any query that we might need to // _recomputeResults on, because _modifyAndNotify will mutate the objects in diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index c03e885995..ae8c12867a 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -2301,13 +2301,7 @@ Tinytest.add("minimongo - modify", function (test) { {'a.x': 1, 'a.y': 3}, {$set: {'a.$.z': 5}}, {a: [{x: 1}, {y: 3, z: 5}]}); - // with $near, make sure it finds the closest one - modifyWithQuery({a: [{b: [1,1]}, - {b: [ [3,3], [4,4] ]}, - {b: [9,9]}]}, - {'a.b': {$near: [5, 5]}}, - {$set: {'a.$.b': 'k'}}, - {a: [{b: [1,1]}, {b: 'k'}, {b: [9,9]}]}); + // with $near, make sure it does not find the closest one (#3599) modifyWithQuery({a: [{x: 1}, {y: 1}, {x: 1, y: 1}]}, {a: {$elemMatch: {x: 1, y: 1}}}, {$set: {'a.$.x': 2}}, @@ -2316,6 +2310,38 @@ Tinytest.add("minimongo - modify", function (test) { {'a.b': {$elemMatch: {x: 1, y: 1}}}, {$set: {'a.$.b': 3}}, {a: [{b: 3}]}); + modifyWithQuery({a: []}, + {'a.b': {$near: [5, 5]}}, + {$set: {'a.$.b': 'k'}}, + {"a":[]}); + modifyWithQuery({a: [{b: [ [3,3], [4,4] ]}]}, + {'a.b': {$near: [5, 5]}}, + {$set: {'a.$.b': 'k'}}, + {"a":[{"b":"k"}]}); + modifyWithQuery({a: [{b: [1,1]}, + {b: [ [3,3], [4,4] ]}, + {b: [9,9]}]}, + {'a.b': {$near: [5, 5]}}, + {$set: {'a.$.b': 'k'}}, + {"a":[{"b":"k"},{"b":[[3,3],[4,4]]},{"b":[9,9]}]}); + modifyWithQuery({a: [{b: [1,1]}, + {b: [ [3,3], [4,4] ]}, + {b: [9,9]}]}, + {'a.b': {$near: [9, 9]}}, + {$set: {'a.$.b': 'k'}}, + {"a":[{"b":"k"},{"b":[[3,3],[4,4]]},{"b":[9,9]}]}); + modifyWithQuery({a: [{b: [9,9]}, + {b: [ [3,3], [4,4] ]}, + {b: [9,9]}]}, + {'a.b': {$near: [9, 9]}}, + {$set: {'a.$.b': 'k'}}, + {"a":[{"b":"k"},{"b":[[3,3],[4,4]]},{"b":[9,9]}]}); + modifyWithQuery({a: [{b: [9,9]}, + {b: [ [3,3], [4,4] ]}, + {b: [1,1]}]}, + {'a.b': {$near: [9, 9]}}, + {$set: {'a.$.b': 'k'}}, + {"a":[{"b":"k"},{"b":[[3,3],[4,4]]},{"b":[1,1]}]}); // $inc modify({a: 1, b: 2}, {$inc: {a: 10}}, {a: 11, b: 2}); diff --git a/packages/minimongo/selector.js b/packages/minimongo/selector.js index f3034094d1..355abbf3e8 100644 --- a/packages/minimongo/selector.js +++ b/packages/minimongo/selector.js @@ -18,7 +18,7 @@ // Main entry point. // var matcher = new Minimongo.Matcher({a: {$gt: 5}}); // if (matcher.documentMatches({a: 7})) ... -Minimongo.Matcher = function (selector) { +Minimongo.Matcher = function (selector, isUpdate = false) { var self = this; // A set (object mapping string -> *) of all of the document paths looked // at by the selector. Also includes the empty string if it may look at any @@ -41,6 +41,10 @@ Minimongo.Matcher = function (selector) { // Sorter._useWithMatcher. self._selector = null; self._docMatcher = self._compileSelector(selector); + // Set to true if selection is done for an update operation + // Default is false + // Used for $near array update (issue #3599) + self._isUpdate = isUpdate; }; _.extend(Minimongo.Matcher.prototype, { @@ -497,7 +501,9 @@ var VALUE_OPERATORS = { return; result.result = true; result.distance = curDistance; - if (!branch.arrayIndices) + if (matcher._isUpdate) + result.arrayIndices = [0,0]; + else if (!branch.arrayIndices) delete result.arrayIndices; else result.arrayIndices = branch.arrayIndices; From 9efa22fd8fc49c8c549e985203cf0f477e9f0421 Mon Sep 17 00:00:00 2001 From: mutdmour Date: Thu, 27 Apr 2017 08:49:12 +0300 Subject: [PATCH 3/4] More test cases for update with $near (#3599) --- packages/minimongo/minimongo_tests.js | 27 +++++++++++++++++++----- packages/minimongo/selector.js | 30 +++++++++++++++------------ 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index ae8c12867a..2ad86ad6c4 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -2301,7 +2301,6 @@ Tinytest.add("minimongo - modify", function (test) { {'a.x': 1, 'a.y': 3}, {$set: {'a.$.z': 5}}, {a: [{x: 1}, {y: 3, z: 5}]}); - // with $near, make sure it does not find the closest one (#3599) modifyWithQuery({a: [{x: 1}, {y: 1}, {x: 1, y: 1}]}, {a: {$elemMatch: {x: 1, y: 1}}}, {$set: {'a.$.x': 2}}, @@ -2310,6 +2309,7 @@ Tinytest.add("minimongo - modify", function (test) { {'a.b': {$elemMatch: {x: 1, y: 1}}}, {$set: {'a.$.b': 3}}, {a: [{b: 3}]}); + // with $near, make sure it does not find the closest one (#3599) modifyWithQuery({a: []}, {'a.b': {$near: [5, 5]}}, {$set: {'a.$.b': 'k'}}, @@ -2323,7 +2323,13 @@ Tinytest.add("minimongo - modify", function (test) { {b: [9,9]}]}, {'a.b': {$near: [5, 5]}}, {$set: {'a.$.b': 'k'}}, - {"a":[{"b":"k"},{"b":[[3,3],[4,4]]},{"b":[9,9]}]}); + {"a":[{"b":"k"},{"b":[[3,3],[4,4]]},{"b":[9,9]}]}); + modifyWithQuery({a: [{b: [1,1]}, + {b: [ [3,3], [4,4] ]}, + {b: [9,9]}]}, + {'a.b': {$near: [9, 9], $maxDistance: 1}}, + {$set: {'a.$.b': 'k'}}, + {"a":[{"b":"k"},{"b":[[3,3],[4,4]]},{"b":[9,9]}]}); modifyWithQuery({a: [{b: [1,1]}, {b: [ [3,3], [4,4] ]}, {b: [9,9]}]}, @@ -2336,12 +2342,23 @@ Tinytest.add("minimongo - modify", function (test) { {'a.b': {$near: [9, 9]}}, {$set: {'a.$.b': 'k'}}, {"a":[{"b":"k"},{"b":[[3,3],[4,4]]},{"b":[9,9]}]}); - modifyWithQuery({a: [{b: [9,9]}, + modifyWithQuery({a: [{b:[4,3]}, + {c: [1,1]}]}, + {'a.c': {$near: [1, 1]}}, + {$set: {'a.$.c': 'k'}}, + {"a":[{"c": "k", "b":[4,3]},{"c":[1,1]}]}); + modifyWithQuery({a: [{c: [9,9]}, {b: [ [3,3], [4,4] ]}, {b: [1,1]}]}, - {'a.b': {$near: [9, 9]}}, + {'a.b': {$near: [1, 1]}}, {$set: {'a.$.b': 'k'}}, - {"a":[{"b":"k"},{"b":[[3,3],[4,4]]},{"b":[1,1]}]}); + {"a":[{"c": [9,9], "b":"k"},{"b": [ [3,3], [4,4]]},{"b":[1,1]}]}); + modifyWithQuery({a: [{c: [9,9], b:[4,3]}, + {b: [ [3,3], [4,4] ]}, + {b: [1,1]}]}, + {'a.b': {$near: [1, 1]}}, + {$set: {'a.$.b': 'k'}}, + {"a":[{"c": [9,9], "b":"k"},{"b": [ [3,3], [4,4]]},{"b":[1,1]}]}); // $inc modify({a: 1, b: 2}, {$inc: {a: 10}}, {a: 11, b: 2}); diff --git a/packages/minimongo/selector.js b/packages/minimongo/selector.js index 355abbf3e8..7088fa751a 100644 --- a/packages/minimongo/selector.js +++ b/packages/minimongo/selector.js @@ -488,25 +488,29 @@ var VALUE_OPERATORS = { // each within-$maxDistance branching point. branchedValues = expandArraysInBranches(branchedValues); var result = {result: false}; - _.each(branchedValues, function (branch) { - if (!(typeof branch.value === "object")){ - return; + _.every(branchedValues, function (branch) { + // if operation is an update, don't skip branches, just return the first one (#3599) + if (!matcher._isUpdate){ + if (!(typeof branch.value === "object")){ + return true; + } + var curDistance = distance(branch.value); + // Skip branches that aren't real points or are too far away. + if (curDistance === null || curDistance > maxDistance) + return true; + // Skip anything that's a tie. + if (result.distance !== undefined && result.distance <= curDistance) + return true; } - var curDistance = distance(branch.value); - // Skip branches that aren't real points or are too far away. - if (curDistance === null || curDistance > maxDistance) - return; - // Skip anything that's a tie. - if (result.distance !== undefined && result.distance <= curDistance) - return; result.result = true; result.distance = curDistance; - if (matcher._isUpdate) - result.arrayIndices = [0,0]; - else if (!branch.arrayIndices) + if (!branch.arrayIndices) delete result.arrayIndices; else result.arrayIndices = branch.arrayIndices; + if (matcher._isUpdate) + return false; + return true; }); return result; }; From a061567f4075d9a0620efaaf843ae888ed567a7a Mon Sep 17 00:00:00 2001 From: mutdmour Date: Thu, 27 Apr 2017 08:57:38 +0300 Subject: [PATCH 4/4] comment grammar (#3599) --- packages/minimongo/minimongo_tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index 2ad86ad6c4..8ab18a39c7 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -3251,7 +3251,7 @@ Tinytest.add("minimongo - $near operator tests", function (test) { testNear([2, 2], 1000, ['x', 'y']); // issue #3599 - // Ensure that distance are not used as a tie-breaker for sort. + // Ensure that distance is not used as a tie-breaker for sort. test.equal( _.pluck(coll.find({'a.b': {$near: [1, 1]}}, {sort: {k: 1}}).fetch(), '_id'), ['x', 'y']);