-
Notifications
You must be signed in to change notification settings - Fork 102
Add random sampler aggregation #2944
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
Conversation
* Must be greater than 0, less than 0.5, or exactly 1. | ||
* The lower the probability, the fewer documents are matched. | ||
*/ | ||
probability: float |
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.
Docs say float, but server says double
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.
Thanks, I should have checked that part, sorry. Done in 919eb559 (#2944).
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.
Added a suggestion for what I believe is a typo.
* | ||
* A single bucket aggregation that randomly includes documents in the aggregated results. | ||
* Sampling provides significant speed improvement at the cost of accuracy. | ||
* @doc_id search-aggregations-pipeline-random-sampler-aggregation |
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.
* @doc_id search-aggregations-pipeline-random-sampler-aggregation | |
* @doc_id search-aggregations-bucket-random-sampler-aggregation |
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.
Thanks! I removed bucket
too because it's not in the Elasticsearch URL for the random sampler aggregation:
- Random sampler: https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-random-sampler-aggregation.html
- Sampler: https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-bucket-sampler-aggregation.html
Done in 919eb559 (#2944).
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.
LGTM!
* Add random sampler aggregation * Address review comments * Run make contrib (cherry picked from commit 3c75c87)
* Add random sampler aggregation * Address review comments * Run make contrib (cherry picked from commit 3c75c87)
No description provided.