Skip to content
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 for some new aggregation features #2499

Merged
merged 2 commits into from
Feb 27, 2023

Conversation

aturki
Copy link
Contributor

@aturki aturki commented Jan 6, 2023

Q A
Type feature
BC Break no
Fixed issues #2063 (Partially)

Summary

Provides implementation of some aggregation operators

@malarzm malarzm added this to the 2.5.0 milestone Jan 7, 2023
@malarzm malarzm added the Feature label Jan 7, 2023
Copy link
Member

@alcaeus alcaeus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding those operators. I've left some comments, but have not yet reviewed the tests.

I'll also note that methods in the class are currently sorted alphabetically - I'd appreciate it if you could keep the ordering and move newly added methods to the correct place.

lib/Doctrine/ODM/MongoDB/Aggregation/Expr.php Outdated Show resolved Hide resolved
lib/Doctrine/ODM/MongoDB/Aggregation/Expr.php Outdated Show resolved Hide resolved
lib/Doctrine/ODM/MongoDB/Aggregation/Expr.php Outdated Show resolved Hide resolved
lib/Doctrine/ODM/MongoDB/Aggregation/Expr.php Outdated Show resolved Hide resolved
lib/Doctrine/ODM/MongoDB/Aggregation/Expr.php Outdated Show resolved Hide resolved
lib/Doctrine/ODM/MongoDB/Aggregation/Expr.php Outdated Show resolved Hide resolved
lib/Doctrine/ODM/MongoDB/Aggregation/Expr.php Outdated Show resolved Hide resolved
lib/Doctrine/ODM/MongoDB/Aggregation/Expr.php Outdated Show resolved Hide resolved
lib/Doctrine/ODM/MongoDB/Aggregation/Expr.php Outdated Show resolved Hide resolved
lib/Doctrine/ODM/MongoDB/Aggregation/Expr.php Outdated Show resolved Hide resolved
@aturki aturki force-pushed the aggregation-operators branch 2 times, most recently from b203e75 to d70ce64 Compare February 17, 2023 13:48
@alcaeus alcaeus force-pushed the aggregation-operators branch from d70ce64 to 60d7029 Compare February 24, 2023 09:08
@alcaeus
Copy link
Member

alcaeus commented Feb 24, 2023

I've extracted an interface from the Expr class to ensure that new methods are added to the abstract Operator stage. In addition, I've removed the __call magic. The idea behind the egg builder is to provide an expressive interface to help people along, and the call magic does the opposite. It was a fallback for when we couldn't have methods named switch, default, etc.

@alcaeus alcaeus requested review from malarzm and IonBazan February 24, 2023 09:24
@IonBazan IonBazan merged commit b41c549 into doctrine:2.5.x Feb 27, 2023
@IonBazan
Copy link
Member

Thanks @aturki! 🚀

@alcaeus alcaeus removed the request for review from malarzm February 27, 2023 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants