Skip to content

Fix sorting agg buckets by doc_count #53617

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
Merged
merged 2 commits into from
Mar 16, 2020
Merged

Fix sorting agg buckets by doc_count #53617

merged 2 commits into from
Mar 16, 2020

Conversation

@nik9000
Copy link
Member

@nik9000 nik9000 commented Mar 16, 2020

I broke sorting aggregations by doc_count in #51271 by mixing up true
and false. This flips that comparison and adds a few tests to double
check that we don't so this again.

@nik9000
Fix sorting agg buckets by doc_count
3095538 
I broke sorting aggregations by `doc_count` in elastic#51271 by mixing up true
and false. This flips that comparison and adds a few tests to double
check that we don't so this again.
@nik9000 nik9000 requested a review from not-napoleon March 16, 2020 17:17
@elasticmachine
Copy link
Collaborator

elasticmachine commented Mar 16, 2020

Pinging @elastic/es-analytics-geo (:Analytics/Aggregations)

@nik9000
Copy link
Member Author

nik9000 commented Mar 16, 2020

I'm using the >non-issue label for this because it fixes a bug in 7.7.0 that hasn't been released yet. I'm not sure if that is the right label, but it is what I'm doing now.

@nik9000 nik9000 mentioned this pull request Mar 16, 2020
Closed
7 tasks
not-napoleon
not-napoleon approved these changes Mar 16, 2020
View reviewed changes
Copy link
Member

@not-napoleon not-napoleon left a comment

Choose a reason for hiding this comment

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

LGTM

@nik9000 nik9000 merged commit 3b7843d into elastic:master Mar 16, 2020
nik9000 added a commit to nik9000/elasticsearch that referenced this pull request Mar 16, 2020
@nik9000
Fix sorting agg buckets by doc_count (elastic#53617)
385a791 
I broke sorting aggregations by `doc_count` in elastic#51271 by mixing up true
and false. This flips that comparison and adds a few tests to double
check that we don't so this again.
nik9000 added a commit that referenced this pull request Mar 16, 2020
@nik9000
Update skip before backport
f2f0fc0 
PR #53617 has yet to be backported so we should skip its integration
test on nodes earlier than 8.0.0
nik9000 added a commit that referenced this pull request Mar 16, 2020
@nik9000
Fix sorting agg buckets by doc_count (backport of #53617) (#53627)
9845dbb 
I broke sorting aggregations by `doc_count` in #51271 by mixing up true
and false. This flips that comparison and adds a few tests to double
check that we don't so this again.
@nik9000
Copy link
Member Author

nik9000 commented Mar 16, 2020

Merged and backported, thanks @not-napoleon !

@lizozom
Copy link

lizozom commented Mar 19, 2020

@nik9000 did the backport go through?

@FrankHassanabad FrankHassanabad mentioned this pull request Mar 19, 2020
Closed
@nik9000
Copy link
Member Author

nik9000 commented Mar 19, 2020

@nik9000 did the backport go through?

I believe 9845dbb is the commit that backported it to 7.x. So the fix will go out with 7.7.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@not-napoleon not-napoleon not-napoleon approved these changes

Assignees

No one assigned

Labels

:Analytics/Aggregations Aggregations >non-issue v7.7.0 v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@nik9000 @elasticmachine @lizozom @not-napoleon @jakelandis