diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregationBuilder.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregationBuilder.java index c17963db05c04..4a4a2a6844db6 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregationBuilder.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregationBuilder.java @@ -58,7 +58,7 @@ public static void registerAggregators(ValuesSourceRegistry.Builder registry) { registry.register( REGISTRY_KEY, List.of(CoreValuesSourceType.KEYWORD, CoreValuesSourceType.IP), - TopMetricsAggregator.GlobalOrdsValues::new, + TopMetricsAggregator.SegmentOrdsValues::new, false ); } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregator.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregator.java index 0059c7362950d..26943d269a0cb 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregator.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregator.java @@ -19,6 +19,7 @@ import org.elasticsearch.common.util.BitArray; import org.elasticsearch.common.util.DoubleArray; import org.elasticsearch.common.util.LongArray; +import org.elasticsearch.common.util.ObjectArray; import org.elasticsearch.index.fielddata.NumericDoubleValues; import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.MultiValueMode; @@ -33,8 +34,8 @@ import org.elasticsearch.search.sort.BucketedSort; import org.elasticsearch.search.sort.SortBuilder; import org.elasticsearch.search.sort.SortValue; -import org.elasticsearch.xpack.core.common.search.aggregations.MissingHelper; import org.elasticsearch.xpack.analytics.topmetrics.InternalTopMetrics.MetricValue; +import org.elasticsearch.xpack.core.common.search.aggregations.MissingHelper; import java.io.IOException; import java.util.ArrayList; @@ -398,20 +399,32 @@ public void close() { } /** - * Loads metrics for whole numbers. + * Loads metrics fields with segment ordinals. */ - static class GlobalOrdsValues extends CollectingMetricValues { + static class SegmentOrdsValues extends CollectingMetricValues { private final ValuesSource.Bytes.WithOrdinals valuesSource; - private SortedSetDocValues globalOrds; - private LongArray values; + /** + * Reference to the segment's doc values so we can convert the + * {@link #segmentOrds} into strings on the way out. + */ + private ObjectArray segmentResolve; + /** + * Terms ordinal in the segment. + */ + private LongArray segmentOrds; - GlobalOrdsValues(int size, BigArrays bigArrays, String name, ValuesSourceConfig config) { + SegmentOrdsValues(int size, BigArrays bigArrays, String name, ValuesSourceConfig config) { super(bigArrays, name, config); - if (false == config.hasGlobalOrdinals()) { - throw new IllegalArgumentException("top_metrics can only collect bytes that have global ordinals"); + try { + valuesSource = (ValuesSource.Bytes.WithOrdinals) config.getValuesSource(); + } catch (ClassCastException e) { + throw new IllegalArgumentException( + "top_metrics can only collect bytes that have segment ordinals but " + config.getDescription() + " does not", + e + ); } - this.valuesSource = (ValuesSource.Bytes.WithOrdinals) config.getValuesSource(); - values = bigArrays.newLongArray(size, false); + segmentResolve = bigArrays.newObjectArray(size); + segmentOrds = bigArrays.newLongArray(size, false); } @Override @@ -421,43 +434,48 @@ public double doubleValue(long index) { @Override public MetricValue metricValue(long index) throws IOException { - if (globalOrds == null) { - // We didn't collect a single segment. - return null; - } - long ord = values.get(index); + long ord = segmentOrds.get(index); if (ord == -1) { return null; } - return new MetricValue(config.format(), SortValue.from(BytesRef.deepCopyOf(globalOrds.lookupOrd(ord)))); + SortedSetDocValues resolve = segmentResolve.get(index); + return new MetricValue(config.format(), SortValue.from(BytesRef.deepCopyOf(resolve.lookupOrd(ord)))); } @Override public void swap(long lhs, long rhs) { - long tmp = values.get(lhs); - values.set(lhs, values.get(rhs)); - values.set(rhs, tmp); + SortedSetDocValues tempSegmentResolve = segmentResolve.get(lhs); + segmentResolve.set(lhs, segmentResolve.get(rhs)); + segmentResolve.set(rhs, tempSegmentResolve); + long tmpSegmentOrd = segmentOrds.get(lhs); + segmentOrds.set(lhs, segmentOrds.get(rhs)); + segmentOrds.set(rhs, tmpSegmentOrd); } @Override public Loader loader(LeafReaderContext ctx) throws IOException { - globalOrds = valuesSource.globalOrdinalsValues(ctx); + SortedSetDocValues segmentOrdValues = valuesSource.ordinalsValues(ctx); // For now just return the value that sorts first. return (index, doc) -> { - if (false == globalOrds.advanceExact(doc)) { - values.set(index, -1); - return; + if (index >= segmentResolve.size()) { + segmentResolve = bigArrays.grow(segmentResolve, index + 1); } - if (index >= values.size()) { - values = bigArrays.grow(values, index + 1); + if (index >= segmentOrds.size()) { + segmentOrds = bigArrays.grow(segmentOrds, index + 1); } - values.set(index, globalOrds.nextOrd()); + if (false == segmentOrdValues.advanceExact(doc)) { + segmentResolve.set(index, null); + segmentOrds.set(index, -1); + return; + } + segmentResolve.set(index, segmentOrdValues); + segmentOrds.set(index, segmentOrdValues.nextOrd()); }; } @Override public void close() { - Releasables.close(values); + Releasables.close(segmentResolve, segmentOrds); } } diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregatorMetricsTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregatorMetricsTests.java index b50ebee77e60a..98cfcfe910228 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregatorMetricsTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregatorMetricsTests.java @@ -227,9 +227,9 @@ private ValuesSourceConfig toConfig(SortedNumericDoubleValues values) throws IOE private ValuesSourceConfig toConfig(SortedSetDocValues values) throws IOException { ValuesSource.Bytes.WithOrdinals source = mock(ValuesSource.Bytes.WithOrdinals.class); - when(source.globalOrdinalsValues(null)).thenReturn(values); + when(source.ordinalsValues(null)).thenReturn(values); ValuesSourceConfig config = toConfig(source, CoreValuesSourceType.KEYWORD, DocValueFormat.RAW, true); - when(config.hasGlobalOrdinals()).thenReturn(true); + when(config.hasGlobalOrdinals()).thenReturn(false); // We don't need global orindals. Only segment ordinals. return config; } diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/analytics/top_metrics.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/analytics/top_metrics.yml index dd2305c858e24..092c6eafb0cd7 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/analytics/top_metrics.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/analytics/top_metrics.yml @@ -583,3 +583,69 @@ - match: { aggregations.tm.top.1.sort: [2] } - match: { aggregations.tm.top.2.metrics.animal\.keyword: chicken} - match: { aggregations.tm.top.2.sort: [3] } + +--- +"fetch runtime keyword fails with nice error message": + - do: + bulk: + index: test + refresh: true + body: + - '{"index": {}}' + - '{"s": 1, "animal": "cat"}' + - '{"index": {}}' + - '{"s": 2, "animal": "dog"}' + - '{"index": {}}' + - '{"s": 3, "animal": "chicken"}' + + - do: + catch: /top_metrics can only collect bytes that have segment ordinals but Field \[species\] of type \[keyword\] does not/ + search: + size: 0 + body: + runtime_mappings: + species: + type: keyword + script: | + emit(doc['animal.keyword'].value) + aggs: + tm: + top_metrics: + metrics: + field: species + sort: + s: asc + size: 3 + +--- +"fetch keyword fields with some missing": + - do: + bulk: + index: test + refresh: true + body: + - '{"index": {}}' + - '{"s": 1, "animal": "cat"}' + - '{"index": {}}' + - '{"s": 2, "animal": "dog"}' + - '{"index": {}}' + - '{"s": 3}' + + - do: + search: + size: 0 + body: + aggs: + tm: + top_metrics: + metrics: + field: animal.keyword + sort: + s: asc + size: 3 + - match: { aggregations.tm.top.0.metrics.animal\.keyword: cat} + - match: { aggregations.tm.top.0.sort: [1] } + - match: { aggregations.tm.top.1.metrics.animal\.keyword: dog} + - match: { aggregations.tm.top.1.sort: [2] } + - match: { aggregations.tm.top.2.metrics.animal\.keyword: null} + - match: { aggregations.tm.top.2.sort: [3] }