Skip to content

Commit 23059d5

Browse files
committed
Use geonear or collstats as first steps and add warning in aggregate doc (to emphasize the flaws of current design in case of inheritance or skip/limit/etc as our pipeline will have an arbitrary ordering)
1 parent 4dd8432 commit 23059d5

File tree

2 files changed

+60
-4
lines changed

2 files changed

+60
-4
lines changed

mongoengine/queryset/base.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1343,6 +1343,14 @@ def from_json(self, json_data):
13431343
def aggregate(self, pipeline, *suppl_pipeline, **kwargs):
13441344
"""Perform an aggregate function based on your queryset params
13451345
1346+
If the queryset contains a query or skip/limit/sort or if the target Document class
1347+
uses inheritance, this method will add steps prior to the provided pipeline in an arbitrary order.
1348+
This may affect the performance or outcome of the aggregation, so use it consciously.
1349+
1350+
For complex/critical pipelines, we recommended to use the aggregation framework of Pymongo directly,
1351+
it is available through the collection object (YourDocument._collection.aggregate) and will guarantee
1352+
that you have full control on the pipeline.
1353+
13461354
:param pipeline: list of aggregation commands,
13471355
see: https://www.mongodb.com/docs/manual/core/aggregation-pipeline/
13481356
:param suppl_pipeline: unpacked list of pipeline (added to support deprecation of the old interface)
@@ -1380,7 +1388,18 @@ def aggregate(self, pipeline, *suppl_pipeline, **kwargs):
13801388
if self._skip is not None:
13811389
initial_pipeline.append({"$skip": self._skip})
13821390

1383-
final_pipeline = initial_pipeline + user_pipeline
1391+
# geoNear and collStats must be the first stages in the pipeline if present
1392+
first_step = []
1393+
new_user_pipeline = []
1394+
for step_step in user_pipeline:
1395+
if "$geoNear" in step_step:
1396+
first_step.append(step_step)
1397+
elif "$collStats" in step_step:
1398+
first_step.append(step_step)
1399+
else:
1400+
new_user_pipeline.append(step_step)
1401+
1402+
final_pipeline = first_step + initial_pipeline + new_user_pipeline
13841403

13851404
collection = self._collection
13861405
if self._read_preference is not None or self._read_concern is not None:

tests/queryset/test_queryset_aggregation.py

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import unittest
21
import warnings
32

43
from pymongo.read_preferences import ReadPreference
@@ -294,6 +293,44 @@ class Person(Document):
294293

295294
assert list(data) == []
296295

296+
def test_aggregate_geo_near_used_as_initial_step_before_cls_implicit_step(self):
297+
class BaseClass(Document):
298+
meta = {"allow_inheritance": True}
297299

298-
if __name__ == "__main__":
299-
unittest.main()
300+
class Aggr(BaseClass):
301+
name = StringField()
302+
c = PointField()
303+
304+
BaseClass.drop_collection()
305+
306+
x = Aggr(name="X", c=[10.634584, 35.8245029]).save()
307+
y = Aggr(name="Y", c=[10.634584, 35.8245029]).save()
308+
309+
pipeline = [
310+
{
311+
"$geoNear": {
312+
"near": {"type": "Point", "coordinates": [10.634584, 35.8245029]},
313+
"distanceField": "c",
314+
"spherical": True,
315+
}
316+
}
317+
]
318+
res = list(Aggr.objects.aggregate(*pipeline))
319+
assert res == [
320+
{"_cls": "BaseClass.Aggr", "_id": x.id, "c": 0.0, "name": "X"},
321+
{"_cls": "BaseClass.Aggr", "_id": y.id, "c": 0.0, "name": "Y"},
322+
]
323+
324+
def test_aggregate_collstats_used_as_initial_step_before_cls_implicit_step(self):
325+
class SomeDoc(Document):
326+
name = StringField()
327+
328+
SomeDoc.drop_collection()
329+
330+
SomeDoc(name="X").save()
331+
SomeDoc(name="Y").save()
332+
333+
pipeline = [{"$collStats": {"count": {}}}]
334+
res = list(SomeDoc.objects.aggregate(pipeline))
335+
assert len(res) == 1
336+
assert res[0]["count"] == 2

0 commit comments

Comments
 (0)