-
-
Notifications
You must be signed in to change notification settings - Fork 505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support new aggregation operators in builder #2514
Conversation
6e1575f
to
75ab4da
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not done with the review, but I'm publishing what comments I have so far.
I'm intentionally not reviewing the interfaces in detail and am instead focusing on the implementations.
lib/Doctrine/ODM/MongoDB/Aggregation/Operator/ArrayOperators.php
Outdated
Show resolved
Hide resolved
tests/Doctrine/ODM/MongoDB/Tests/Aggregation/Stage/BucketAutoTest.php
Outdated
Show resolved
Hide resolved
Note: test failures on PHP 7.4 are due to |
lib/Doctrine/ODM/MongoDB/Aggregation/Operator/CustomOperators.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to make the change to accumulator()
arg order on your own.
Everything else LGTM.
db72b9c
to
99f1ece
Compare
Note: ProvidesGroupAccumulatorOperators errors were ignored as the trait is designed to be used in conjunction with the corresponding interface.
99f1ece
to
9e62307
Compare
9e62307
to
b6d1fc5
Compare
Ignoring PHPStan as those failures are the same we're already seeing due to psalm types. |
* Add interfaces for aggregation pipeline operators * Implement new aggregation pipeline operators in stages * Update static analysis baselines * Use consistent formatting for operators with optional arguments * Use consistent capitalisation for "date object" * Wrap docblock comments * Rename ensureArray to something more descriptive * Remove duplicated docblocks * Use func_get_args when proxying expression methods * Fix wrong return types for internal expr methods * Use func_get_args for operators where possible * Disambiguate closure param in tests * Fix docblock errors Note: ProvidesGroupAccumulatorOperators errors were ignored as the trait is designed to be used in conjunction with the corresponding interface. * Make initArgs parameter for $accumulator optional * Use static return types across aggregation builder * Fix handling of optional parameters for $range and $slice
Summary
This PR adds the following new aggregation pipeline operators to the builder:
$accumulator
,$binarySize
,$bottom
,$bottomN
,$bsonSize
,$dateAdd
,$dateDiff
,$dateFromParts
,$dateFromString
,$dateSubtract
,$dateToParts
,$dateTrunc
,$firstN
,$function
,$getField
,$lastsN
,$maxN
,$mergeObjects
,$minN
,$rand
,$regexFind
,$regexFindAll
,$regexMatch
,$replaceAll
,$replaceOne
,$sampleRate
,$setField
,$sortArray
,$top
,$topN
,$tsIncrement
,$tsSecond
.This covers all aggregation pipeline operators currently supported in MongoDB, with the exception of accumulator operators only supported in the
$setWindowFields
stage, which will be introduced in a separate pull request.I've also taken liberty to organise the code a bit. We now have interfaces for the various operator groups instead of one generic operator interface, and I've cleaned up the logic around accumulator stages (such as
$sum
) which have two different forms depending on the stage they're being used in. In the$group
,$bucket
, and$bucketAuto
stages they take a single argument, whereas they take multiple arguments in other stages such as$project
. This is now properly accounted for and easier to handle due to a trait that handles this more gracefully.Note: GitHub Copilot was instrumental in getting this work done as it automated away certain repetitive tasks (such as adding proxy methods to the
Operator
class. I've also marked that class as@internal
as we may want to do away with it in the future in favour of multiple traits that add specific operator groups, similar to how we have interfaces for the groups.