mirrors
/
elasticsearch
mirror of https://mirror.osredm.com/root/elasticsearch.git
2
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[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.
This commit is contained in:
Larisa Motova 2025-03-28 10:16:26 -10:00 committed by GitHub
parent 70bc143dee
commit b4f534cb25
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 252 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
docs/changelog/125191.yaml Normal file
View File

@ -0,0 +1,5 @@
pr: 125191
summary: Fix sorting when `aggregate_metric_double` present
area: ES|QL
type: enhancement
issues: []

2
x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java
View File

@ -577,7 +577,7 @@ public enum DataType {
}
public static boolean isSortable(DataType t) {
return false == (t == SOURCE || isCounter(t) || isSpatial(t));
return false == (t == SOURCE || isCounter(t) || isSpatial(t) || t == AGGREGATE_METRIC_DOUBLE);
}
public String nameUpper() {

1
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/ResultBuilder.java
View File

@ -54,6 +54,7 @@ interface ResultBuilder extends Releasable {
case DOUBLE -> new ResultBuilderForDouble(blockFactory, encoder, inKey, positions);
case NULL -> new ResultBuilderForNull(blockFactory);
case DOC -> new ResultBuilderForDoc(blockFactory, positions);
case COMPOSITE -> new ResultBuilderForComposite(blockFactory, positions);
default -> {
assert false : "Result builder for [" + elementType + "]";
throw new UnsupportedOperationException("Result builder for [" + elementType + "]");

61
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/ResultBuilderForComposite.java Normal file
View File

@ -0,0 +1,61 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
package org.elasticsearch.compute.operator.topn;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.compute.data.AggregateMetricDoubleBlockBuilder;
import org.elasticsearch.compute.data.Block;
import org.elasticsearch.compute.data.BlockFactory;
import org.elasticsearch.index.mapper.BlockLoader;
import java.util.List;
public class ResultBuilderForComposite implements ResultBuilder {
private final AggregateMetricDoubleBlockBuilder builder;
ResultBuilderForComposite(BlockFactory blockFactory, int positions) {
this.builder = blockFactory.newAggregateMetricDoubleBlockBuilder(positions);
}
@Override
public void decodeKey(BytesRef keys) {
throw new AssertionError("Composite Block can't be a key");
}
@Override
public void decodeValue(BytesRef values) {
for (BlockLoader.DoubleBuilder subBuilder : List.of(builder.min(), builder.max(), builder.sum())) {
if (TopNEncoder.DEFAULT_UNSORTABLE.decodeBoolean(values)) {
subBuilder.appendDouble(TopNEncoder.DEFAULT_UNSORTABLE.decodeDouble(values));
} else {
subBuilder.appendNull();
}
}
if (TopNEncoder.DEFAULT_UNSORTABLE.decodeBoolean(values)) {
builder.count().appendInt(TopNEncoder.DEFAULT_UNSORTABLE.decodeInt(values));
} else {
builder.count().appendNull();
}
}
@Override
public Block build() {
return builder.build();
}
@Override
public String toString() {
return "ValueExtractorForComposite";
}
@Override
public void close() {
builder.close();
}
}

2
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/ValueExtractor.java
View File

@ -10,6 +10,7 @@ package org.elasticsearch.compute.operator.topn;
import org.elasticsearch.compute.data.Block;
import org.elasticsearch.compute.data.BooleanBlock;
import org.elasticsearch.compute.data.BytesRefBlock;
import org.elasticsearch.compute.data.CompositeBlock;
import org.elasticsearch.compute.data.DocBlock;
import org.elasticsearch.compute.data.DoubleBlock;
import org.elasticsearch.compute.data.ElementType;
@ -37,6 +38,7 @@ interface ValueExtractor {
case DOUBLE -> ValueExtractorForDouble.extractorFor(encoder, inKey, (DoubleBlock) block);
case NULL -> new ValueExtractorForNull();
case DOC -> new ValueExtractorForDoc(encoder, ((DocBlock) block).asVector());
case COMPOSITE -> new ValueExtractorForComposite(encoder, (CompositeBlock) block);
default -> {
assert false : "No value extractor for [" + block.elementType() + "]";
throw new UnsupportedOperationException("No value extractor for [" + block.elementType() + "]");

57
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/ValueExtractorForComposite.java Normal file
View File

@ -0,0 +1,57 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
package org.elasticsearch.compute.operator.topn;
import org.elasticsearch.compute.data.AggregateMetricDoubleBlockBuilder;
import org.elasticsearch.compute.data.CompositeBlock;
import org.elasticsearch.compute.data.DoubleBlock;
import org.elasticsearch.compute.data.IntBlock;
import org.elasticsearch.compute.operator.BreakingBytesRefBuilder;
import java.util.List;
public class ValueExtractorForComposite implements ValueExtractor {
private final CompositeBlock block;
ValueExtractorForComposite(TopNEncoder encoder, CompositeBlock block) {
assert encoder == TopNEncoder.DEFAULT_UNSORTABLE;
this.block = block;
}
@Override
public void writeValue(BreakingBytesRefBuilder values, int position) {
if (block.getBlockCount() != AggregateMetricDoubleBlockBuilder.Metric.values().length) {
throw new UnsupportedOperationException("Composite Blocks for non-aggregate-metric-doubles do not have value extractors");
}
for (AggregateMetricDoubleBlockBuilder.Metric metric : List.of(
AggregateMetricDoubleBlockBuilder.Metric.MIN,
AggregateMetricDoubleBlockBuilder.Metric.MAX,
AggregateMetricDoubleBlockBuilder.Metric.SUM
)) {
DoubleBlock doubleBlock = block.getBlock(metric.getIndex());
if (doubleBlock.isNull(position)) {
TopNEncoder.DEFAULT_UNSORTABLE.encodeBoolean(false, values);
} else {
TopNEncoder.DEFAULT_UNSORTABLE.encodeBoolean(true, values);
TopNEncoder.DEFAULT_UNSORTABLE.encodeDouble(doubleBlock.getDouble(position), values);
}
}
IntBlock intBlock = block.getBlock(AggregateMetricDoubleBlockBuilder.Metric.COUNT.getIndex());
if (intBlock.isNull(position)) {
TopNEncoder.DEFAULT_UNSORTABLE.encodeBoolean(false, values);
} else {
TopNEncoder.DEFAULT_UNSORTABLE.encodeBoolean(true, values);
TopNEncoder.DEFAULT_UNSORTABLE.encodeInt(intBlock.getInt(position), values);
}
}
@Override
public String toString() {
return "ValueExtractorForComposite";
}
}

7
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java
View File

@ -937,7 +937,12 @@ public class EsqlCapabilities {
/**
* Make numberOfChannels consistent with layout in DefaultLayout by removing duplicated ChannelSet.
*/
MAKE_NUMBER_OF_CHANNELS_CONSISTENT_WITH_LAYOUT;
MAKE_NUMBER_OF_CHANNELS_CONSISTENT_WITH_LAYOUT,
/**
* Support for sorting when aggregate_metric_doubles are present
*/
AGGREGATE_METRIC_DOUBLE_SORTING(AGGREGATE_METRIC_DOUBLE_FEATURE_FLAG);
private final boolean enabled;

6
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java
View File

@ -417,10 +417,10 @@ public class LocalExecutionPlanner {
case VERSION -> TopNEncoder.VERSION;
case BOOLEAN, NULL, BYTE, SHORT, INTEGER, LONG, DOUBLE, FLOAT, HALF_FLOAT, DATETIME, DATE_NANOS, DATE_PERIOD, TIME_DURATION,
OBJECT, SCALED_FLOAT, UNSIGNED_LONG, DOC_DATA_TYPE, TSID_DATA_TYPE -> TopNEncoder.DEFAULT_SORTABLE;
case GEO_POINT, CARTESIAN_POINT, GEO_SHAPE, CARTESIAN_SHAPE, COUNTER_LONG, COUNTER_INTEGER, COUNTER_DOUBLE, SOURCE ->
TopNEncoder.DEFAULT_UNSORTABLE;
case GEO_POINT, CARTESIAN_POINT, GEO_SHAPE, CARTESIAN_SHAPE, COUNTER_LONG, COUNTER_INTEGER, COUNTER_DOUBLE, SOURCE,
AGGREGATE_METRIC_DOUBLE -> TopNEncoder.DEFAULT_UNSORTABLE;
// unsupported fields are encoded as BytesRef, we'll use the same encoder; all values should be null at this point
case PARTIAL_AGG, UNSUPPORTED, AGGREGATE_METRIC_DOUBLE -> TopNEncoder.UNSUPPORTED;
case PARTIAL_AGG, UNSUPPORTED -> TopNEncoder.UNSUPPORTED;
};
}
List<TopNOperator.SortOrder> orders = topNExec.order().stream().map(order -> {

8
x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateMetricDoubleFieldMapper.java
View File

@ -627,22 +627,22 @@ public class AggregateMetricDoubleFieldMapper extends FieldMapper {
}
private void readSingleRow(int docId, AggregateMetricDoubleBuilder builder) throws IOException {
if (minValues.advanceExact(docId)) {
if (minValues != null && minValues.advanceExact(docId)) {
builder.min().appendDouble(NumericUtils.sortableLongToDouble(minValues.longValue()));
} else {
builder.min().appendNull();
}
if (maxValues.advanceExact(docId)) {
if (maxValues != null && maxValues.advanceExact(docId)) {
builder.max().appendDouble(NumericUtils.sortableLongToDouble(maxValues.longValue()));
} else {
builder.max().appendNull();
}
if (sumValues.advanceExact(docId)) {
if (sumValues != null && sumValues.advanceExact(docId)) {
builder.sum().appendDouble(NumericUtils.sortableLongToDouble(sumValues.longValue()));
} else {
builder.sum().appendNull();
}
if (valueCountValues.advanceExact(docId)) {
if (valueCountValues != null && valueCountValues.advanceExact(docId)) {
builder.count().appendInt(Math.toIntExact(valueCountValues.longValue()));
} else {
builder.count().appendNull();

48
x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/40_tsdb.yml
View File

@ -375,6 +375,54 @@ grouping stats on aggregate_metric_double:
- match: {values.1.3: 16.0}
- match: {values.1.4: "B"}
---
sorting with aggregate_metric_double with partial submetrics:
- requires:
test_runner_features: [capabilities]
capabilities:
- method: POST
path: /_query
parameters: []
capabilities: [aggregate_metric_double_sorting]
reason: "Support for sorting when aggregate_metric_double present"
- do:
allowed_warnings_regex:
- "No limit defined, adding default limit of \\[.*\\]"
esql.query:
body:
query: 'FROM test3 | SORT @timestamp | KEEP @timestamp, agg_metric'
- length: {values: 4}
- length: {values.0: 2}
- match: {columns.0.name: "@timestamp"}
- match: {columns.0.type: "date"}
- match: {columns.1.name: "agg_metric"}
- match: {columns.1.type: "aggregate_metric_double"}
- match: {values.0.0: "2021-04-28T19:50:04.467Z"}
- match: {values.1.0: "2021-04-28T19:50:24.467Z"}
- match: {values.2.0: "2021-04-28T19:50:44.467Z"}
- match: {values.3.0: "2021-04-28T19:51:04.467Z"}
- match: {values.0.1: '{"min":-3.0,"max":1.0}'}
- match: {values.1.1: '{"min":3.0,"max":10.0}'}
- match: {values.2.1: '{"min":2.0,"max":17.0}'}
- match: {values.3.1: null}
---
aggregate_metric_double unsortable:
- requires:
test_runner_features: [capabilities]
capabilities:
- method: POST
path: /_query
parameters: []
capabilities: [aggregate_metric_double_sorting]
reason: "Support for sorting when aggregate_metric_double present"
- do:
catch: /cannot sort on aggregate_metric_double/
esql.query:
body:
query: 'FROM test2 | sort agg_metric'
---
stats on aggregate_metric_double with partial submetrics:
- requires:

126
x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/40_unsupported_types.yml
View File

@ -310,8 +310,8 @@ unsupported with sort:
- method: POST
path: /_query
parameters: [ ]
capabilities: [ aggregate_metric_double ]
reason: "support for aggregate_metric_double type"
capabilities: [ aggregate_metric_double_sorting ]
reason: "support for sorting when aggregate_metric_double present"
- do:
allowed_warnings_regex:
@ -319,95 +319,97 @@ unsupported with sort:
- "No limit defined, adding default limit of \\[.*\\]"
esql.query:
body:
query: 'from test | drop aggregate_metric_double | sort some_doc.bar'
query: 'from test | sort some_doc.bar'
- match: { columns.0.name: binary }
- match: { columns.0.type: unsupported }
- match: { columns.1.name: completion }
- match: { columns.0.name: aggregate_metric_double }
- match: { columns.0.type: aggregate_metric_double }
- match: { columns.1.name: binary }
- match: { columns.1.type: unsupported }
- match: { columns.2.name: date_nanos }
- match: { columns.2.type: date_nanos }
- match: { columns.3.name: date_range }
- match: { columns.3.type: unsupported }
- match: { columns.4.name: dense_vector }
- match: { columns.2.name: completion }
- match: { columns.2.type: unsupported }
- match: { columns.3.name: date_nanos }
- match: { columns.3.type: date_nanos }
- match: { columns.4.name: date_range }
- match: { columns.4.type: unsupported }
- match: { columns.5.name: double_range }
- match: { columns.5.name: dense_vector }
- match: { columns.5.type: unsupported }
- match: { columns.6.name: float_range }
- match: { columns.6.name: double_range }
- match: { columns.6.type: unsupported }
- match: { columns.7.name: geo_point }
- match: { columns.7.type: geo_point }
- match: { columns.8.name: geo_point_alias }
- match: { columns.7.name: float_range }
- match: { columns.7.type: unsupported }
- match: { columns.8.name: geo_point }
- match: { columns.8.type: geo_point }
- match: { columns.9.name: geo_shape }
- match: { columns.9.type: geo_shape }
- match: { columns.10.name: histogram }
- match: { columns.10.type: unsupported }
- match: { columns.11.name: integer_range }
- match: { columns.9.name: geo_point_alias }
- match: { columns.9.type: geo_point }
- match: { columns.10.name: geo_shape }
- match: { columns.10.type: geo_shape }
- match: { columns.11.name: histogram }
- match: { columns.11.type: unsupported }
- match: { columns.12.name: ip_range }
- match: { columns.12.name: integer_range }
- match: { columns.12.type: unsupported }
- match: { columns.13.name: long_range }
- match: { columns.13.name: ip_range }
- match: { columns.13.type: unsupported }
- match: { columns.14.name: match_only_text }
- match: { columns.14.type: text }
- match: { columns.15.name: name }
- match: { columns.15.type: keyword }
- match: { columns.16.name: point }
- match: { columns.16.type: cartesian_point }
- match: { columns.17.name: rank_feature }
- match: { columns.17.type: unsupported }
- match: { columns.18.name: rank_features }
- match: { columns.14.name: long_range }
- match: { columns.14.type: unsupported }
- match: { columns.15.name: match_only_text }
- match: { columns.15.type: text }
- match: { columns.16.name: name }
- match: { columns.16.type: keyword }
- match: { columns.17.name: point }
- match: { columns.17.type: cartesian_point }
- match: { columns.18.name: rank_feature }
- match: { columns.18.type: unsupported }
- match: { columns.19.name: search_as_you_type }
- match: { columns.19.name: rank_features }
- match: { columns.19.type: unsupported }
- match: { columns.20.name: search_as_you_type._2gram }
- match: { columns.20.name: search_as_you_type }
- match: { columns.20.type: unsupported }
- match: { columns.21.name: search_as_you_type._3gram }
- match: { columns.21.name: search_as_you_type._2gram }
- match: { columns.21.type: unsupported }
- match: { columns.22.name: search_as_you_type._index_prefix }
- match: { columns.22.name: search_as_you_type._3gram }
- match: { columns.22.type: unsupported }
- match: { columns.23.name: shape }
- match: { columns.23.type: cartesian_shape }
- match: { columns.24.name: some_doc.bar }
- match: { columns.24.type: long }
- match: { columns.25.name: some_doc.foo }
- match: { columns.25.type: keyword }
- match: { columns.26.name: text }
- match: { columns.26.type: text }
- match: { columns.27.name: token_count }
- match: { columns.27.type: integer }
- match: { columns.23.name: search_as_you_type._index_prefix }
- match: { columns.23.type: unsupported }
- match: { columns.24.name: shape }
- match: { columns.24.type: cartesian_shape }
- match: { columns.25.name: some_doc.bar }
- match: { columns.25.type: long }
- match: { columns.26.name: some_doc.foo }
- match: { columns.26.type: keyword }
- match: { columns.27.name: text }
- match: { columns.27.type: text }
- match: { columns.28.name: token_count }
- match: { columns.28.type: integer }
- length: { values: 1 }
- match: { values.0.0: null }
- match: { values.0.0: '{"min":1.0,"max":3.0,"sum":10.1,"value_count":5}' }
- match: { values.0.1: null }
- match: { values.0.2: "2015-01-01T12:10:30.123456789Z" }
- match: { values.0.3: null }
- match: { values.0.2: null }
- match: { values.0.3: "2015-01-01T12:10:30.123456789Z" }
- match: { values.0.4: null }
- match: { values.0.5: null }
- match: { values.0.6: null }
- match: { values.0.7: "POINT (10.0 12.0)" }
- match: { values.0.7: null }
- match: { values.0.8: "POINT (10.0 12.0)" }
- match: { values.0.9: "LINESTRING (-97.154 25.996, -97.159 25.998, -97.181 25.991, -97.187 25.985)" }
- match: { values.0.10: null }
- match: { values.0.9: "POINT (10.0 12.0)" }
- match: { values.0.10: "LINESTRING (-97.154 25.996, -97.159 25.998, -97.181 25.991, -97.187 25.985)" }
- match: { values.0.11: null }
- match: { values.0.12: null }
- match: { values.0.13: null }
- match: { values.0.14: "foo bar baz" }
- match: { values.0.15: Alice }
- match: { values.0.16: "POINT (-97.15447 25.9961525)" }
- match: { values.0.17: null }
- match: { values.0.14: null }
- match: { values.0.15: "foo bar baz" }
- match: { values.0.16: Alice }
- match: { values.0.17: "POINT (-97.15447 25.9961525)" }
- match: { values.0.18: null }
- match: { values.0.19: null }
- match: { values.0.20: null }
- match: { values.0.21: null }
- match: { values.0.22: null }
- match: { values.0.23: "LINESTRING (-377.03653 389.897676, -377.009051 389.889939)" }
- match: { values.0.24: 12 }
- match: { values.0.25: xy }
- match: { values.0.26: "foo bar" }
- match: { values.0.27: 3 }
- match: { values.0.23: null }
- match: { values.0.24: "LINESTRING (-377.03653 389.897676, -377.009051 389.889939)" }
- match: { values.0.25: 12 }
- match: { values.0.26: xy }
- match: { values.0.27: "foo bar" }
- match: { values.0.28: 3 }
---
nested declared inline:
- do: