DOCSP-41988: Aggregation

[norareidy]

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-41988
Staging - https://preview-mongodbnorareidy.gatsbyjs.io/php-library/DOCSP-41988-aggregation/aggregation/

Self-Review Checklist

• Is this free of any warnings or errors in the RST?
• Did you run a spell-check?
• Did you run a grammar-check?
• Are all the links working?
• Are the facets and meta keywords accurate?

[jordan-smith721]

Nice work! LGTM w/ a couple suggestions!

[jordan-smith721]

Nit:

Suggested change

	Keep the following limitations in mind when using aggregation operations:
	Consider the following limitations when performing aggregation operations:

[jordan-smith721]

I think this admonition should be indented since it applies to the second bullet above

[jordan-smith721]

Suggested change

	An execution plan is a potential way MongoDB can complete an operation.
	An execution plan is a potential way in which MongoDB can complete an operation.

[jordan-smith721]

S: The word "set" here made me think you were going to explain what these fields needed to be set to

Suggested change

	the command information to the ``MongoDB\Database::command()`` method. You must set the
	``aggregate``, ``pipeline``, and ``cursor`` fields of the ``explain`` command document
	to explain the aggregation.
	the command information to the ``MongoDB\Database::command()`` method. You must specify the
	``aggregate``, ``pipeline``, and ``cursor`` fields in the ``explain`` command document
	to explain the aggregation.

[jordan-smith721]

Suggested change

	To learn about assembling an aggregation pipeline and view examples, see
	To learn about assembling an aggregation pipeline and to view examples, see

[jmikola]

Could "aggregation pipeline stages" be simplified as "pipeline stages"? I think the reader already has the context and we could avoid word repetition.

[jmikola]

Note: PHPLIB actually has an explain command helper: MongoDB\Collection::explain().

I think it'd be preferable for the example to demonstrate that, and it'd also provide an opportunity to link to the API reference for that method (like you do for aggregate() above).

Users will need to construct a MongoDB\Operation\Aggregate object to pass to the explain() method. An existing code example of that can be found in the Collection::aggregate() helper. The benefit of this approach is that the pipeline and any objects will receive the same validation as the aggregate() helper, and users won't need to think about the cursor field.

[jmikola]

In contrast to #113, I think you're fine to use json_encode() here because the aggregation result documents will only include strings and integers. If that ever changes, you'd want to use PHPC's BSON API to output Extended JSON.

[jmikola]

If you end up using Collection::explain(), there will be no need to assign a separate $db variable.

[jmikola]

Suggested change

	['$group' => ['_id' => '$borough', 'count' => ['$sum' => 1]]]
	['$group' => ['_id' => '$borough', 'count' => ['$sum' => 1]]],

Note: it's typically good practice to use trailing commas after all array elements, even though it's not required for the final element. I believe we actually enforce this in the PHPLIB coding standard, as it leads to more concise diffs when changing arrays.

This can apply to the $pipeline assignment below, too. I assume that is intentionally duplicated since it's displayed in a different location within the tutorial.

[jmikola]

Consider the following to utilize MongoDB\Collection::explain():

$aggregate = new MongoDB\Operation\Aggregate(
    $collection->getDatabaseName(),
    $collection->getCollectionName(),
    $pipeline
);

$result = $collection->explain($aggregate);

Note that Collection::explain() returns a single document instead of a cursor, so there's no need to call toArray().

[jmikola]

Would "with the" read better than "by using the"? There's already an active verb with "executing".

[norareidy]

We have style guidelines about with / by using: https://www.mongodb.com/docs/meta/style-guide/terminology/alphabetical-terms/#std-term-using

[jmikola]

Optional small suggestions I missed previously.

[jmikola]

Note: I dunno offhand if explain output might include more complex BSON types, but since this is essentially debugging output I don't think we need to be pedantic about using the driver's BSON functions to print actual extended JSON.

[jmikola]

echo supports any number of comma-delimited arguments, so you could also consider:

Suggested change

	echo json_encode($doc) . PHP_EOL;
	echo json_encode($doc), PHP_EOL;

This avoids string concatenation and would be slightly more efficient.

Also applies to echo for the explain output. I might have missed this in previous PRs, so up to you if you'd like to address. OK to leave as-is if a bunch of docs pages are already doing . PHP_EOL.

[norareidy]

I'll change to a comma here and add that to the cleanup tasks