From feb52cb8bb13010c808f7849a569adacc3d62d02 Mon Sep 17 00:00:00 2001 From: Matthew Boros Date: Mon, 1 Jul 2024 14:31:57 -0400 Subject: [PATCH] SERVER-91932 Create query result comparator as C++ shell helper (#24082) GitOrigin-RevId: ef542ebbf50acd635ad82f1b83b3010f849dc9ec --- .eslintrc.yml | 2 +- jstests/core/shell/result_comparison.js | 142 ++++++++++++++++++ .../property_test_helpers/query_properties.js | 6 +- jstests/noPassthrough/query_property_tests.js | 2 +- src/mongo/shell/shell_utils.cpp | 109 +++++++++++++- src/mongo/shell/utils.js | 33 ---- 6 files changed, 255 insertions(+), 39 deletions(-) create mode 100644 jstests/core/shell/result_comparison.js diff --git a/.eslintrc.yml b/.eslintrc.yml index fd6ac955b3b..8ae3e533366 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -147,6 +147,7 @@ globals: indentStr: true _forgetReplSet: true _fnvHashToHexString: true + _resultSetsEqualUnordered: true getStringWidth: true # likely could be replaced with `path` @@ -205,7 +206,6 @@ globals: bsonUnorderedFieldsCompare: true bsonBinaryEqual: true friendlyEqual: true - unorderedFriendlyEqual: true timestampCmp: true decompressBSONColumn: true diff --git a/jstests/core/shell/result_comparison.js b/jstests/core/shell/result_comparison.js new file mode 100644 index 00000000000..25575894049 --- /dev/null +++ b/jstests/core/shell/result_comparison.js @@ -0,0 +1,142 @@ +/* + * Tests `_resultSetsEqualUnordered`, which compares two sets of results (order of documents is + * disregarded) for equality. Field order inside an object is ignored, but array ordering and + * everything else is required for equality. + */ + +const currentDate = new Date(); + +// We should throw for invalid input. This function expects both arguments to be a list of objects. +assert.throwsWithCode(() => _resultSetsEqualUnordered({}, []), 9193201); +assert.throwsWithCode(() => _resultSetsEqualUnordered([], 5), 9193201); +assert.throwsWithCode(() => _resultSetsEqualUnordered([4, 1], []), 9193202); +assert.throwsWithCode(() => _resultSetsEqualUnordered([], ["abc"]), 9193203); +assert.throwsWithCode(() => _resultSetsEqualUnordered([[]], [{a: 1}]), 9193202); +assert.throwsWithCode(() => _resultSetsEqualUnordered([], undefined), 9193201); +assert.throwsWithCode(() => _resultSetsEqualUnordered([], null), 9193201); +assert.throwsWithCode(() => _resultSetsEqualUnordered([null], []), 9193202); + +// Some base cases. +assert(_resultSetsEqualUnordered([], [])); +assert(_resultSetsEqualUnordered([{a: 1}], [{a: 1}])); +assert(_resultSetsEqualUnordered([{a: 1}, {a: 1}], [{a: 1}, {a: 1}])); +assert(_resultSetsEqualUnordered([{a: 1}, {b: 1}], [{b: 1}, {a: 1}])); +assert(_resultSetsEqualUnordered([{a: null}], [{a: null}])); +assert(!_resultSetsEqualUnordered([], [{a: 1}])); +assert(!_resultSetsEqualUnordered([{a: 1}], [])); +// Different types should fail the comparison. +assert(!_resultSetsEqualUnordered([{a: 1}], [{a: '1'}])); +assert(!_resultSetsEqualUnordered([{a: 1}], [{a: NumberLong(1)}])); +assert(!_resultSetsEqualUnordered([{a: 1}], [{a: NumberDecimal(1)}])); +assert(!_resultSetsEqualUnordered([{a: NumberInt(1)}], [{a: NumberDecimal(1)}])); +assert(!_resultSetsEqualUnordered([{a: NumberInt(1)}], [{a: NumberLong(1)}])); +assert(!_resultSetsEqualUnordered([{a: null}], [{}])); +assert(!_resultSetsEqualUnordered([{a: null}], [{b: null}])); +assert(!_resultSetsEqualUnordered([{a: null}], [{a: undefined}])); +assert(!_resultSetsEqualUnordered([{}], [{a: undefined}])); + +/* + * Given two sets of results - `equalResults` and `differentResults`, we test that all pairs of + * results in `equalResults` are equal to each other. We also test that pairs of one result from + * `equalResults` and one result from `differentResults` are unequal. + */ +function assertExpectedOutputs(equalResults, differentResults) { + for (const result1 of equalResults) { + for (const result2 of equalResults) { + assert(_resultSetsEqualUnordered(result1, result2), {result1, result2}); + assert(_resultSetsEqualUnordered(result2, result1), {result1, result2}); + } + } + for (const result1 of equalResults) { + for (const result2 of differentResults) { + assert(!_resultSetsEqualUnordered(result1, result2), {result1, result2}); + assert(!_resultSetsEqualUnordered(result2, result1), {result1, result2}); + } + } +} + +function testIndividualDocumentEquality() { + const doc = { + a: 1, + b: [ + {x: "a string", y: currentDate, z: NumberDecimal(1)}, + {'a1.b1': 5, 'a1.c1': 2, 'a2': [3, 2, 1]} + ] + }; + const docOutOfOrder = { + b: [ + {z: NumberDecimal(1), x: "a string", y: currentDate}, + {'a1.b1': 5, 'a2': [3, 2, 1], 'a1.c1': 2} + ], + a: 1 + }; + + // We change the order of arrays here - our comparator should return false, because arrays need + // to be ordered. + const docAltered1 = { + a: 1, + b: [ + {z: NumberDecimal(1), x: "a string", y: currentDate}, + {'a1.b1': 5, 'a1.c1': 2, 'a2': [1, 2, 3]} + ] + }; + const docAltered2 = { + a: 1, + b: [ + {'a1.b1': 5, 'a1.c1': 2, 'a2': [3, 2, 1]}, + {z: NumberDecimal(1), x: "a string", y: currentDate} + ] + }; + // Change a few values, which should also make our comparator return false. + const docAltered3 = { + a: 1, + b: [ + {x: "a string", y: currentDate, z: NumberDecimal(2)}, + {'a1.b1': 5, 'a1.c1': 2, 'a2': [3, 2, 1]} + ] + }; + const docAltered4 = { + a: 1, + b: [ + {x: "a string", y: currentDate, z: NumberDecimal(1)}, + {'a1.b1': 5, 'a1.c1': 2, 'a2': [3, 3, 1]} + ] + }; + const docAltered5 = { + a: 1, + b: [ + {x: "a different string", y: currentDate, z: NumberDecimal(1)}, + {'a1.b1': 5, 'a1.c1': 2, 'a2': [3, 2, 1]} + ] + }; + + // Each result contains one document for this case. + const equalDocs = [[doc], [docOutOfOrder]]; + const unequalDocs = [[docAltered1], [docAltered2], [docAltered3], [docAltered4], [docAltered5]]; + assertExpectedOutputs(equalDocs, unequalDocs); +} + +function testResultOrderIndifference() { + const result = [{a: 1}, {a: 1, b: 1}, {a: 1, b: 1}, {b: 1}, {c: 1}]; + // Different order of documents. + const resultOutOfOrder = [{b: 1, a: 1}, {c: 1}, {a: 1}, {b: 1}, {a: 1, b: 1}]; + // Change the values, or have completely different documents in the result. + const resultAltered1 = [{a: 1, b: 1}, {d: 1}, {a: 1}, {b: 1}, {a: 1, b: 1}]; + const resultAltered2 = [{a: 1, b: 2}, {c: 1}, {a: 1}, {b: 1}, {a: 1, b: 1}]; + const resultAltered3 = [{a: 1, b: 2}, {c: 1}, {a: 1}, {b: 2}, {a: 1, b: 1}]; + const resultAltered4 = [{a: 1, b: 1}, {c: 1}, {a: 1}, {b: 1}, {'a.a': 1, b: 1}]; + const resultAltered5 = [{a: 1, b: 1}, {c: 1}, {a: 1}, {b: 1}, {a: '1', b: 1}]; + const resultAltered6 = [{a: 1, b: 1}, {c: 1}, {a: 1, b: 1}, {b: 1}, {a: 1, b: 1}]; + + assertExpectedOutputs([result, resultOutOfOrder], [ + resultAltered1, + resultAltered2, + resultAltered3, + resultAltered4, + resultAltered5, + resultAltered6 + ]); +} + +testIndividualDocumentEquality(); +testResultOrderIndifference(); diff --git a/jstests/libs/property_test_helpers/query_properties.js b/jstests/libs/property_test_helpers/query_properties.js index 51306f64f4a..449e8586f7c 100644 --- a/jstests/libs/property_test_helpers/query_properties.js +++ b/jstests/libs/property_test_helpers/query_properties.js @@ -152,13 +152,13 @@ export const propertyTests = [ propertyFn: queryHasSameResultsAsControlCollScan, aggModel: aggPipelineModel, numQueriesNeeded: 10, - numRuns: 100 + numRuns: 300 }, { propertyFn: repeatQueriesReturnSameResults, aggModel: aggPipelineModel, numQueriesNeeded: 1, - numRuns: 50 + numRuns: 100 }, { propertyFn: repeatQueriesUseCache, @@ -170,7 +170,7 @@ export const propertyTests = [ propertyFn: cachedQueriesHaveSameResultsAsControlCollScan, aggModel: aggPipelineModel, numQueriesNeeded: 10, - numRuns: 50 + numRuns: 100 }, { propertyFn: identicalQueryCreatesAtMostOneCacheEntry, diff --git a/jstests/noPassthrough/query_property_tests.js b/jstests/noPassthrough/query_property_tests.js index 26da9123926..040fd3b0ce0 100644 --- a/jstests/noPassthrough/query_property_tests.js +++ b/jstests/noPassthrough/query_property_tests.js @@ -71,7 +71,7 @@ function runProperty(propertyFn, isTs, indexes, pipelines) { experimentColl.createIndex(index.def, index.options); } const testHelpers = { - comp: unorderedFriendlyEqual, + comp: _resultSetsEqualUnordered, experimentDb, controlColl, getPlanCache, diff --git a/src/mongo/shell/shell_utils.cpp b/src/mongo/shell/shell_utils.cpp index d343cc5b4f7..ecc9bc2c2ff 100644 --- a/src/mongo/shell/shell_utils.cpp +++ b/src/mongo/shell/shell_utils.cpp @@ -61,6 +61,7 @@ #include "mongo/base/status.h" #include "mongo/base/status_with.h" #include "mongo/bson/bsonmisc.h" +#include "mongo/bson/bsonobj_comparator.h" #include "mongo/bson/bsonobjbuilder.h" #include "mongo/bson/bsontypes.h" #include "mongo/client/client_api_version_parameters_gen.h" @@ -563,7 +564,10 @@ BSONObj numberDecimalsEqual(const BSONObj& input, void*) { BSONObj numberDecimalsAlmostEqual(const BSONObj& input, void*) { if (input.nFields() != 3) { - return BSON("" << false); + uassert(9193200, + "numberDecimalsAlmostEqual expects three arguments, two NumberDecimal inputs and an" + "integer for how many decimal places to check.", + input.nFields() == 3); } BSONObjIterator i(input); @@ -782,6 +786,108 @@ BSONObj _fnvHashToHexString(const BSONObj& args, void*) { return BSON("" << fmt::format("{0:x}", hashed)); } +void sortBSONObjectInternallyHelper(const BSONObj& input, BSONObjBuilder& bob); + +// Helper for `sortBSONObjectInternally`, handles a BSONElement for different recursion cases. +void sortBSONElementInternally(const BSONElement& el, BSONObjBuilder& bob) { + if (el.type() == BSONType::Array) { + BSONObjBuilder sub(bob.subarrayStart(el.fieldNameStringData())); + for (const auto& child : el.Array()) { + sortBSONElementInternally(child, sub); + } + sub.doneFast(); + } else if (el.type() == BSONType::Object) { + BSONObjBuilder sub(bob.subobjStart(el.fieldNameStringData())); + sortBSONObjectInternallyHelper(el.Obj(), sub); + sub.doneFast(); + } else { + bob.append(el); + } +} + +void sortBSONObjectInternallyHelper(const BSONObj& input, BSONObjBuilder& bob) { + BSONObjIteratorSorted it(input); + while (it.more()) { + sortBSONElementInternally(it.next(), bob); + } +} + +// Returns a new BSON with the same field/value pairings, but is recursively sorted by the fields. +// Arrays are not changed. +BSONObj sortBSONObjectInternally(const BSONObj& input) { + BSONObjBuilder bob(input.objsize()); + sortBSONObjectInternallyHelper(input, bob); + return bob.obj(); +} + +// Sorts a vector of BSON objects by their fields as they appear in the BSON. +void sortQueryResults(std::vector& input) { + std::sort(input.begin(), input.end(), [&](const BSONObj& lhs, const BSONObj& rhs) { + return SimpleBSONObjComparator::kInstance.evaluate(lhs < rhs); + }); +} + +/* + * Takes two arrays of documents, and returns whether they contain the same set of BSON Objects. The + * BSON do not need to be in the same order for this to return true. Has no special logic for + * handling double/NumberDecimal closeness. + */ +BSONObj _resultSetsEqualUnordered(const BSONObj& input, void*) { + BSONObjIterator i(input); + auto first = i.next(); + auto second = i.next(); + uassert(9193201, + str::stream() << "_resultSetsEqualUnordered expects two arrays of containing objects " + "as input received " + << first.type() << " and " << second.type(), + first.type() == BSONType::Array && second.type() == BSONType::Array); + + auto firstAsBson = first.Array(); + auto secondAsBson = second.Array(); + + for (const auto& el : firstAsBson) { + uassert(9193202, + str::stream() << "_resultSetsEqualUnordered expects all elements of input arrays " + "to be objects, received " + << el.type(), + el.type() == BSONType::Object); + } + for (const auto& el : secondAsBson) { + uassert(9193203, + str::stream() << "_resultSetsEqualUnordered expects all elements of input arrays " + "to be objects, received " + << el.type(), + el.type() == BSONType::Object); + } + + if (firstAsBson.size() != secondAsBson.size()) { + return BSON("" << false); + } + + // Optimistically assume they're already in the same order. + if (first.binaryEqualValues(second)) { + return BSON("" << true); + } + + std::vector firstSorted; + std::vector secondSorted; + for (size_t i = 0; i < firstAsBson.size(); i++) { + firstSorted.push_back(sortBSONObjectInternally(firstAsBson[i].Obj())); + secondSorted.push_back(sortBSONObjectInternally(secondAsBson[i].Obj())); + } + + sortQueryResults(firstSorted); + sortQueryResults(secondSorted); + + for (size_t i = 0; i < firstSorted.size(); i++) { + if (!firstSorted[i].binaryEqual(secondSorted[i])) { + return BSON("" << false); + } + } + + return BSON("" << true); +} + void installShellUtils(Scope& scope) { scope.injectNative("getMemInfo", JSGetMemInfo); scope.injectNative("_createSecurityToken", _createSecurityToken); @@ -804,6 +910,7 @@ void installShellUtils(Scope& scope) { scope.injectNative("_closeGoldenData", _closeGoldenData); scope.injectNative("_buildBsonObj", _buildBsonObj); scope.injectNative("_fnvHashToHexString", _fnvHashToHexString); + scope.injectNative("_resultSetsEqualUnordered", _resultSetsEqualUnordered); installShellUtilsLauncher(scope); installShellUtilsExtended(scope); diff --git a/src/mongo/shell/utils.js b/src/mongo/shell/utils.js index b96f6458ba9..083b8652346 100644 --- a/src/mongo/shell/utils.js +++ b/src/mongo/shell/utils.js @@ -267,39 +267,6 @@ friendlyEqual = function(a, b) { return false; }; -// Takes two arrays of documents, and returns whether they contain the same set of documents under -// very lenient conditions. The documents do not need to be in the same order for this to return -// true. Arrays are treated as sets (order is disregarded). Do not use this comparator unless all of -// these behaviors are necessary. -unorderedFriendlyEqual = function(result1, result2) { - // Sort the objects fields recursively. - const orderFn = function(doc) { - const keys = Object.keys(doc); - keys.sort(); - let newDoc = {}; - for (const key of keys) { - // Recurse through arrays and objects. - if (doc[key] instanceof Object) { - newDoc[key] = orderFn(doc[key]); - } else { - newDoc[key] = doc[key]; - } - } - return newDoc; - }; - result1 = result1.map(orderFn); - result2 = result2.map(orderFn); - - const cmpFn = function(doc1, doc2) { - const doc1Json = tojson(doc1); - const doc2Json = tojson(doc2); - return doc1Json < doc2Json ? -1 : (doc1Json > doc2Json ? 1 : 0); - }; - result1.sort(cmpFn); - result2.sort(cmpFn); - return friendlyEqual(result1, result2); -}; - printStackTrace = function() { try { throw new Error("Printing Stack Trace");