diff options
4 files changed, 166 insertions, 5 deletions
diff --git a/jstests/aggregation/sources/graphLookup/collation_graphlookup.js b/jstests/aggregation/sources/graphLookup/collation_graphlookup.js index 7b457289cc6..7838bd63bb5 100644 --- a/jstests/aggregation/sources/graphLookup/collation_graphlookup.js +++ b/jstests/aggregation/sources/graphLookup/collation_graphlookup.js @@ -42,7 +42,8 @@ assert.eq("erica", res[0].username); assert.eq(2, res[0].friendUsers.length); - // Negative test: ensure that we don't find any friends when the collation is simple. + // Negative test: The local collation does not have a default collation, and so we use the + // simple collation. Ensure that we don't find any friends when the collation is simple. res = coll.aggregate([ {$match: {username: "erica"}}, { @@ -60,6 +61,39 @@ assert.eq("erica", res[0].username); assert.eq(0, res[0].friendUsers.length); + // Create a foreign collection with a case-insensitive default collation. + foreignColl.drop(); + assert.commandWorked(db.createCollection(foreignColl.getName(), caseInsensitiveUS)); + assert.writeOK(foreignColl.insert([{username: "JEREMY"}, {username: "JIMMY"}])); + + // Insert some more documents into the local collection. + assert.writeOK(coll.insert({username: "fiona", friends: ["jeremy"]})); + assert.writeOK(coll.insert({username: "gretchen", friends: ["jimmy"]})); + + // Test that $graphLookup uses the simple collation in the case where the collection on which it + // is run does not have a default collation, and that this collation is used instead of the + // default collation of the foreign collection. Exercises the fix for SERVER-43350. + res = coll.aggregate([ + {$match: {username: {$in: ["erica", "fiona", "gretchen"]}}}, + { + $graphLookup: { + from: foreignColl.getName(), + startWith: "$friends", + connectFromField: "friends", + connectToField: "username", + as: "friendUsers" + } + } + ]) + .toArray(); + assert.eq(3, res.length, tojson(res)); + for (let i = 0; i < res.length; ++i) { + assert(["erica", "fiona", "gretchen"].includes(res[i].username)); + assert.eq(0, res[i].friendUsers.length); + } + + // Recreate the local collection with a case-insensitive default collation, and the foreign + // collection with a case-sensitive default collation. coll.drop(); assert.commandWorked(db.createCollection(coll.getName(), caseInsensitiveUS)); assert.writeOK(coll.insert({username: "erica", friends: ["jeremy", "jimmy"]})); diff --git a/jstests/aggregation/sources/lookup/lookup_collation.js b/jstests/aggregation/sources/lookup/lookup_collation.js new file mode 100644 index 00000000000..98fc0c073a0 --- /dev/null +++ b/jstests/aggregation/sources/lookup/lookup_collation.js @@ -0,0 +1,109 @@ +/** + * Tests that $lookup respects the user-specified collation or the inherited local collation + * when performing comparisons on a foreign collection with a different default collation. Exercises + * the fix for SERVER-43350. + * @tags: [assumes_unsharded_collection] + */ +(function() { + + "use strict"; + + const testDB = db.getSiblingDB(jsTestName()); + const noCollationColl = testDB.no_collation; + const caseInsensitiveColl = testDB.case_insensitive; + + caseInsensitiveColl.drop(); + noCollationColl.drop(); + + const caseInsensitiveCollation = {locale: "en_US", strength: 1}; + + const simpleCollation = {locale: "simple"}; + + assert.commandWorked(testDB.createCollection(noCollationColl.getName())); + assert.commandWorked(testDB.createCollection(caseInsensitiveColl.getName(), + {collation: caseInsensitiveCollation})); + + assert.writeOK( + noCollationColl.insert([{_id: "a"}, {_id: "b"}, {_id: "c"}, {_id: "d"}, {_id: "e"}])); + assert.writeOK( + caseInsensitiveColl.insert([{_id: "a"}, {_id: "B"}, {_id: "c"}, {_id: "D"}, {_id: "e"}])); + + const lookupWithPipeline = (foreignColl) => { + return { + $lookup: { + from: foreignColl.getName(), + as: "foreignMatch", + let : {l_id: "$_id"}, + pipeline: [{$match: {$expr: {$eq: ["$_id", "$$l_id"]}}}] + } + }; + }; + const lookupNoPipeline = (foreignColl) => { + return { + $lookup: { + from: foreignColl.getName(), + localField: "_id", + foreignField: "_id", + as: "foreignMatch" + } + }; + }; + + for (let lookupInto of[lookupWithPipeline, lookupNoPipeline]) { + // Verify that a $lookup whose local collection has no default collation uses the simple + // collation for comparisons on a foreign collection with a non-simple default collation. + let results = noCollationColl.aggregate([lookupInto(caseInsensitiveColl)]).toArray(); + assert.docEq(results, [ + {_id: "a", foreignMatch: [{_id: "a"}]}, + {_id: "b", foreignMatch: []}, + {_id: "c", foreignMatch: [{_id: "c"}]}, + {_id: "d", foreignMatch: []}, + {_id: "e", foreignMatch: [{_id: "e"}]} + ]); + + // Verify that a $lookup whose local collection has no default collation but which is + // running in + // a pipeline with a non-simple user-specified collation uses the latter for comparisons on + // the + // foreign collection. + results = + noCollationColl + .aggregate([lookupInto(caseInsensitiveColl)], {collation: caseInsensitiveCollation}) + .toArray(); + assert.docEq(results, [ + {_id: "a", foreignMatch: [{_id: "a"}]}, + {_id: "b", foreignMatch: [{_id: "B"}]}, + {_id: "c", foreignMatch: [{_id: "c"}]}, + {_id: "d", foreignMatch: [{_id: "D"}]}, + {_id: "e", foreignMatch: [{_id: "e"}]} + ]); + + // Verify that a $lookup whose local collection has a non-simple collation uses the latter + // for + // comparisons on a foreign collection with no default collation. + results = caseInsensitiveColl.aggregate([lookupInto(noCollationColl)]).toArray(); + assert.docEq(results, [ + {_id: "a", foreignMatch: [{_id: "a"}]}, + {_id: "B", foreignMatch: [{_id: "b"}]}, + {_id: "c", foreignMatch: [{_id: "c"}]}, + {_id: "D", foreignMatch: [{_id: "d"}]}, + {_id: "e", foreignMatch: [{_id: "e"}]} + ]); + + // Verify that a $lookup whose local collection has a non-simple collation but which is + // running + // in a pipeline with a user-specified simple collation uses the latter for comparisons on + // the + // foreign collection. + results = caseInsensitiveColl + .aggregate([lookupInto(noCollationColl)], {collation: simpleCollation}) + .toArray(); + assert.docEq(results, [ + {_id: "a", foreignMatch: [{_id: "a"}]}, + {_id: "B", foreignMatch: []}, + {_id: "c", foreignMatch: [{_id: "c"}]}, + {_id: "D", foreignMatch: []}, + {_id: "e", foreignMatch: [{_id: "e"}]} + ]); + } +})();
\ No newline at end of file diff --git a/src/mongo/db/pipeline/document_source_graph_lookup.cpp b/src/mongo/db/pipeline/document_source_graph_lookup.cpp index c12f9e0b04d..f22eebb0f04 100644 --- a/src/mongo/db/pipeline/document_source_graph_lookup.cpp +++ b/src/mongo/db/pipeline/document_source_graph_lookup.cpp @@ -471,11 +471,20 @@ DocumentSourceGraphLookUp::DocumentSourceGraphLookUp( _cache(pExpCtx->getValueComparator()), _unwind(unwindSrc) { const auto& resolvedNamespace = pExpCtx->getResolvedNamespace(_from); - _fromExpCtx = pExpCtx->copyWith(resolvedNamespace.ns); - _fromPipeline = resolvedNamespace.pipeline; + + // We always set an explicit collator on the copied expression context, even if the collator is + // null (i.e. the simple collation). Otherwise, in the situation where the user did not specify + // a collation and the simple collation was inherited from the local collection, the execution + // machinery will incorrectly interpret the null collator and empty user collation as an + // indication that it should inherit the foreign collection's collation. + _fromExpCtx = + expCtx->copyWith(resolvedNamespace.ns, + boost::none, + expCtx->getCollator() ? expCtx->getCollator()->clone() : nullptr); // We append an additional BSONObj to '_fromPipeline' as a placeholder for the $match stage // we'll eventually construct from the input document. + _fromPipeline = resolvedNamespace.pipeline; _fromPipeline.reserve(_fromPipeline.size() + 1); _fromPipeline.push_back(BSONObj()); } diff --git a/src/mongo/db/pipeline/document_source_lookup.cpp b/src/mongo/db/pipeline/document_source_lookup.cpp index 5fdd8a95902..2646d125d32 100644 --- a/src/mongo/db/pipeline/document_source_lookup.cpp +++ b/src/mongo/db/pipeline/document_source_lookup.cpp @@ -80,7 +80,16 @@ DocumentSourceLookUp::DocumentSourceLookUp(NamespaceString fromNs, const auto& resolvedNamespace = pExpCtx->getResolvedNamespace(_fromNs); _resolvedNs = resolvedNamespace.ns; _resolvedPipeline = resolvedNamespace.pipeline; - _fromExpCtx = pExpCtx->copyWith(_resolvedNs); + + // We always set an explicit collator on the copied expression context, even if the collator is + // null (i.e. the simple collation). Otherwise, in the situation where the user did not specify + // a collation and the simple collation was inherited from the local collection, the execution + // machinery will incorrectly interpret the null collator and empty user collation as an + // indication that it should inherit the foreign collection's collation. + _fromExpCtx = + pExpCtx->copyWith(_resolvedNs, + boost::none, + pExpCtx->getCollator() ? pExpCtx->getCollator()->clone() : nullptr); _fromExpCtx->subPipelineDepth += 1; uassert(ErrorCodes::MaxSubPipelineDepthExceeded, @@ -827,4 +836,4 @@ intrusive_ptr<DocumentSource> DocumentSourceLookUp::createFromBson( pExpCtx); } } -} +} // namespace mongo |