Skip to content

Commit e11828c

Browse files
original-brownbearoriginal-brownbear
authored
Merge branch 'main' into batched-exec-short
2 parents ffb8d26 + b4f534c commit e11828c

File tree

11 files changed

+252
-71
lines changed

11 files changed

+252
-71
lines changed

‎docs/changelog/125191.yaml

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 125191
2+
summary: Fix sorting when `aggregate_metric_double` present
3+
area: ES|QL
4+
type: enhancement
5+
issues: []

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ public static boolean isSpatial(DataType t) {
577577
}
578578

579579
public static boolean isSortable(DataType t) {
580-
return false == (t == SOURCE || isCounter(t) || isSpatial(t));
580+
return false == (t == SOURCE || isCounter(t) || isSpatial(t) || t == AGGREGATE_METRIC_DOUBLE);
581581
}
582582

583583
public String nameUpper() {

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

+1
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ static ResultBuilder resultBuilderFor(
5454
case DOUBLE -> new ResultBuilderForDouble(blockFactory, encoder, inKey, positions);
5555
case NULL -> new ResultBuilderForNull(blockFactory);
5656
case DOC -> new ResultBuilderForDoc(blockFactory, positions);
57+
case COMPOSITE -> new ResultBuilderForComposite(blockFactory, positions);
5758
default -> {
5859
assert false : "Result builder for [" + elementType + "]";
5960
throw new UnsupportedOperationException("Result builder for [" + elementType + "]");

‎x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/ResultBuilderForComposite.java

+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.compute.operator.topn;
9+
10+
import org.apache.lucene.util.BytesRef;
11+
import org.elasticsearch.compute.data.AggregateMetricDoubleBlockBuilder;
12+
import org.elasticsearch.compute.data.Block;
13+
import org.elasticsearch.compute.data.BlockFactory;
14+
import org.elasticsearch.index.mapper.BlockLoader;
15+
16+
import java.util.List;
17+
18+
public class ResultBuilderForComposite implements ResultBuilder {
19+
20+
private final AggregateMetricDoubleBlockBuilder builder;
21+
22+
ResultBuilderForComposite(BlockFactory blockFactory, int positions) {
23+
this.builder = blockFactory.newAggregateMetricDoubleBlockBuilder(positions);
24+
}
25+
26+
@Override
27+
public void decodeKey(BytesRef keys) {
28+
throw new AssertionError("Composite Block can't be a key");
29+
}
30+
31+
@Override
32+
public void decodeValue(BytesRef values) {
33+
for (BlockLoader.DoubleBuilder subBuilder : List.of(builder.min(), builder.max(), builder.sum())) {
34+
if (TopNEncoder.DEFAULT_UNSORTABLE.decodeBoolean(values)) {
35+
subBuilder.appendDouble(TopNEncoder.DEFAULT_UNSORTABLE.decodeDouble(values));
36+
} else {
37+
subBuilder.appendNull();
38+
}
39+
}
40+
if (TopNEncoder.DEFAULT_UNSORTABLE.decodeBoolean(values)) {
41+
builder.count().appendInt(TopNEncoder.DEFAULT_UNSORTABLE.decodeInt(values));
42+
} else {
43+
builder.count().appendNull();
44+
}
45+
}
46+
47+
@Override
48+
public Block build() {
49+
return builder.build();
50+
}
51+
52+
@Override
53+
public String toString() {
54+
return "ValueExtractorForComposite";
55+
}
56+
57+
@Override
58+
public void close() {
59+
builder.close();
60+
}
61+
}

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

+2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.elasticsearch.compute.data.Block;
1111
import org.elasticsearch.compute.data.BooleanBlock;
1212
import org.elasticsearch.compute.data.BytesRefBlock;
13+
import org.elasticsearch.compute.data.CompositeBlock;
1314
import org.elasticsearch.compute.data.DocBlock;
1415
import org.elasticsearch.compute.data.DoubleBlock;
1516
import org.elasticsearch.compute.data.ElementType;
@@ -37,6 +38,7 @@ static ValueExtractor extractorFor(ElementType elementType, TopNEncoder encoder,
3738
case DOUBLE -> ValueExtractorForDouble.extractorFor(encoder, inKey, (DoubleBlock) block);
3839
case NULL -> new ValueExtractorForNull();
3940
case DOC -> new ValueExtractorForDoc(encoder, ((DocBlock) block).asVector());
41+
case COMPOSITE -> new ValueExtractorForComposite(encoder, (CompositeBlock) block);
4042
default -> {
4143
assert false : "No value extractor for [" + block.elementType() + "]";
4244
throw new UnsupportedOperationException("No value extractor for [" + block.elementType() + "]");

‎x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/ValueExtractorForComposite.java

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.compute.operator.topn;
9+
10+
import org.elasticsearch.compute.data.AggregateMetricDoubleBlockBuilder;
11+
import org.elasticsearch.compute.data.CompositeBlock;
12+
import org.elasticsearch.compute.data.DoubleBlock;
13+
import org.elasticsearch.compute.data.IntBlock;
14+
import org.elasticsearch.compute.operator.BreakingBytesRefBuilder;
15+
16+
import java.util.List;
17+
18+
public class ValueExtractorForComposite implements ValueExtractor {
19+
private final CompositeBlock block;
20+
21+
ValueExtractorForComposite(TopNEncoder encoder, CompositeBlock block) {
22+
assert encoder == TopNEncoder.DEFAULT_UNSORTABLE;
23+
this.block = block;
24+
}
25+
26+
@Override
27+
public void writeValue(BreakingBytesRefBuilder values, int position) {
28+
if (block.getBlockCount() != AggregateMetricDoubleBlockBuilder.Metric.values().length) {
29+
throw new UnsupportedOperationException("Composite Blocks for non-aggregate-metric-doubles do not have value extractors");
30+
}
31+
for (AggregateMetricDoubleBlockBuilder.Metric metric : List.of(
32+
AggregateMetricDoubleBlockBuilder.Metric.MIN,
33+
AggregateMetricDoubleBlockBuilder.Metric.MAX,
34+
AggregateMetricDoubleBlockBuilder.Metric.SUM
35+
)) {
36+
DoubleBlock doubleBlock = block.getBlock(metric.getIndex());
37+
if (doubleBlock.isNull(position)) {
38+
TopNEncoder.DEFAULT_UNSORTABLE.encodeBoolean(false, values);
39+
} else {
40+
TopNEncoder.DEFAULT_UNSORTABLE.encodeBoolean(true, values);
41+
TopNEncoder.DEFAULT_UNSORTABLE.encodeDouble(doubleBlock.getDouble(position), values);
42+
}
43+
}
44+
IntBlock intBlock = block.getBlock(AggregateMetricDoubleBlockBuilder.Metric.COUNT.getIndex());
45+
if (intBlock.isNull(position)) {
46+
TopNEncoder.DEFAULT_UNSORTABLE.encodeBoolean(false, values);
47+
} else {
48+
TopNEncoder.DEFAULT_UNSORTABLE.encodeBoolean(true, values);
49+
TopNEncoder.DEFAULT_UNSORTABLE.encodeInt(intBlock.getInt(position), values);
50+
}
51+
}
52+
53+
@Override
54+
public String toString() {
55+
return "ValueExtractorForComposite";
56+
}
57+
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -937,7 +937,12 @@ public enum Cap {
937937
/**
938938
* Make numberOfChannels consistent with layout in DefaultLayout by removing duplicated ChannelSet.
939939
*/
940-
MAKE_NUMBER_OF_CHANNELS_CONSISTENT_WITH_LAYOUT;
940+
MAKE_NUMBER_OF_CHANNELS_CONSISTENT_WITH_LAYOUT,
941+
942+
/**
943+
* Support for sorting when aggregate_metric_doubles are present
944+
*/
945+
AGGREGATE_METRIC_DOUBLE_SORTING(AGGREGATE_METRIC_DOUBLE_FEATURE_FLAG);
941946

942947
private final boolean enabled;
943948

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -417,10 +417,10 @@ private PhysicalOperation planTopN(TopNExec topNExec, LocalExecutionPlannerConte
417417
case VERSION -> TopNEncoder.VERSION;
418418
case BOOLEAN, NULL, BYTE, SHORT, INTEGER, LONG, DOUBLE, FLOAT, HALF_FLOAT, DATETIME, DATE_NANOS, DATE_PERIOD, TIME_DURATION,
419419
OBJECT, SCALED_FLOAT, UNSIGNED_LONG, DOC_DATA_TYPE, TSID_DATA_TYPE -> TopNEncoder.DEFAULT_SORTABLE;
420-
case GEO_POINT, CARTESIAN_POINT, GEO_SHAPE, CARTESIAN_SHAPE, COUNTER_LONG, COUNTER_INTEGER, COUNTER_DOUBLE, SOURCE ->
421-
TopNEncoder.DEFAULT_UNSORTABLE;
420+
case GEO_POINT, CARTESIAN_POINT, GEO_SHAPE, CARTESIAN_SHAPE, COUNTER_LONG, COUNTER_INTEGER, COUNTER_DOUBLE, SOURCE,
421+
AGGREGATE_METRIC_DOUBLE -> TopNEncoder.DEFAULT_UNSORTABLE;
422422
// unsupported fields are encoded as BytesRef, we'll use the same encoder; all values should be null at this point
423-
case PARTIAL_AGG, UNSUPPORTED, AGGREGATE_METRIC_DOUBLE -> TopNEncoder.UNSUPPORTED;
423+
case PARTIAL_AGG, UNSUPPORTED -> TopNEncoder.UNSUPPORTED;
424424
};
425425
}
426426
List<TopNOperator.SortOrder> orders = topNExec.order().stream().map(order -> {

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -627,22 +627,22 @@ public void read(int docId, StoredFields storedFields, Builder builder) throws I
627627
}
628628

629629
private void readSingleRow(int docId, AggregateMetricDoubleBuilder builder) throws IOException {
630-
if (minValues.advanceExact(docId)) {
630+
if (minValues != null && minValues.advanceExact(docId)) {
631631
builder.min().appendDouble(NumericUtils.sortableLongToDouble(minValues.longValue()));
632632
} else {
633633
builder.min().appendNull();
634634
}
635-
if (maxValues.advanceExact(docId)) {
635+
if (maxValues != null && maxValues.advanceExact(docId)) {
636636
builder.max().appendDouble(NumericUtils.sortableLongToDouble(maxValues.longValue()));
637637
} else {
638638
builder.max().appendNull();
639639
}
640-
if (sumValues.advanceExact(docId)) {
640+
if (sumValues != null && sumValues.advanceExact(docId)) {
641641
builder.sum().appendDouble(NumericUtils.sortableLongToDouble(sumValues.longValue()));
642642
} else {
643643
builder.sum().appendNull();
644644
}
645-
if (valueCountValues.advanceExact(docId)) {
645+
if (valueCountValues != null && valueCountValues.advanceExact(docId)) {
646646
builder.count().appendInt(Math.toIntExact(valueCountValues.longValue()));
647647
} else {
648648
builder.count().appendNull();

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

+48
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,54 @@ grouping stats on aggregate_metric_double:
375375
- match: {values.1.3: 16.0}
376376
- match: {values.1.4: "B"}
377377

378+
---
379+
sorting with aggregate_metric_double with partial submetrics:
380+
- requires:
381+
test_runner_features: [capabilities]
382+
capabilities:
383+
- method: POST
384+
path: /_query
385+
parameters: []
386+
capabilities: [aggregate_metric_double_sorting]
387+
reason: "Support for sorting when aggregate_metric_double present"
388+
- do:
389+
allowed_warnings_regex:
390+
- "No limit defined, adding default limit of \\[.*\\]"
391+
esql.query:
392+
body:
393+
query: 'FROM test3 | SORT @timestamp | KEEP @timestamp, agg_metric'
394+
395+
- length: {values: 4}
396+
- length: {values.0: 2}
397+
- match: {columns.0.name: "@timestamp"}
398+
- match: {columns.0.type: "date"}
399+
- match: {columns.1.name: "agg_metric"}
400+
- match: {columns.1.type: "aggregate_metric_double"}
401+
- match: {values.0.0: "2021-04-28T19:50:04.467Z"}
402+
- match: {values.1.0: "2021-04-28T19:50:24.467Z"}
403+
- match: {values.2.0: "2021-04-28T19:50:44.467Z"}
404+
- match: {values.3.0: "2021-04-28T19:51:04.467Z"}
405+
- match: {values.0.1: '{"min":-3.0,"max":1.0}'}
406+
- match: {values.1.1: '{"min":3.0,"max":10.0}'}
407+
- match: {values.2.1: '{"min":2.0,"max":17.0}'}
408+
- match: {values.3.1: null}
409+
410+
---
411+
aggregate_metric_double unsortable:
412+
- requires:
413+
test_runner_features: [capabilities]
414+
capabilities:
415+
- method: POST
416+
path: /_query
417+
parameters: []
418+
capabilities: [aggregate_metric_double_sorting]
419+
reason: "Support for sorting when aggregate_metric_double present"
420+
- do:
421+
catch: /cannot sort on aggregate_metric_double/
422+
esql.query:
423+
body:
424+
query: 'FROM test2 | sort agg_metric'
425+
378426
---
379427
stats on aggregate_metric_double with partial submetrics:
380428
- requires:

0 commit comments

Comments
 (0)