[ES|QL] Fix sorting when aggregate_metric_double present #125191
Merged
Conversation
This file contains hidden or 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
limotova
force-pushed
the
fix-sorting-with-agg-metric
branch
2 times, most recently
from
d19a1ee to
5946e82
Compare
Previously if an aggregate_metric_double was present amongst fields and you tried to sort on any (not necessarily even on the agg_metric itself) field in ES|QL, it would break the results. This commit doesn't add support for sorting _on_ aggregate_metric_double (it is unclear what aspect would be sorted), but it fixes the previous behavior.
limotova
force-pushed
the
fix-sorting-with-agg-metric
branch
from
5946e82 to
8604600
Compare
limotova
changed the title
15 tasks
dnhatn
approved these changes
Mar 27, 2025
We do indeed... maybe we should introduce a new block type or add some additional information to composite blocks to say what they're representing? |
elasticsearchmachine
added
the
needs:triage
limotova
added
:Analytics/ES|QL
elasticsearchmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
|
Hi @limotova, I've created a changelog YAML for you. |
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
martijnvg
approved these changes
Mar 28, 2025
There was a problem hiding this comment.
LGTM, thanks Larisa!
maybe we should introduce a new block type or add some additional information to composite blocks to say what they're representing?
I'm also a little bit worried that the generic CompositeBlock now has very specific usages. Maybe introducing CompositeType enum and adding as a field to CompositeBlock is suffcient? (
++. We can do it in a follow-up @limotova. |
omricohenn
pushed a commit
to omricohenn/elasticsearch
that referenced
this pull request
Mar 28, 2025
) Previously if an aggregate_metric_double was present amongst fields and you tried to sort on any (not necessarily even on the agg_metric itself) field in ES|QL, it would break the results. This commit doesn't add support for sorting _on_ aggregate_metric_double (it is unclear what aspect would be sorted), but it fixes the previous behavior.
limotova
added a commit
to limotova/elasticsearch
that referenced
this pull request
Apr 8, 2025
) Previously if an aggregate_metric_double was present amongst fields and you tried to sort on any (not necessarily even on the agg_metric itself) field in ES|QL, it would break the results. This commit doesn't add support for sorting _on_ aggregate_metric_double (it is unclear what aspect would be sorted), but it fixes the previous behavior.
elasticsearchmachine
pushed a commit
that referenced
this pull request
Apr 8, 2025
* [ES|QL] ToAggregateMetricDouble function (#124595) This commit adds a conversion function from numerics (and aggregate metric doubles) to aggregate metric doubles. It is most useful when you have multiple indices, where one index uses aggregate metric double (e.g. a downsampled index) and another uses a normal numeric type like long or double (e.g. an index prior to downsampling). * remove old docs * [ES|QL] Add ToAggregateMetricDouble example (#125518) Adds AggregateMetricDouble to the ES|QL CSV tests and examples of how to use the ToAggregateMetricDouble function * [ES|QL] Fix sorting when aggregate_metric_double present (#125191) Previously if an aggregate_metric_double was present amongst fields and you tried to sort on any (not necessarily even on the agg_metric itself) field in ES|QL, it would break the results. This commit doesn't add support for sorting _on_ aggregate_metric_double (it is unclear what aspect would be sorted), but it fixes the previous behavior. * drop old style docs again * add new style docs * [CI] Auto commit changes from spotless --------- Co-authored-by: elasticsearchmachine <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Previously if an aggregate_metric_double was present amongst fields and
you tried to sort on any (not necessarily even on the agg_metric itself)
field in ES|QL, it would break the results.
This commit doesn't add support for sorting on aggregate_metric_double
(it is unclear what aspect would be sorted), but it fixes the previous
behavior.