Skip to content

Conversation

@costin
Copy link
Member

@costin costin commented Apr 16, 2019

When specifying a limit over an agg sorting, the limit will be pushed
down to the grouping which affects the custom sorting. This commit fixes
that and restricts the limit only to sorting.

Fix #40984

When specifying a limit over an agg sorting, the limit will be pushed
down to the grouping which affects the custom sorting. This commit fixes
that and restricts the limit only to sorting.

Fix elastic#40984
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@costin costin requested review from astefan and matriv April 16, 2019 14:30
Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

LGTM.
One small picky comment: if I'm not mistaken the query from the original issue is not in the tests? Indeed, there are many variants, but that specific one is not there, something like SELECT gender g, count(*) total FROM test_emp GROUP BY g ORDER BY total desc LIMIT 5.

@costin
Copy link
Member Author

costin commented Apr 16, 2019

@astefan no but it's one quite close to it:
SELECT MAX(salary) AS max, COUNT(*) AS c FROM test_emp GROUP BY gender ORDER BY c DESC LIMIT 3; which is even harder (since the group is not listed).
I'll add this one is as well.

Copy link
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

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

LGTM

@costin costin merged commit da37265 into elastic:master Apr 16, 2019
costin added a commit that referenced this pull request Apr 16, 2019
When specifying a limit over an agg sorting, the limit will be pushed
down to the grouping which affects the custom sorting. This commit fixes
that and restricts the limit only to sorting.

Fix #40984

(cherry picked from commit da37265)
costin added a commit that referenced this pull request Apr 16, 2019
When specifying a limit over an agg sorting, the limit will be pushed
down to the grouping which affects the custom sorting. This commit fixes
that and restricts the limit only to sorting.

Fix #40984

(cherry picked from commit da37265)
costin added a commit that referenced this pull request Apr 16, 2019
When specifying a limit over an agg sorting, the limit will be pushed
down to the grouping which affects the custom sorting. This commit fixes
that and restricts the limit only to sorting.

Fix #40984

(cherry picked from commit da37265)
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
When specifying a limit over an agg sorting, the limit will be pushed
down to the grouping which affects the custom sorting. This commit fixes
that and restricts the limit only to sorting.

Fix elastic#40984
@costin costin deleted the fix-40984 branch July 6, 2020 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SQL provides incorrect results when sorting by an aggregation and using LIMIT

5 participants