Skip to content

Commit 15e73dc

Browse files
author
Jacob Evans
committed
SERVER-51303 Fix lookup match absorbtion optimization for $type
1 parent 2b0d78e commit 15e73dc

File tree

3 files changed

+237
-52
lines changed

3 files changed

+237
-52
lines changed

‎jstests/aggregation/sources/lookup/lookup_absorb_match.js‎

Lines changed: 218 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
2-
* Tests that a $match with a geo expression still returns the correct results if it has been
3-
* absorbed by a $lookup.
2+
* Tests that $match stages with a variety of expressions still return the correct results if they
3+
* have been absorbed by $lookup stages.
44
*
55
* Accessed collections cannot be implicitly sharded because you cannot $lookup into a sharded
66
* collection.
@@ -13,8 +13,16 @@ let testDB = db.getSiblingDB("lookup_absorb_match");
1313
testDB.dropDatabase();
1414

1515
let locations = testDB.getCollection("locations");
16-
assert.commandWorked(locations.insert({_id: "doghouse", coordinates: [25.0, 60.0]}));
17-
assert.commandWorked(locations.insert({_id: "bullpen", coordinates: [-25.0, -60.0]}));
16+
assert.commandWorked(locations.insert({
17+
_id: "doghouse",
18+
coordinates: [25.0, 60.0],
19+
extra: {breeds: ["terrier", "dachshund", "bulldog"]}
20+
}));
21+
assert.commandWorked(locations.insert({
22+
_id: "bullpen",
23+
coordinates: [-25.0, -60.0],
24+
extra: {breeds: "Scottish Highland", feeling: "bullish"}
25+
}));
1826

1927
let animals = testDB.getCollection("animals");
2028
assert.commandWorked(animals.insert({_id: "dog", locationId: "doghouse"}));
@@ -25,32 +33,33 @@ assert.commandWorked(animals.insert({_id: "bull", locationId: "bullpen"}));
2533
let result = testDB.animals
2634
.aggregate([
2735
{
28-
$lookup: {
29-
from: "locations",
30-
localField: "locationId",
31-
foreignField: "_id",
32-
as: "location"
33-
}
36+
$lookup: {
37+
from: "locations",
38+
localField: "locationId",
39+
foreignField: "_id",
40+
as: "location"
41+
}
3442
},
3543
{$unwind: "$location"},
3644
{
37-
$match: {
38-
"location.coordinates": {
39-
$geoWithin: {
40-
$geometry: {
41-
type: "MultiPolygon",
42-
coordinates: [[[
43-
[20.0, 70.0],
44-
[30.0, 70.0],
45-
[30.0, 50.0],
46-
[20.0, 50.0],
47-
[20.0, 70.0]
48-
]]]
49-
}
50-
}
51-
}
52-
}
53-
}
45+
$match: {
46+
"location.coordinates": {
47+
$geoWithin: {
48+
$geometry: {
49+
type: "MultiPolygon",
50+
coordinates: [[[
51+
[20.0, 70.0],
52+
[30.0, 70.0],
53+
[30.0, 50.0],
54+
[20.0, 50.0],
55+
[20.0, 70.0]
56+
]]]
57+
}
58+
}
59+
}
60+
}
61+
},
62+
{$project: {"location.extra": false}}
5463
])
5564
.toArray();
5665
let expected =
@@ -61,35 +70,194 @@ assert.eq(result, expected);
6170
result = testDB.animals
6271
.aggregate([
6372
{
64-
$lookup: {
65-
from: "locations",
66-
localField: "locationId",
67-
foreignField: "_id",
68-
as: "location"
69-
}
73+
$lookup: {
74+
from: "locations",
75+
localField: "locationId",
76+
foreignField: "_id",
77+
as: "location"
78+
}
7079
},
7180
{$unwind: "$location"},
7281
{
73-
$match: {
74-
"location.coordinates": {
75-
$geoIntersects: {
76-
$geometry: {
77-
type: "MultiPolygon",
78-
coordinates: [[[
79-
[-20.0, -70.0],
80-
[-30.0, -70.0],
81-
[-30.0, -50.0],
82-
[-20.0, -50.0],
83-
[-20.0, -70.0]
84-
]]]
85-
}
86-
}
87-
}
88-
}
89-
}
82+
$match: {
83+
"location.coordinates": {
84+
$geoIntersects: {
85+
$geometry: {
86+
type: "MultiPolygon",
87+
coordinates: [[[
88+
[-20.0, -70.0],
89+
[-30.0, -70.0],
90+
[-30.0, -50.0],
91+
[-20.0, -50.0],
92+
[-20.0, -70.0]
93+
]]]
94+
}
95+
}
96+
}
97+
}
98+
},
99+
{$project: {"location.extra": false}}
90100
])
91101
.toArray();
92102
expected =
93103
[{_id: "bull", locationId: "bullpen", location: {_id: "bullpen", coordinates: [-25.0, -60.0]}}];
94104
assert.eq(result, expected);
105+
106+
// Test that a $match with $type works as expected when absorbed by a $lookup.
107+
result = testDB.animals
108+
.aggregate([
109+
{
110+
$lookup: {
111+
from: "locations",
112+
localField: "locationId",
113+
foreignField: "_id",
114+
as: "location"
115+
}
116+
},
117+
{$unwind: "$location"},
118+
{
119+
$match: {
120+
"location.extra.breeds": {
121+
$type: "array"
122+
}
123+
}
124+
},
125+
{$project: {"location.extra": false}}
126+
])
127+
.toArray();
128+
expected =
129+
[{_id: "dog", locationId: "doghouse", location: {_id: "doghouse", coordinates: [25.0, 60.0]}}];
130+
assert.eq(result, expected);
131+
132+
// Test that a $match with $jsonSchema works as expected although ineligable for absorbtion by a
133+
// $lookup.
134+
result = testDB.animals
135+
.aggregate([
136+
{
137+
$lookup: {
138+
from: "locations",
139+
localField: "locationId",
140+
foreignField: "_id",
141+
as: "location"
142+
}
143+
},
144+
{$unwind: "$location"},
145+
{
146+
$match: {
147+
$jsonSchema: {
148+
properties: {location: {
149+
properties: {extra: {
150+
properties: {breeds: {type: "string"}}
151+
}}
152+
}}
153+
}
154+
}
155+
},
156+
{$project: {"location.extra": false}}
157+
])
158+
.toArray();
159+
expected =
160+
[{_id: "bull", locationId: "bullpen", location: {_id: "bullpen", coordinates: [-25.0, -60.0]}}];
161+
assert.eq(result, expected);
162+
163+
// Test that a more complex $match with $jsonSchema works as expected although ineligable for
164+
// absorbtion by a $lookup.
165+
result = testDB.animals
166+
.aggregate([
167+
{
168+
$lookup: {
169+
from: "locations",
170+
localField: "locationId",
171+
foreignField: "_id",
172+
as: "location"
173+
}
174+
},
175+
{$unwind: "$location"},
176+
{
177+
$match: {
178+
$jsonSchema: {
179+
properties: {location: {properties: {extra: {minProperties: 2}}}}
180+
}
181+
}
182+
},
183+
{$project: {"location.extra": false}}
184+
])
185+
.toArray();
186+
expected =
187+
[{_id: "bull", locationId: "bullpen", location: {_id: "bullpen", coordinates: [-25.0, -60.0]}}];
188+
assert.eq(result, expected);
189+
190+
// Test that a $match with $alwaysTrue works as expected although ineligable for absorbtion by a
191+
// $lookup.
192+
result = testDB.animals
193+
.aggregate([
194+
{
195+
$lookup: {
196+
from: "locations",
197+
localField: "locationId",
198+
foreignField: "_id",
199+
as: "location"
200+
}
201+
},
202+
{$unwind: "$location"},
203+
{
204+
$match: {$alwaysTrue: 1}
205+
},
206+
{$project: {"location.extra": false}},
207+
{$sort: {_id: 1}}
208+
])
209+
.toArray();
210+
expected = [
211+
{_id: "bull", locationId: "bullpen", location: {_id: "bullpen", coordinates: [-25.0, -60.0]}},
212+
{_id: "dog", locationId: "doghouse", location: {_id: "doghouse", coordinates: [25.0, 60.0]}}
213+
];
214+
assert.eq(result, expected);
215+
216+
// Test that a $match with $alwaysFalse works as expected although ineligable for absorbtion by a
217+
// $lookup.
218+
result = testDB.animals
219+
.aggregate([
220+
{
221+
$lookup: {
222+
from: "locations",
223+
localField: "locationId",
224+
foreignField: "_id",
225+
as: "location"
226+
}
227+
},
228+
{$unwind: "$location"},
229+
{
230+
$match: {$alwaysFalse: 1}
231+
},
232+
{$project: {"location.extra": false}},
233+
])
234+
.toArray();
235+
expected = [];
236+
assert.eq(result, expected);
237+
238+
// Test that a $match with $expr works as expected although ineligable for absorbtion by a $lookup.
239+
result = testDB.animals
240+
.aggregate([
241+
{
242+
$lookup: {
243+
from: "locations",
244+
localField: "locationId",
245+
foreignField: "_id",
246+
as: "location"
247+
}
248+
},
249+
{$unwind: "$location"},
250+
{
251+
$match: {
252+
$expr: {
253+
$in: [25.0, "$location.coordinates"]
254+
}
255+
}
256+
},
257+
{$project: {"location.extra": false}},
258+
])
259+
.toArray();
260+
expected =
261+
[{_id: "dog", locationId: "doghouse", location: {_id: "doghouse", coordinates: [25.0, 60.0]}}];
262+
assert.eq(result, expected);
95263
}());

‎src/mongo/db/pipeline/document_source_match.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -446,8 +446,7 @@ boost::intrusive_ptr<DocumentSourceMatch> DocumentSourceMatch::descendMatchOnPat
446446
invariant(expression::isPathPrefixOf(descendOn, leafPath));
447447

448448
auto newPath = leafPath.substr(descendOn.size() + 1);
449-
if (node->getCategory() == MatchExpression::MatchCategory::kLeaf &&
450-
node->matchType() != MatchExpression::TYPE_OPERATOR) {
449+
if (node->getCategory() == MatchExpression::MatchCategory::kLeaf) {
451450
auto leafNode = static_cast<LeafMatchExpression*>(node);
452451
leafNode->setPath(newPath);
453452
} else if (node->getCategory() == MatchExpression::MatchCategory::kArrayMatching) {

‎src/mongo/db/pipeline/pipeline_test.cpp‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,24 @@ TEST(PipelineOptimizationTest, LookupShouldAbsorbUnwindMatch) {
519519
assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, serializedPipe);
520520
}
521521

522+
TEST(PipelineOptimizationTest, LookupShouldAbsorbUnwindAndTypeMatch) {
523+
string inputPipe =
524+
"[{$lookup: {from: 'lookupColl', as: 'asField', localField: 'y', foreignField: "
525+
"'z'}}, "
526+
"{$unwind: '$asField'}, "
527+
"{$match: {'asField.subfield': {$type: [2]}}}]";
528+
string outputPipe =
529+
"[{$lookup: {from: 'lookupColl', as: 'asField', localField: 'y', foreignField: 'z', "
530+
" unwinding: {preserveNullAndEmptyArrays: false}, "
531+
" matching: {subfield: {$type: [2]}}}}]";
532+
string serializedPipe =
533+
"[{$lookup: {from: 'lookupColl', as: 'asField', localField: 'y', foreignField: "
534+
"'z'}}, "
535+
"{$unwind: {path: '$asField'}}, "
536+
"{$match: {'asField.subfield': {$type: [2]}}}]";
537+
assertPipelineOptimizesAndSerializesTo(inputPipe, outputPipe, serializedPipe);
538+
}
539+
522540
TEST(PipelineOptimizationTest, LookupWithPipelineSyntaxShouldAbsorbUnwindMatch) {
523541
string inputPipe =
524542
"[{$lookup: {from: 'lookupColl', as: 'asField', pipeline: []}}, "

0 commit comments

Comments
 (0)