SERVER-35765 Allow small discrepancy in computed geo distances.

4 files changed, 223 insertions, 99 deletions

diff --git a/jstests/aggregation/extras/testutils.js b/jstests/aggregation/extras/testutils.js
deleted file mode 100644
index bd05ea835f0..00000000000
--- a/jstests/aggregation/extras/testutils.js
+++ /dev/null
@@ -1,54 +0,0 @@
-/*
-  Test the test utilities themselves
-*/
-var verbose = false;
-
-var t1result = [
-    {"_id": ObjectId("4dc07fedd8420ab8d0d4066d"), "pageViews": 5, "tags": ["fun", "good"]},
-    {"_id": ObjectId("4dc07fedd8420ab8d0d4066e"), "pageViews": 7, "tags": ["fun", "nasty"]},
-    {"_id": ObjectId("4dc07fedd8420ab8d0d4066f"), "pageViews": 6, "tags": ["nasty", "filthy"]}
-];
-
-assert(arrayEq(t1result, t1result, verbose), 't0a failed');
-assert(resultsEq(t1result, t1result, verbose), 't0b failed');
-
-var t1resultr = [
-    {"_id": ObjectId("4dc07fedd8420ab8d0d4066d"), "pageViews": 5, "tags": ["fun", "good"]},
-    {"_id": ObjectId("4dc07fedd8420ab8d0d4066f"), "pageViews": 6, "tags": ["nasty", "filthy"]},
-    {"_id": ObjectId("4dc07fedd8420ab8d0d4066e"), "pageViews": 7, "tags": ["fun", "nasty"]},
-];
-
-assert(resultsEq(t1resultr, t1result, verbose), 'tr1 failed');
-assert(resultsEq(t1result, t1resultr, verbose), 'tr2 failed');
-
-var t1resultf1 = [
-    {"_id": ObjectId("4dc07fedd8420ab8d0d4066e"), "pageViews": 7, "tags": ["fun", "nasty"]},
-    {"_id": ObjectId("4dc07fedd8420ab8d0d4066f"), "pageViews": 6, "tags": ["nasty", "filthy"]}
-];
-
-assert(!resultsEq(t1result, t1resultf1, verbose), 't1a failed');
-assert(!resultsEq(t1resultf1, t1result, verbose), 't1b failed');
-
-var t1resultf2 = [
-    {"pageViews": 5, "tags": ["fun", "good"]},
-    {"pageViews": 7, "tags": ["fun", "nasty"]},
-    {"pageViews": 6, "tags": ["nasty", "filthy"]}
-];
-
-assert(!resultsEq(t1result, t1resultf2, verbose), 't2a failed');
-assert(!resultsEq(t1resultf2, t1result, verbose), 't2b failed');
-
-var t1resultf3 = [
-    {
-      "_id": ObjectId("4dc07fedd8420ab8d0d4066d"),
-      "pageViews": 5,
-      "tags": [
-          "fun",
-      ]
-    },
-    {"_id": ObjectId("4dc07fedd8420ab8d0d4066e"), "pageViews": 7, "tags": ["fun", "nasty"]},
-    {"_id": ObjectId("4dc07fedd8420ab8d0d4066f"), "pageViews": 6, "tags": ["filthy"]}
-];
-
-assert(!resultsEq(t1result, t1resultf3, verbose), 't3a failed');
-assert(!resultsEq(t1resultf3, t1result, verbose), 't3b failed');
diff --git a/jstests/aggregation/extras/utils.js b/jstests/aggregation/extras/utils.js
index ff2e4c07a68..be246362ac8 100644
--- a/jstests/aggregation/extras/utils.js
+++ b/jstests/aggregation/extras/utils.js
@@ -25,10 +25,10 @@ function testExpressionWithCollation(coll, expression, result, collationSpec) {
 /**
  * Returns true if 'al' is the same as 'ar'. If the two are arrays, the arrays can be in any order.
  * Objects (either 'al' and 'ar' themselves, or embedded objects) must have all the same properties,
- * with the exception of '_id'. If 'al' and 'ar' are neither object nor arrays, they must be the
- * same value.
+ * with the exception of '_id'. If 'al' and 'ar' are neither object nor arrays, they must compare
+ * equal using 'valueComparator', or == if not provided.
  */
-function anyEq(al, ar, verbose = false) {
+function anyEq(al, ar, verbose = false, valueComparator) {
     const debug = msg => verbose ? print(msg) : null;  // Helper to log 'msg' iff 'verbose' is true.
 
     if (al instanceof Array) {
@@ -37,7 +37,7 @@ function anyEq(al, ar, verbose = false) {
             return false;
         }
 
-        if (!arrayEq(al, ar, verbose)) {
+        if (!arrayEq(al, ar, verbose, valueComparator)) {
             debug(`anyEq: arrayEq(al, ar): false; al=${tojson(al)}, ar=${tojson(ar)}`);
             return false;
         }
@@ -49,11 +49,12 @@ function anyEq(al, ar, verbose = false) {
             return false;
         }
 
-        if (!documentEq(al, ar, verbose)) {
+        if (!documentEq(al, ar, verbose, valueComparator)) {
             debug(`anyEq: documentEq(al, ar): false; al=${tojson(al)}, ar=${tojson(ar)}`);
             return false;
         }
-    } else if (al != ar) {
+    } else if ((valueComparator && !valueComparator(al, ar)) || (!valueComparator && al !== ar)) {
+        // Neither an object nor an array, use the custom comparator if provided.
         debug(`anyEq: (al != ar): false; al=${tojson(al)}, ar=${tojson(ar)}`);
         return false;
     }
@@ -63,10 +64,19 @@ function anyEq(al, ar, verbose = false) {
 }
 
 /**
+ * Compares two documents for equality using a custom comparator for the values which returns true
+ * or false. Returns true or false. Only equal if they have the exact same set of properties, and
+ * all the properties' values match according to 'valueComparator'.
+ */
+function customDocumentEq({left, right, verbose, valueComparator}) {
+    return documentEq(left, right, verbose, valueComparator);
+}
+
+/**
  * Compare two documents for equality. Returns true or false. Only equal if they have the exact same
  * set of properties, and all the properties' values match.
  */
-function documentEq(dl, dr, verbose = false) {
+function documentEq(dl, dr, verbose = false, valueComparator) {
     const debug = msg => verbose ? print(msg) : null;  // Helper to log 'msg' iff 'verbose' is true.
 
     // Make sure these are both objects.
@@ -95,7 +105,7 @@ function documentEq(dl, dr, verbose = false) {
         if (propertyName == '_id')
             continue;
 
-        if (!anyEq(dl[propertyName], dr[propertyName], verbose)) {
+        if (!anyEq(dl[propertyName], dr[propertyName], verbose, valueComparator)) {
             return false;
         }
     }
@@ -117,7 +127,7 @@ function documentEq(dl, dr, verbose = false) {
     return true;
 }
 
-function arrayEq(al, ar, verbose = false) {
+function arrayEq(al, ar, verbose = false, valueComparator) {
     const debug = msg => verbose ? print(msg) : null;  // Helper to log 'msg' iff 'verbose' is true.
 
     // Check that these are both arrays.
@@ -136,15 +146,19 @@ function arrayEq(al, ar, verbose = false) {
         return false;
     }
 
-    let i = 0;
-    let j = 0;
-    while (i < al.length) {
-        if (anyEq(al[i], ar[j], verbose)) {
-            j = 0;
-            i++;
-        } else if (j < ar.length) {
-            j++;
-        } else {
+    // Keep a set of which indexes we've already used to avoid considering [1,1] as equal to [1,2].
+    const matchedElementsInRight = new Set();
+    for (let leftElem of al) {
+        let foundMatch = false;
+        for (let i = 0; i < ar.length; ++i) {
+            if (!matchedElementsInRight.has(i) &&
+                anyEq(leftElem, ar[i], verbose, valueComparator)) {
+                matchedElementsInRight.add(i);  // Don't use the same value each time.
+                foundMatch = true;
+                break;
+            }
+        }
+        if (!foundMatch) {
             return false;
         }
     }
diff --git a/jstests/aggregation/sources/geonear/distancefield_and_includelocs.js b/jstests/aggregation/sources/geonear/distancefield_and_includelocs.js
index 540a3f61caf..80e884c2c36 100644
--- a/jstests/aggregation/sources/geonear/distancefield_and_includelocs.js
+++ b/jstests/aggregation/sources/geonear/distancefield_and_includelocs.js
@@ -5,6 +5,8 @@
 (function() {
     "use strict";
 
+    load("jstests/aggregation/extras/utils.js");  // For 'customDocumentEq'.
+
     const coll = db.getCollection("geonear_distancefield_and_includelocs");
     coll.drop();
 
@@ -56,54 +58,72 @@
     assert.writeOK(coll.insert(docWithGeoPoint));
     assert.writeOK(coll.insert(docWithGeoLine));
 
+    // Define a custom way to compare documents since the results here might differ by insignificant
+    // amounts.
+    const assertCloseEnough = (left, right) =>
+        assert(customDocumentEq({
+                   left: left,
+                   right: right,
+                   valueComparator: (a, b) => {
+                       if (typeof a !== "number") {
+                           return a === b;
+                       }
+                       // Allow some minor differences in the numbers.
+                       return Math.abs(a - b) < 1e-10;
+                   }
+               }),
+               () => `[${tojson(left)}] != [${tojson(right)}]`);
+
     [docWithLegacyPoint, docWithGeoPoint, docWithGeoLine].forEach(doc => {
         const docPlusNewFields = (newDoc) => Object.extend(Object.extend({}, doc), newDoc);
 
         //
         // Tests for "distanceField".
         //
-        const expectedDistance = 0;
+        const expectedDistance = 0.0000000000000001;
 
         // Test that "distanceField" can be computed in a new field.
-        assert.docEq(firstGeoNearResult({near: doc.ptForNearQuery, distanceField: "newField"}),
-                     docPlusNewFields({newField: expectedDistance}));
+        assertCloseEnough(firstGeoNearResult({near: doc.ptForNearQuery, distanceField: "newField"}),
+                          docPlusNewFields({newField: expectedDistance}));
 
         // Test that "distanceField" can be computed in a new nested field.
-        assert.docEq(firstGeoNearResult({near: doc.ptForNearQuery, distanceField: "nested.field"}),
-                     docPlusNewFields({nested: {field: expectedDistance}}));
+        assertCloseEnough(
+            firstGeoNearResult({near: doc.ptForNearQuery, distanceField: "nested.field"}),
+            docPlusNewFields({nested: {field: expectedDistance}}));
 
         // Test that "distanceField" can overwrite an existing scalar field.
-        assert.docEq(firstGeoNearResult({near: doc.ptForNearQuery, distanceField: "scalar"}),
-                     docPlusNewFields({scalar: expectedDistance}));
+        assertCloseEnough(firstGeoNearResult({near: doc.ptForNearQuery, distanceField: "scalar"}),
+                          docPlusNewFields({scalar: expectedDistance}));
 
         // Test that "distanceField" can completely overwrite an existing array field.
-        assert.docEq(firstGeoNearResult({near: doc.ptForNearQuery, distanceField: "arr"}),
-                     docPlusNewFields({arr: expectedDistance}));
+        assertCloseEnough(firstGeoNearResult({near: doc.ptForNearQuery, distanceField: "arr"}),
+                          docPlusNewFields({arr: expectedDistance}));
 
         // TODO (SERVER-35561): When "includeLocs" shares a path prefix with an existing field, the
         // fields are overwritten, even if they could be preserved.
-        assert.docEq(firstGeoNearResult({near: doc.ptForNearQuery, distanceField: "arr.b"}),
-                     docPlusNewFields({arr: {b: expectedDistance}}));
+        assertCloseEnough(firstGeoNearResult({near: doc.ptForNearQuery, distanceField: "arr.b"}),
+                          docPlusNewFields({arr: {b: expectedDistance}}));
 
         //
         // Tests for both "includeLocs" and "distanceField".
         //
 
         // Test that "distanceField" and "includeLocs" can both be specified.
-        assert.docEq(firstGeoNearResult(
-                         {near: doc.ptForNearQuery, distanceField: "dist", includeLocs: "loc"}),
-                     docPlusNewFields({dist: expectedDistance, loc: doc.geo}));
+        assertCloseEnough(
+            firstGeoNearResult(
+                {near: doc.ptForNearQuery, distanceField: "dist", includeLocs: "loc"}),
+            docPlusNewFields({dist: expectedDistance, loc: doc.geo}));
 
         // Test that "distanceField" and "includeLocs" can be the same path. The result is arbitrary
         // ("includeLocs" wins).
-        assert.docEq(
+        assertCloseEnough(
             firstGeoNearResult(
                 {near: doc.ptForNearQuery, distanceField: "newField", includeLocs: "newField"}),
             docPlusNewFields({newField: doc.geo}));
 
         // Test that "distanceField" and "includeLocs" are both preserved when their paths share a
         // prefix but do not conflict.
-        assert.docEq(
+        assertCloseEnough(
             firstGeoNearResult(
                 {near: doc.ptForNearQuery, distanceField: "comp.dist", includeLocs: "comp.loc"}),
             docPlusNewFields({comp: {dist: expectedDistance, loc: doc.geo}}));
@@ -114,33 +134,34 @@
         const removeDistFieldProj = {d: 0};
 
         // Test that "includeLocs" can be computed in a new field.
-        assert.docEq(firstGeoNearResult(
-                         {near: doc.ptForNearQuery, distanceField: "d", includeLocs: "newField"},
-                         removeDistFieldProj),
-                     docPlusNewFields({newField: doc.geo}));
+        assertCloseEnough(
+            firstGeoNearResult(
+                {near: doc.ptForNearQuery, distanceField: "d", includeLocs: "newField"},
+                removeDistFieldProj),
+            docPlusNewFields({newField: doc.geo}));
 
         // Test that "includeLocs" can be computed in a new nested field.
-        assert.docEq(
+        assertCloseEnough(
             firstGeoNearResult(
                 {near: doc.ptForNearQuery, distanceField: "d", includeLocs: "nested.field"},
                 removeDistFieldProj),
             docPlusNewFields({nested: {field: doc.geo}}));
 
         // Test that "includeLocs" can overwrite an existing scalar field.
-        assert.docEq(firstGeoNearResult(
-                         {near: doc.ptForNearQuery, distanceField: "d", includeLocs: "scalar"},
-                         removeDistFieldProj),
-                     docPlusNewFields({scalar: doc.geo}));
+        assertCloseEnough(firstGeoNearResult(
+                              {near: doc.ptForNearQuery, distanceField: "d", includeLocs: "scalar"},
+                              removeDistFieldProj),
+                          docPlusNewFields({scalar: doc.geo}));
 
         // Test that "includeLocs" can completely overwrite an existing array field.
-        assert.docEq(
+        assertCloseEnough(
             firstGeoNearResult({near: doc.ptForNearQuery, distanceField: "d", includeLocs: "arr"},
                                removeDistFieldProj),
             docPlusNewFields({arr: doc.geo}));
 
         // TODO (SERVER-35561): When "includeLocs" shares a path prefix with an existing field, the
         // fields are overwritten, even if they could be preserved.
-        assert.docEq(
+        assertCloseEnough(
             firstGeoNearResult({near: doc.ptForNearQuery, distanceField: "d", includeLocs: "arr.a"},
                                removeDistFieldProj),
             docPlusNewFields({arr: {a: doc.geo}}));
diff --git a/jstests/aggregation/testutils.js b/jstests/aggregation/testutils.js
new file mode 100644
index 00000000000..f4c5c1e296a
--- /dev/null
+++ b/jstests/aggregation/testutils.js
@@ -0,0 +1,143 @@
+// Tests the test utilities themselves.
+(function() {
+    load("jstests/aggregation/extras/utils.js");
+
+    const verbose = false;
+
+    const example = [
+        {_id: ObjectId("4dc07fedd8420ab8d0d4066d"), pageViews: 5, tags: ["fun", "good"]},
+        {_id: ObjectId("4dc07fedd8420ab8d0d4066e"), pageViews: 7, tags: ["fun", "nasty"]},
+        {_id: ObjectId("4dc07fedd8420ab8d0d4066f"), pageViews: 6, tags: ["nasty", "filthy"]}
+    ];
+
+    assert(arrayEq(example, example, verbose));
+    assert(resultsEq(example, example, verbose));
+
+    const exampleDifferentOrder = [
+        {_id: ObjectId("4dc07fedd8420ab8d0d4066d"), pageViews: 5, tags: ["fun", "good"]},
+        {_id: ObjectId("4dc07fedd8420ab8d0d4066f"), pageViews: 6, tags: ["nasty", "filthy"]},
+        {_id: ObjectId("4dc07fedd8420ab8d0d4066e"), pageViews: 7, tags: ["fun", "nasty"]},
+    ];
+
+    assert(resultsEq(exampleDifferentOrder, example, verbose));
+    assert(resultsEq(example, exampleDifferentOrder, verbose));
+    assert(!orderedArrayEq(example, exampleDifferentOrder, verbose));
+
+    const exampleFewerEntries = [
+        {_id: ObjectId("4dc07fedd8420ab8d0d4066e"), pageViews: 7, tags: ["fun", "nasty"]},
+        {_id: ObjectId("4dc07fedd8420ab8d0d4066f"), pageViews: 6, tags: ["nasty", "filthy"]}
+    ];
+
+    assert(!resultsEq(example, exampleFewerEntries, verbose));
+    assert(!resultsEq(exampleFewerEntries, example, verbose));
+
+    const exampleNoIds = [
+        {pageViews: 5, tags: ["fun", "good"]},
+        {pageViews: 7, tags: ["fun", "nasty"]},
+        {pageViews: 6, tags: ["nasty", "filthy"]}
+    ];
+
+    assert(!resultsEq(example, exampleNoIds, verbose));
+    assert(!resultsEq(exampleNoIds, example, verbose));
+
+    const exampleMissingTags = [
+        {_id: ObjectId("4dc07fedd8420ab8d0d4066d"), pageViews: 5, tags: ["fun"]},
+        {_id: ObjectId("4dc07fedd8420ab8d0d4066e"), pageViews: 7, tags: ["fun", "nasty"]},
+        {_id: ObjectId("4dc07fedd8420ab8d0d4066f"), pageViews: 6, tags: ["filthy"]}
+    ];
+
+    assert(!resultsEq(example, exampleMissingTags, verbose));
+    assert(!resultsEq(exampleMissingTags, example, verbose));
+
+    const exampleDifferentIds = [
+        {_id: 0, pageViews: 5, tags: ["fun", "good"]},
+        {_id: 1, pageViews: 7, tags: ["fun", "nasty"]},
+        {_id: 2, pageViews: 6, tags: ["nasty", "filthy"]}
+    ];
+    assert(resultsEq(example, exampleDifferentIds));
+    assert(resultsEq(exampleDifferentIds, example));
+
+    // Test using a custom comparator.
+    assert(customDocumentEq({
+        left: {a: 1, b: 3},
+        right: {a: "ignore", b: 3},
+        verbose: verbose,
+        valueComparator: (l, r) => {
+            if (l == "ignore" || r == "ignore") {
+                return true;
+            }
+            return l == r;
+        }
+    }));
+    assert(!customDocumentEq({
+        left: {a: 1, b: 3},
+        right: {a: 3, b: 3},
+        valueComparator: (l, r) => {
+            if (l == "ignore" || r == "ignore") {
+                return true;
+            }
+            return l == r;
+        }
+    }));
+
+    // Test using a custom comparator with arrays.
+    assert(customDocumentEq({
+        left: {a: [1, 2], b: 3},
+        right: {a: [2, "ignore"], b: 3},
+        verbose: verbose,
+        valueComparator: (l, r) => {
+            if (l == "ignore" || r == "ignore") {
+                return true;
+            }
+            return l == r;
+        }
+    }));
+    assert(!customDocumentEq({
+        left: {a: [1, 2], b: 3},
+        right: {a: [3, "ignore"], b: 3},
+        verbose: verbose,
+        valueComparator: (l, r) => {
+            if (l == "ignore" || r == "ignore") {
+                return true;
+            }
+            return l == r;
+        }
+    }));
+
+    // Test using a custom comparator with arrays of objects.
+    assert(customDocumentEq({
+        left: {a: [{b: 1}, {b: 2}, {b: 3}]},
+        right: {a: [{b: "ignore"}, {b: 2}, {b: 3}]},
+        verbose: verbose,
+        valueComparator: (l, r) => {
+            if (l == "ignore" || r == "ignore") {
+                return true;
+            }
+            return l == r;
+        }
+    }));
+    assert(!customDocumentEq({
+        left: {a: [{b: 1}, {b: 2}, {b: 1}]},
+        right: {a: [{b: "ignore"}, {b: 2}, {b: 3}]},
+        verbose: verbose,
+        valueComparator: (l, r) => {
+            if (l == "ignore" || r == "ignore") {
+                return true;
+            }
+            return l == r;
+        }
+    }));
+
+    assert(!anyEq(5, [5], verbose));
+    assert(!anyEq([5], 5, verbose));
+    assert(!anyEq("5", 5, verbose));
+    assert(!anyEq(5, "5", verbose));
+
+    assert(arrayEq([{c: 6}, [5], [4, 5], 2, undefined, 3, null, 4, 5],
+                   [undefined, null, 2, 3, 4, 5, {c: 6}, [4, 5], [5]],
+                   verbose));
+
+    assert(arrayEq([undefined, null, 2, 3, 4, 5, {c: 6}, [4, 5], [5]],
+                   [{c: 6}, [5], [4, 5], 2, undefined, 3, null, 4, 5],
+                   verbose));
+}());