Skip to content

GH-3726: Add support for $sampleRate match expression. #3765

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

GH-3726: Add support for $sampleRate match expression. #3765

wants to merge 2 commits into from

Conversation

JamesMcNee
Copy link
Contributor

Closes #3726

Hey 👋, hope this implementation is good, this new $sampleRate expression threw me a little as it looks like the first one we have that can only be applied within the match pipeline stage and is not available outside of aggregation (so not a Criteria).

  • You have read the Spring Data contribution guidelines.
  • You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.
  • You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

@JamesMcNee JamesMcNee mentioned this pull request Aug 7, 2021
Closed
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 7, 2021
mp911de
mp911de requested changes Aug 25, 2021
View reviewed changes
Copy link
Member

@mp911de mp911de left a comment

Choose a reason for hiding this comment

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

I'm reluctant introducing another abstraction for $sampleRate. We should find a better representation. Right now, $sampleRate is applicable to aggregations only, while the mentioned $rand operators works in normal queries. I anticipate that sooner or later, $sampleRate would be officially allowed as criteria operator. MongoDB 4.4. accepts already $sampleRate via db.getCollection(…).find({$sampleRate:0.33}).

Therefore, I'd ask you to update this pull request to introduce a sampleRate operator to the Criteria type. Doing so would significantly reduce complexity of this change.

...ata-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/Aggregation.java Outdated
* @return new instance of {@link MatchOperation}.
* @since 3.2.4
*/
public static MatchOperation match(MatchExpression matchExpression) {
Copy link
Member

Choose a reason for hiding this comment

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

We should not introduce another abstraction.

@JamesMcNee
Copy link
Contributor Author

Hi @mp911de Thanks for the review. Makes sense, will update implementation to use Criteria 👍 Should have some time this weekend.

@mp911de
Polishing.
4491301 
Add support for Pattern. Extract Regex flags translation from Criteria into RegexFlags utility class. Add since and author tags. Simplify tests. Update reference documentation.

See spring-projects#3725.
Original pull request: spring-projects#3781.
@JamesMcNee JamesMcNee force-pushed the GH-3726-sample-rate-expression branch from caff79f to a40d15c Compare August 26, 2021 20:25
@JamesMcNee JamesMcNee force-pushed the GH-3726-sample-rate-expression branch from a40d15c to 8c0fe2c Compare August 26, 2021 20:26
@JamesMcNee
Copy link
Contributor Author

@mp911de Hey Mark, have had another crack at it. Hope I have not missed anything 🙈 Thanks!

@mp911de
Copy link
Member

mp911de commented Aug 27, 2021

Thanks a lot. That looks good. I'm taking this PR from here.

@mp911de mp911de added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 27, 2021
@mp911de mp911de added this to the 3.3 M3 (2021.1.0) milestone Aug 27, 2021
mp911de pushed a commit that referenced this pull request Aug 27, 2021
@JamesMcNee @mp911de
Add support for $sampleRate criteria.
62eb719 
Closes #3726
Original pull request: #3765.
mp911de added a commit that referenced this pull request Aug 27, 2021
@mp911de
Polishing.
f662d7c 
Tweak Javadoc. Add since tag, reformat code. Simplify tests. Move documentation bits into the right place.

See #3726.
Original pull request: #3765.
@mp911de
Copy link
Member

mp911de commented Aug 27, 2021

Thank you for your contribution. That's merged and polished now.

@mp911de mp911de closed this Aug 27, 2021
@JamesMcNee JamesMcNee deleted the GH-3726-sample-rate-expression branch August 28, 2021 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mp911de mp911de mp911de requested changes

Assignees
No one assigned
Labels
type: enhancement A general enhancement
Projects
None yet
Milestone
3.3 M3 (2021.1.0)
Development

Successfully merging this pull request may close these issues.

Add support for $sampleRate aggregation operator
3 participants
@JamesMcNee @mp911de @spring-projects-issues