INTPYTHON-833 Remove $facet in top level group stages

[WaVEV]

Generated query:

[{'$collStats': {}},
 {'$lookup': {'as': 'wrapped',
              'from': 'aggregation_author',
              'pipeline': [{'$group': {'_id': None,
                                       'count': {'$sum': {'$cond': {'else': 1,
                                                                    'if': {'$in': [{'$type': '$_id'},
                                                                                   ['missing',
                                                                                    'null']]},
                                                                    'then': None}}}}}]}},
 {'$replaceWith': {'$cond': [{'$eq': ['$wrapped', []]}, {}, {'$first': '$wrapped'}]}},
 {'$project': {'count': {'$ifNull': ['$count', {'$literal': 0}]}}}]

[aclark4life]

Awesome, thank you and thanks @asya999

[aclark4life]

Can you add a test, particular one that would test something that breaks in the Django admin with QE (even though QE is not merged yet)?

[timgraham]

Please add link to the SERVER ticket and add an explanatory comment about the purpose of the $unionWith clause.

I think we can also mention it in the release notes as a performance improvement since the team said $facet prevents the use of optimizations.

[WaVEV]

Sadly, this cannot be implemented as it is.
To handle the global aggregation and the default values (for example:

SELECT coalesce(avg(age), 35) FROM people WHERE age > 100;

we need to wrap the aggregation so MongoDB always returns a single document.
This is done by wrapping the pipeline inside a lookup. Example:

db.z.aggregate([
    { $collStats: {} },
    { $lookup: { from: "z", as: "wrap", pipeline: [...pipeline stages] } },
    { $replaceWith: {"$cond": [{"$eq": ["$wrap", []]}, {}, {$first: "$wrap"}]}}
])

It’s not very intuitive use of $collStats, but it works 😄. It replaces the need for $unionWith.
$collStats is used as a pivot so we can apply the required operator after the group result.

[aclark4life]

Well played (I think?) 😄

[WaVEV]

😆 The idea comes from the team

[Jibola]

Can you provide, in this PR, an example of what the removal of $facet makes the new query signatures look like?

[timgraham]

I updated the test on the Django fork so it won't fail.

[timgraham]

Need a test with assertAggregateQuery, perhaps in tests/aggregation_.

[timgraham]

Add a comment about what fails without this?

[timgraham]

As discussed on Zoom, it sounds like $collStats causes extra filesystem access that could be avoided by using $document. If it's straightforward, use that for now and I'll add a condition to use $collStats only for encrypted connections in that PR.

[WaVEV]

this make the test pass, but I think this should be tested in deep, need to add a test that having filter all the groups.

[WaVEV]

the current django tests cover the case and it is right.