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

Add support for $rand aggregation operator. #3724

Closed
4 tasks
christophstrobl opened this issue Jul 14, 2021 · 5 comments
Closed
4 tasks

Add support for $rand aggregation operator. #3724

christophstrobl opened this issue Jul 14, 2021 · 5 comments
Assignees
Labels
in: aggregation-framework Aggregation framework support status: first-timers-only An issue that can only be worked on by brand new contributors type: enhancement A general enhancement

Comments

@christophstrobl
Copy link
Member

christophstrobl commented Jul 14, 2021

This is a first-timers-only issue.

This means we've worked to make it more legible to folks who either haven't contributed to our codebase before or even folks who haven't contributed to open source before.

If that's you, we're interested in helping you take the first step and can answer questions and help you out as you do. Note that we're especially interested in contributions from people from groups underrepresented in free and open source software!

If you have contributed before, consider leaving this one for someone new, and looking through our general ideal-for-contribution issues.
Thanks!

Problem

MongoDB 4.4.2 introduced a new aggregation operator $rand that we want to support via the Aggregation Framework.
Details about the operator can be found within the MongoDB Reference Documentation.

Solution

Introduce MathOperators to the org.springframework.data.mongodb.core.aggregation package and add a Rand type that implements AggregationExpression. You may want to have a look at ArithmeticOperators to get an impression what this could look like.
Update org.springframework.data.mongodb.core.spel.MethodReferenceNode to add SpEL support for the operator via a rand() method.

Make sure to provide tests that assert the expression can be used within an pipeline and produces the desired command.
Again you may want to have a look at existing examples (eg. ArithmeticOperatorsUnitTests & SpelExpressionTransformerUnitTests).

Steps to Fix

  • Claim this issue with a comment below and ask any clarifying questions you need
  • Set up a repository locally following the Contributing Guidelines
  • Try to fix the issue following the steps above
  • Commit your changes and create a pull request
@christophstrobl christophstrobl added type: enhancement A general enhancement in: aggregation-framework Aggregation framework support status: first-timers-only An issue that can only be worked on by brand new contributors labels Jul 14, 2021
@ahmedmq
Copy link
Contributor

ahmedmq commented Jul 27, 2021

@christophstrobl - Can I work on this issue please?

@christophstrobl
Copy link
Member Author

sure - please assign it to yourself so others know you're taking care of it.
Happy to help if questions arise. The part with the MethodReferenceNode may benefit from changes in #3741 adding emptyRef().

@ahmedmq
Copy link
Contributor

ahmedmq commented Jul 27, 2021

@christophstrobl - I am not seeing the button to assign the issue to myself, probably an access issue. could you assign it to me please.

@ahmedmq
Copy link
Contributor

ahmedmq commented Jul 28, 2021

@christophstrobl - Any reason why we do not want to have the new rand() aggregation as part of the existing ArithmeticOperators? I was thinking we would mostly use the rand() operator to perform some arithmetic computation as part of the aggregation pipeline.

@christophstrobl
Copy link
Member Author

It's fine to have them there. In case we want to introduce MathOperators at some point we can still delegate.

ahmedmq added a commit to ahmedmq/spring-data-mongodb that referenced this issue Jul 31, 2021
mp911de added a commit that referenced this issue Aug 25, 2021
Add author tags, tweak Javadoc style. Simplify tests. Document operator.

See #3724
Original pull request: #3759.
@mp911de mp911de linked a pull request Aug 25, 2021 that will close this issue
@mp911de mp911de added this to the 3.3 M3 (2021.1.0) milestone Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: aggregation-framework Aggregation framework support status: first-timers-only An issue that can only be worked on by brand new contributors type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants