SQL: Fix nested ORDER BY #76277
Merged
SQL: Fix nested ORDER BY #76277
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Luegg
added
:Analytics/SQL
Luegg
force-pushed
the
fix/nestedOrderBy
branch
from
c3baea4 to
9890d36
Compare
Luegg
force-pushed
the
fix/nestedOrderBy
branch
from
9890d36 to
0da612e
Compare
elasticmachine
added
the
Team:QL (Deprecated)
|
Pinging @elastic/es-ql (Team:QL) |
costin
approved these changes
Aug 10, 2021
Comment on lines
707
to
708
| for (int i = plan.order().size() - 1; i >= 0; i--) { | ||
| Order order = plan.order().get(i); |
There was a problem hiding this comment.
Imo, it's cleaner to use the listIterator instead of the index
for (ListIterator<Order> i = plan.order().listIterator(); i.hasPrevious(); ) {
Order order = i.previous();
There was a problem hiding this comment.
👍 I was looking for something to avoid the index lookup but was not aware of ListIterator.
|
@elasticmachine run elasticsearch-ci/part-1 |
Luegg
pushed a commit
to Luegg/elasticsearch
that referenced
this pull request
Aug 11, 2021
* introduce PushDownAndCombineOrderBy optimization * move merging of order by clauses to QueryFolder * address review comments * fix iteration
Luegg
pushed a commit
to Luegg/elasticsearch
that referenced
this pull request
Aug 11, 2021
* introduce PushDownAndCombineOrderBy optimization * move merging of order by clauses to QueryFolder * address review comments * fix iteration
elasticsearchmachine
pushed a commit
that referenced
this pull request
Aug 11, 2021
Luegg
pushed a commit
that referenced
this pull request
Aug 11, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #76013. Follow up of #75960
As discussed in #75960, the merging of sort orders from multiple
OrderByis now implemented in the query folder.