From 5087b9fb9697cb11ab8a88da84244c49e36cdd2a Mon Sep 17 00:00:00 2001 From: Lukas Wegmann Date: Mon, 13 Sep 2021 15:49:45 +0200 Subject: [PATCH] [7.x] Support "_first" and "_last" ordering of missing values in composite aggs (#76740) (#77620) * Support "_first" and "_last" ordering of missing values in composite aggs (#76740) * Support "first" and "last" ordering of missing values in composite aggs * skip API compat tests for 7.16 * use proper versions for bwc * use "missing_order" parameter instead of "missing" * address review comments # Conflicts: # server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceBuilder.java * adjust versions for BWC --- .../test/search.aggregation/230_composite.yml | 142 ++++++++ .../bucket/composite/BinaryValuesSource.java | 15 +- .../CompositeValuesSourceBuilder.java | 45 ++- .../CompositeValuesSourceConfig.java | 8 + .../CompositeValuesSourceParserHelper.java | 1 + .../composite/DateHistogramValuesSource.java | 13 +- .../DateHistogramValuesSourceBuilder.java | 7 +- .../bucket/composite/DoubleValuesSource.java | 17 +- .../GeoTileGridValuesSourceBuilder.java | 7 +- .../bucket/composite/GeoTileValuesSource.java | 3 +- .../HistogramValuesSourceBuilder.java | 7 +- .../bucket/composite/LongValuesSource.java | 15 +- .../bucket/composite/MissingOrder.java | 76 ++++ .../bucket/composite/OrdinalValuesSource.java | 23 +- .../SingleDimensionValuesSource.java | 4 + .../composite/TermsValuesSourceBuilder.java | 14 +- .../CompositeAggregationBuilderTests.java | 3 + .../composite/CompositeAggregatorTests.java | 335 ++++++++++++++++++ .../CompositeValuesCollectorQueueTests.java | 4 + .../SingleDimensionValuesSourceTests.java | 34 +- 20 files changed, 726 insertions(+), 47 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/MissingOrder.java diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/230_composite.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/230_composite.yml index 7e951ffea1d29..2c4d28fa6ce30 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/230_composite.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/230_composite.yml @@ -1392,3 +1392,145 @@ setup: - length: {aggregations.not_one.keez.buckets: 2} - match: {aggregations.not_one.keez.buckets.0.key.key: "three"} - match: {aggregations.not_one.keez.buckets.1.key.key: "two"} + +--- +"Simple Composite aggregation with missing_bucket": + - do: + search: + rest_total_hits_as_int: true + index: test + body: + aggregations: + test: + composite: + sources: [ + "kw": { + "terms": { + "field": "keyword", + "missing_bucket": true + } + } + ] + + - length: { aggregations.test.buckets: 3 } + - match: { aggregations.test.buckets.0.key.kw: null } + - match: { aggregations.test.buckets.0.doc_count: 2 } + +--- +"Simple Composite aggregation with missing_order": + - skip: + version: " - 7.15.99" + reason: "`missing_order` has been introduced in 7.16" + - do: + search: + rest_total_hits_as_int: true + index: test + body: + aggregations: + test: + composite: + sources: [ + "kw": { + "terms": { + "field": "keyword", + "missing_bucket": true, + "missing_order": "last" + } + } + ] + + - length: { aggregations.test.buckets: 3 } + - match: { aggregations.test.buckets.2.key.kw: null } + - match: { aggregations.test.buckets.2.doc_count: 2 } + +--- +"missing_order with missing_bucket = false": + - skip: + version: " - 7.15.99" + reason: "`missing_order` has been introduced in 7.16" + - do: + catch: /missingOrder can only be set if missingBucket is true/ + search: + rest_total_hits_as_int: true + index: test + body: + aggregations: + test: + composite: + sources: [ + "kw": { + "terms": { + "field": "keyword", + "missing_bucket": false, + "missing_order": "first" + } + } + ] + +--- +"missing_order without missing_bucket": + - skip: + version: " - 7.15.99" + reason: "`missing_order` has been introduced in 7.16" + - do: + catch: /missingOrder can only be set if missingBucket is true/ + search: + rest_total_hits_as_int: true + index: test + body: + aggregations: + test: + composite: + sources: [ + "kw": { + "terms": { + "field": "keyword", + "missing_order": "first" + } + } + ] + +--- +"Nested Composite aggregation with missing_order": + - skip: + version: " - 7.15.99" + reason: "`missing_order` has been introduced in 7.16" + - do: + search: + rest_total_hits_as_int: true + index: test + body: + aggregations: + test: + composite: + sources: [ + { + "long": { + "terms": { + "field": "long", + "missing_bucket": true, + "missing_order": "default" + } + } + }, + { + "kw": { + "terms": { + "field": "keyword", + "missing_bucket": true, + "missing_order": "last" + } + } + } + ] + + - length: { aggregations.test.buckets: 8 } + - match: { aggregations.test.buckets.0.key.long: null} + - match: { aggregations.test.buckets.0.key.kw: "bar" } + - match: { aggregations.test.buckets.0.doc_count: 1 } + - match: { aggregations.test.buckets.1.key.long: null} + - match: { aggregations.test.buckets.1.key.kw: "foo" } + - match: { aggregations.test.buckets.1.doc_count: 1 } + - match: { aggregations.test.buckets.2.key.long: null} + - match: { aggregations.test.buckets.2.key.kw: null } + - match: { aggregations.test.buckets.2.doc_count: 2 } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/BinaryValuesSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/BinaryValuesSource.java index 04ef6088a3f09..8d8cf61353ebf 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/BinaryValuesSource.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/BinaryValuesSource.java @@ -44,10 +44,11 @@ class BinaryValuesSource extends SingleDimensionValuesSource { CheckedFunction docValuesFunc, DocValueFormat format, boolean missingBucket, + MissingOrder missingOrder, int size, int reverseMul ) { - super(bigArrays, format, fieldType, missingBucket, size, reverseMul); + super(bigArrays, format, fieldType, missingBucket, missingOrder, size, reverseMul); this.breakerConsumer = breakerConsumer; this.docValuesFunc = docValuesFunc; this.values = bigArrays.newObjectArray(Math.min(size, 100)); @@ -78,9 +79,9 @@ void copyCurrent(int slot) { int compare(int from, int to) { if (missingBucket) { if (values.get(from) == null) { - return values.get(to) == null ? 0 : -1 * reverseMul; + return values.get(to) == null ? 0 : -1 * missingOrder.compareAnyValueToMissing(reverseMul); } else if (values.get(to) == null) { - return reverseMul; + return missingOrder.compareAnyValueToMissing(reverseMul); } } return compareValues(values.get(from), values.get(to)); @@ -90,9 +91,9 @@ int compare(int from, int to) { int compareCurrent(int slot) { if (missingBucket) { if (currentValue == null) { - return values.get(slot) == null ? 0 : -1 * reverseMul; + return values.get(slot) == null ? 0 : -1 * missingOrder.compareAnyValueToMissing(reverseMul); } else if (values.get(slot) == null) { - return reverseMul; + return missingOrder.compareAnyValueToMissing(reverseMul); } } return compareValues(currentValue, values.get(slot)); @@ -102,9 +103,9 @@ int compareCurrent(int slot) { int compareCurrentWithAfter() { if (missingBucket) { if (currentValue == null) { - return afterValue == null ? 0 : -1 * reverseMul; + return afterValue == null ? 0 : -1 * missingOrder.compareAnyValueToMissing(reverseMul); } else if (afterValue == null) { - return reverseMul; + return missingOrder.compareAnyValueToMissing(reverseMul); } } return compareValues(currentValue, afterValue); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceBuilder.java index 6d147f1159437..922d23e42cb52 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceBuilder.java @@ -37,6 +37,7 @@ public abstract class CompositeValuesSourceBuilder createValuesSource( private final DocValueFormat format; private final int reverseMul; private final boolean missingBucket; + private final MissingOrder missingOrder; private final boolean hasScript; private final SingleDimensionValuesSourceProvider singleDimensionValuesSourceProvider; @@ -50,6 +51,7 @@ SingleDimensionValuesSource createValuesSource( * @param format The {@link DocValueFormat} of this source. * @param order The sort order associated with this source. * @param missingBucket If true an explicit null bucket will represent documents with missing values. + * @param missingOrder How to order missing buckets if missingBucket is true. * @param hasScript true if the source contains a script that can change the value. */ CompositeValuesSourceConfig( @@ -59,6 +61,7 @@ SingleDimensionValuesSource createValuesSource( DocValueFormat format, SortOrder order, boolean missingBucket, + MissingOrder missingOrder, boolean hasScript, SingleDimensionValuesSourceProvider singleDimensionValuesSourceProvider ) { @@ -68,6 +71,7 @@ SingleDimensionValuesSource createValuesSource( this.format = format; this.reverseMul = order == SortOrder.ASC ? 1 : -1; this.missingBucket = missingBucket; + this.missingOrder = missingOrder; this.hasScript = hasScript; this.singleDimensionValuesSourceProvider = singleDimensionValuesSourceProvider; } @@ -108,6 +112,10 @@ boolean missingBucket() { return missingBucket; } + MissingOrder missingOrder() { + return missingOrder; + } + /** * Returns true if the source contains a script that can change the value. */ diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceParserHelper.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceParserHelper.java index 060654aa4a664..1c4a55935f5f3 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceParserHelper.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceParserHelper.java @@ -30,6 +30,7 @@ public class CompositeValuesSourceParserHelper { static , T> void declareValuesSourceFields(AbstractObjectParser objectParser) { objectParser.declareField(VB::field, XContentParser::text, new ParseField("field"), ObjectParser.ValueType.STRING); objectParser.declareBoolean(VB::missingBucket, new ParseField("missing_bucket")); + objectParser.declareString(VB::missingOrder, new ParseField("missing_order")); objectParser.declareField(VB::userValuetypeHint, p -> { ValueType valueType = ValueType.lenientParse(p.text()); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DateHistogramValuesSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DateHistogramValuesSource.java index a93fd04936ea3..54a00dbdc2d38 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DateHistogramValuesSource.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DateHistogramValuesSource.java @@ -26,10 +26,21 @@ public class DateHistogramValuesSource extends LongValuesSource implements Sized RoundingValuesSource roundingValuesSource, DocValueFormat format, boolean missingBucket, + MissingOrder missingOrder, int size, int reverseMul ) { - super(bigArrays, fieldType, roundingValuesSource::longValues, roundingValuesSource::round, format, missingBucket, size, reverseMul); + super( + bigArrays, + fieldType, + roundingValuesSource::longValues, + roundingValuesSource::round, + format, + missingBucket, + missingOrder, + size, + reverseMul + ); this.preparedRounding = roundingValuesSource; } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DateHistogramValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DateHistogramValuesSourceBuilder.java index 849235c7d371f..fa28880daf347 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DateHistogramValuesSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DateHistogramValuesSourceBuilder.java @@ -58,6 +58,7 @@ CompositeValuesSourceConfig apply( boolean hasScript, // probably redundant with the config, but currently we check this two different ways... String format, boolean missingBucket, + MissingOrder missingOrder, SortOrder order ); } @@ -271,7 +272,7 @@ public static void register(ValuesSourceRegistry.Builder builder) { builder.register( REGISTRY_KEY, List.of(CoreValuesSourceType.DATE, CoreValuesSourceType.NUMERIC), - (valuesSourceConfig, rounding, name, hasScript, format, missingBucket, order) -> { + (valuesSourceConfig, rounding, name, hasScript, format, missingBucket, missingOrder, order) -> { ValuesSource.Numeric numeric = (ValuesSource.Numeric) valuesSourceConfig.getValuesSource(); // TODO once composite is plugged in to the values source registry or at least understands Date values source types use it // here @@ -287,6 +288,7 @@ public static void register(ValuesSourceRegistry.Builder builder) { docValueFormat, order, missingBucket, + missingOrder, hasScript, ( BigArrays bigArrays, @@ -301,6 +303,7 @@ public static void register(ValuesSourceRegistry.Builder builder) { roundingValuesSource, compositeValuesSourceConfig.format(), compositeValuesSourceConfig.missingBucket(), + compositeValuesSourceConfig.missingOrder(), size, compositeValuesSourceConfig.reverseMul() ); @@ -320,6 +323,6 @@ protected ValuesSourceType getDefaultValuesSourceType() { protected CompositeValuesSourceConfig innerBuild(ValuesSourceRegistry registry, ValuesSourceConfig config) throws IOException { Rounding rounding = dateHistogramInterval.createRounding(timeZone(), offset); return registry.getAggregator(REGISTRY_KEY, config) - .apply(config, rounding, name, config.script() != null, format(), missingBucket(), order()); + .apply(config, rounding, name, config.script() != null, format(), missingBucket(), missingOrder(), order()); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DoubleValuesSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DoubleValuesSource.java index 747e3a1b14a76..bf75097beca9a 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DoubleValuesSource.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DoubleValuesSource.java @@ -39,12 +39,13 @@ class DoubleValuesSource extends SingleDimensionValuesSource { CheckedFunction docValuesFunc, DocValueFormat format, boolean missingBucket, + MissingOrder missingOrder, int size, int reverseMul ) { - super(bigArrays, format, fieldType, missingBucket, size, reverseMul); + super(bigArrays, format, fieldType, missingBucket, missingOrder, size, reverseMul); this.docValuesFunc = docValuesFunc; - this.bits = missingBucket ? new BitArray(100, bigArrays) : null; + this.bits = this.missingBucket ? new BitArray(100, bigArrays) : null; this.values = bigArrays.newDoubleArray(Math.min(size, 100), false); } @@ -66,9 +67,9 @@ void copyCurrent(int slot) { int compare(int from, int to) { if (missingBucket) { if (bits.get(from) == false) { - return bits.get(to) ? -1 * reverseMul : 0; + return bits.get(to) ? -1 * missingOrder.compareAnyValueToMissing(reverseMul) : 0; } else if (bits.get(to) == false) { - return reverseMul; + return missingOrder.compareAnyValueToMissing(reverseMul); } } return compareValues(values.get(from), values.get(to)); @@ -78,9 +79,9 @@ int compare(int from, int to) { int compareCurrent(int slot) { if (missingBucket) { if (missingCurrentValue) { - return bits.get(slot) ? -1 * reverseMul : 0; + return bits.get(slot) ? -1 * missingOrder.compareAnyValueToMissing(reverseMul) : 0; } else if (bits.get(slot) == false) { - return reverseMul; + return missingOrder.compareAnyValueToMissing(reverseMul); } } return compareValues(currentValue, values.get(slot)); @@ -90,9 +91,9 @@ int compareCurrent(int slot) { int compareCurrentWithAfter() { if (missingBucket) { if (missingCurrentValue) { - return afterValue != null ? -1 * reverseMul : 0; + return afterValue != null ? -1 * missingOrder.compareAnyValueToMissing(reverseMul) : 0; } else if (afterValue == null) { - return reverseMul; + return missingOrder.compareAnyValueToMissing(reverseMul); } } return compareValues(currentValue, afterValue); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java index 323d03ebf9401..b8b3555808d30 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileGridValuesSourceBuilder.java @@ -47,6 +47,7 @@ CompositeValuesSourceConfig apply( boolean hasScript, // probably redundant with the config, but currently we check this two different ways... String format, boolean missingBucket, + MissingOrder missingOrder, SortOrder order ); } @@ -78,7 +79,7 @@ static void register(ValuesSourceRegistry.Builder builder) { builder.register( REGISTRY_KEY, CoreValuesSourceType.GEOPOINT, - (valuesSourceConfig, precision, boundingBox, name, hasScript, format, missingBucket, order) -> { + (valuesSourceConfig, precision, boundingBox, name, hasScript, format, missingBucket, missingOrder, order) -> { ValuesSource.GeoPoint geoPoint = (ValuesSource.GeoPoint) valuesSourceConfig.getValuesSource(); // is specified in the builder. final MappedFieldType fieldType = valuesSourceConfig.fieldType(); @@ -90,6 +91,7 @@ static void register(ValuesSourceRegistry.Builder builder) { DocValueFormat.GEOTILE, order, missingBucket, + missingOrder, hasScript, ( BigArrays bigArrays, @@ -107,6 +109,7 @@ static void register(ValuesSourceRegistry.Builder builder) { LongUnaryOperator.identity(), compositeValuesSourceConfig.format(), compositeValuesSourceConfig.missingBucket(), + compositeValuesSourceConfig.missingOrder(), size, compositeValuesSourceConfig.reverseMul() ); @@ -196,7 +199,7 @@ protected ValuesSourceType getDefaultValuesSourceType() { @Override protected CompositeValuesSourceConfig innerBuild(ValuesSourceRegistry registry, ValuesSourceConfig config) throws IOException { return registry.getAggregator(REGISTRY_KEY, config) - .apply(config, precision, geoBoundingBox(), name, script() != null, format(), missingBucket(), order()); + .apply(config, precision, geoBoundingBox(), name, script() != null, format(), missingBucket(), missingOrder(), order()); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileValuesSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileValuesSource.java index df406b29c726a..174e684bcabad 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileValuesSource.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/GeoTileValuesSource.java @@ -32,10 +32,11 @@ class GeoTileValuesSource extends LongValuesSource { LongUnaryOperator rounding, DocValueFormat format, boolean missingBucket, + MissingOrder missingOrder, int size, int reverseMul ) { - super(bigArrays, fieldType, docValuesFunc, rounding, format, missingBucket, size, reverseMul); + super(bigArrays, fieldType, docValuesFunc, rounding, format, missingBucket, missingOrder, size, reverseMul); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/HistogramValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/HistogramValuesSourceBuilder.java index 30790ddf39bff..dd6a0cc1b6aae 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/HistogramValuesSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/HistogramValuesSourceBuilder.java @@ -43,6 +43,7 @@ CompositeValuesSourceConfig apply( boolean hasScript, // probably redundant with the config, but currently we check this two different ways... String format, boolean missingBucket, + MissingOrder missingOrder, SortOrder order ); } @@ -68,7 +69,7 @@ public static void register(ValuesSourceRegistry.Builder builder) { builder.register( REGISTRY_KEY, List.of(CoreValuesSourceType.DATE, CoreValuesSourceType.NUMERIC), - (valuesSourceConfig, interval, name, hasScript, format, missingBucket, order) -> { + (valuesSourceConfig, interval, name, hasScript, format, missingBucket, missingOrder, order) -> { ValuesSource.Numeric numeric = (ValuesSource.Numeric) valuesSourceConfig.getValuesSource(); final HistogramValuesSource vs = new HistogramValuesSource(numeric, interval); final MappedFieldType fieldType = valuesSourceConfig.fieldType(); @@ -79,6 +80,7 @@ public static void register(ValuesSourceRegistry.Builder builder) { valuesSourceConfig.format(), order, missingBucket, + missingOrder, hasScript, ( BigArrays bigArrays, @@ -93,6 +95,7 @@ public static void register(ValuesSourceRegistry.Builder builder) { numericValuesSource::doubleValues, compositeValuesSourceConfig.format(), compositeValuesSourceConfig.missingBucket(), + compositeValuesSourceConfig.missingOrder(), size, compositeValuesSourceConfig.reverseMul() ); @@ -169,6 +172,6 @@ protected ValuesSourceType getDefaultValuesSourceType() { @Override protected CompositeValuesSourceConfig innerBuild(ValuesSourceRegistry registry, ValuesSourceConfig config) throws IOException { return registry.getAggregator(REGISTRY_KEY, config) - .apply(config, interval, name, script() != null, format(), missingBucket(), order()); + .apply(config, interval, name, script() != null, format(), missingBucket(), missingOrder(), order()); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/LongValuesSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/LongValuesSource.java index 78d5c22bbd64b..5bc9ee0906fe8 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/LongValuesSource.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/LongValuesSource.java @@ -55,10 +55,11 @@ class LongValuesSource extends SingleDimensionValuesSource { LongUnaryOperator rounding, DocValueFormat format, boolean missingBucket, + MissingOrder missingOrder, int size, int reverseMul ) { - super(bigArrays, format, fieldType, missingBucket, size, reverseMul); + super(bigArrays, format, fieldType, missingBucket, missingOrder, size, reverseMul); this.bigArrays = bigArrays; this.docValuesFunc = docValuesFunc; this.rounding = rounding; @@ -84,9 +85,9 @@ void copyCurrent(int slot) { int compare(int from, int to) { if (missingBucket) { if (bits.get(from) == false) { - return bits.get(to) ? -1 * reverseMul : 0; + return bits.get(to) ? -1 * missingOrder.compareAnyValueToMissing(reverseMul) : 0; } else if (bits.get(to) == false) { - return reverseMul; + return missingOrder.compareAnyValueToMissing(reverseMul); } } return compareValues(values.get(from), values.get(to)); @@ -96,9 +97,9 @@ int compare(int from, int to) { int compareCurrent(int slot) { if (missingBucket) { if (missingCurrentValue) { - return bits.get(slot) ? -1 * reverseMul : 0; + return bits.get(slot) ? -1 * missingOrder.compareAnyValueToMissing(reverseMul) : 0; } else if (bits.get(slot) == false) { - return reverseMul; + return missingOrder.compareAnyValueToMissing(reverseMul); } } return compareValues(currentValue, values.get(slot)); @@ -108,9 +109,9 @@ int compareCurrent(int slot) { int compareCurrentWithAfter() { if (missingBucket) { if (missingCurrentValue) { - return afterValue != null ? -1 * reverseMul : 0; + return afterValue != null ? -1 * missingOrder.compareAnyValueToMissing(reverseMul) : 0; } else if (afterValue == null) { - return reverseMul; + return missingOrder.compareAnyValueToMissing(reverseMul); } } return compareValues(currentValue, afterValue); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/MissingOrder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/MissingOrder.java new file mode 100644 index 0000000000000..b219d495cc344 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/MissingOrder.java @@ -0,0 +1,76 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.search.aggregations.bucket.composite; + +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; + +import java.io.IOException; +import java.util.Locale; + +public enum MissingOrder implements Writeable { + /** + * Place buckets for missing values first if the source uses ASC ordering or last otherwise. + */ + DEFAULT { + @Override + public int compareAnyValueToMissing(int reverseMul) { + return reverseMul; + } + + @Override + public String toString() { + return "default"; + } + }, + /** + * Place buckets for missing values first. + */ + FIRST { + @Override + public int compareAnyValueToMissing(int reverseMul) { + return 1; + } + + @Override + public String toString() { + return "first"; + } + }, + /** + * Place buckets for missing values last. + */ + LAST { + @Override + public int compareAnyValueToMissing(int reverseMul) { + return -1; + } + + @Override + public String toString() { + return "last"; + } + }; + + public abstract int compareAnyValueToMissing(int reverseMul); + + public static MissingOrder readFromStream(StreamInput in) throws IOException { + return in.readEnum(MissingOrder.class); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeEnum(this); + } + + public static MissingOrder fromString(String op) { + return valueOf(op.toUpperCase(Locale.ROOT)); + } +} diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/OrdinalValuesSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/OrdinalValuesSource.java index 79affbaff7039..9c12805215f65 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/OrdinalValuesSource.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/OrdinalValuesSource.java @@ -73,10 +73,11 @@ class OrdinalValuesSource extends SingleDimensionValuesSource { CheckedFunction docValuesFunc, DocValueFormat format, boolean missingBucket, + MissingOrder missingOrder, int size, int reverseMul ) { - super(bigArrays, format, type, missingBucket, size, reverseMul); + super(bigArrays, format, type, missingBucket, missingOrder, size, reverseMul); this.breakerConsumer = breakerConsumer; this.docValuesFunc = docValuesFunc; this.valuesOrd = bigArrays.newLongArray(Math.min(size, 100), false); @@ -145,20 +146,20 @@ private void setValueWithBreaking(long index, BytesRef newValue) { @Override int compare(int from, int to) { assert from < numSlots && to < numSlots; - return compareInternal(valuesOrd.get(from), valuesOrd.get(to), valuesUnmapped.get(from), valuesUnmapped.get(to)) * reverseMul; + return compareInternal(valuesOrd.get(from), valuesOrd.get(to), valuesUnmapped.get(from), valuesUnmapped.get(to)); } @Override int compareCurrent(int slot) { assert currentValueOrd != null; assert slot < numSlots; - return compareInternal(currentValueOrd, valuesOrd.get(slot), currentValueUnmapped, valuesUnmapped.get(slot)) * reverseMul; + return compareInternal(currentValueOrd, valuesOrd.get(slot), currentValueUnmapped, valuesUnmapped.get(slot)); } @Override int compareCurrentWithAfter() { assert currentValueOrd != null && afterValueOrd != null; - return compareInternal(currentValueOrd, afterValueOrd, currentValueUnmapped, afterValue) * reverseMul; + return compareInternal(currentValueOrd, afterValueOrd, currentValueUnmapped, afterValue); } @Override @@ -175,31 +176,31 @@ int hashCodeCurrent() { private int compareInternal(long ord1, long ord2, BytesRef bytesRef1, BytesRef bytesRef2) { if (ord1 >= 0 && ord2 >= 0) { - return Long.compare(ord1, ord2); + return Long.compare(ord1, ord2) * reverseMul; } else if (ord1 == Long.MIN_VALUE || ord2 == Long.MIN_VALUE) { - return Long.compare(ord1, ord2); + return Long.compare(ord1, ord2) * missingOrder.compareAnyValueToMissing(reverseMul); } else if (ord1 < 0 && ord2 < 0) { if (ord1 == ord2) { // we need to compare actual terms to properly order assert bytesRef1 != null && bytesRef2 != null; return bytesRef1.compareTo(bytesRef2); } - return Long.compare(-ord1 - 1, -ord2 - 1); + return Long.compare(-ord1 - 1, -ord2 - 1) * reverseMul; } else { if (ord1 < 0) { assert ord1 < 0 && ord2 >= 0; int cmp = Long.compare(-ord1 - 1, ord2); if (cmp == 0) { - return -1; + return -1 * reverseMul; } - return cmp; + return cmp * reverseMul; } else { assert ord1 >= 0 && ord2 < 0; int cmp = Long.compare(ord1, -ord2 - 1); if (cmp == 0) { - return 1; + return reverseMul; } - return cmp; + return cmp * reverseMul; } } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java index 7b5d4ca7d3351..9a2e5c97e0366 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/SingleDimensionValuesSource.java @@ -30,6 +30,7 @@ abstract class SingleDimensionValuesSource> implements R @Nullable protected final MappedFieldType fieldType; protected final boolean missingBucket; + protected final MissingOrder missingOrder; protected final int size; protected final int reverseMul; @@ -43,6 +44,7 @@ abstract class SingleDimensionValuesSource> implements R * @param format The format of the source. * @param fieldType The field type or null if the source is a script. * @param missingBucket If true, an explicit `null bucket represents documents with missing values. + * @param missingOrder How to order missing buckets if missingBucket is true. * @param size The number of values to record. * @param reverseMul -1 if the natural order ({@link SortOrder#ASC} should be reversed. */ @@ -51,6 +53,7 @@ abstract class SingleDimensionValuesSource> implements R DocValueFormat format, @Nullable MappedFieldType fieldType, boolean missingBucket, + MissingOrder missingOrder, int size, int reverseMul ) { @@ -58,6 +61,7 @@ abstract class SingleDimensionValuesSource> implements R this.format = format; this.fieldType = fieldType; this.missingBucket = missingBucket; + this.missingOrder = missingOrder; this.size = size; this.reverseMul = reverseMul; this.afterValue = null; diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/TermsValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/TermsValuesSourceBuilder.java index 192a5d71aac03..15c3e16b3c26c 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/TermsValuesSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/TermsValuesSourceBuilder.java @@ -44,6 +44,7 @@ CompositeValuesSourceConfig apply( boolean hasScript, // probably redundant with the config, but currently we check this two different ways... String format, boolean missingBucket, + MissingOrder missingOrder, SortOrder order ); } @@ -87,7 +88,7 @@ static void register(ValuesSourceRegistry.Builder builder) { builder.register( REGISTRY_KEY, List.of(CoreValuesSourceType.DATE, CoreValuesSourceType.NUMERIC, CoreValuesSourceType.BOOLEAN), - (valuesSourceConfig, name, hasScript, format, missingBucket, order) -> { + (valuesSourceConfig, name, hasScript, format, missingBucket, missingOrder, order) -> { final DocValueFormat docValueFormat; if (format == null && valuesSourceConfig.valueSourceType() == CoreValuesSourceType.DATE) { // defaults to the raw format on date fields (preserve timestamp as longs). @@ -102,6 +103,7 @@ static void register(ValuesSourceRegistry.Builder builder) { docValueFormat, order, missingBucket, + missingOrder, hasScript, ( BigArrays bigArrays, @@ -118,6 +120,7 @@ static void register(ValuesSourceRegistry.Builder builder) { vs::doubleValues, compositeValuesSourceConfig.format(), compositeValuesSourceConfig.missingBucket(), + compositeValuesSourceConfig.missingOrder(), size, compositeValuesSourceConfig.reverseMul() ); @@ -132,6 +135,7 @@ static void register(ValuesSourceRegistry.Builder builder) { rounding, compositeValuesSourceConfig.format(), compositeValuesSourceConfig.missingBucket(), + compositeValuesSourceConfig.missingOrder(), size, compositeValuesSourceConfig.reverseMul() ); @@ -146,13 +150,14 @@ static void register(ValuesSourceRegistry.Builder builder) { builder.register( REGISTRY_KEY, List.of(CoreValuesSourceType.KEYWORD, CoreValuesSourceType.IP), - (valuesSourceConfig, name, hasScript, format, missingBucket, order) -> new CompositeValuesSourceConfig( + (valuesSourceConfig, name, hasScript, format, missingBucket, missingOrder, order) -> new CompositeValuesSourceConfig( name, valuesSourceConfig.fieldType(), valuesSourceConfig.getValuesSource(), valuesSourceConfig.format(), order, missingBucket, + missingOrder, hasScript, ( BigArrays bigArrays, @@ -170,6 +175,7 @@ static void register(ValuesSourceRegistry.Builder builder) { vs::ordinalsValues, compositeValuesSourceConfig.format(), compositeValuesSourceConfig.missingBucket(), + compositeValuesSourceConfig.missingOrder(), size, compositeValuesSourceConfig.reverseMul() ); @@ -182,6 +188,7 @@ static void register(ValuesSourceRegistry.Builder builder) { vs::bytesValues, compositeValuesSourceConfig.format(), compositeValuesSourceConfig.missingBucket(), + compositeValuesSourceConfig.missingOrder(), size, compositeValuesSourceConfig.reverseMul() ); @@ -199,6 +206,7 @@ protected ValuesSourceType getDefaultValuesSourceType() { @Override protected CompositeValuesSourceConfig innerBuild(ValuesSourceRegistry registry, ValuesSourceConfig config) throws IOException { - return registry.getAggregator(REGISTRY_KEY, config).apply(config, name, script() != null, format(), missingBucket(), order()); + return registry.getAggregator(REGISTRY_KEY, config) + .apply(config, name, script() != null, format(), missingBucket(), missingOrder(), order()); } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java index de1a900a45800..edc04e5f0e704 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilderTests.java @@ -45,6 +45,7 @@ private DateHistogramValuesSourceBuilder randomDateHistogramSourceBuilder() { if (randomBoolean()) { histo.missingBucket(true); } + histo.missingOrder(randomFrom(MissingOrder.values())); return histo; } @@ -70,6 +71,7 @@ private TermsValuesSourceBuilder randomTermsSourceBuilder() { if (randomBoolean()) { terms.missingBucket(true); } + terms.missingOrder(randomFrom(MissingOrder.values())); return terms; } @@ -83,6 +85,7 @@ private HistogramValuesSourceBuilder randomHistogramSourceBuilder() { if (randomBoolean()) { histo.missingBucket(true); } + histo.missingOrder(randomFrom(MissingOrder.values())); histo.interval(randomDoubleBetween(Math.nextUp(0), Double.MAX_VALUE, false)); return histo; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java index 0ae4a5c2b1efb..3f12f4e0b9891 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java @@ -1207,6 +1207,341 @@ public void testWithKeywordLongAndMissingBucket() throws Exception { assertEquals(1L, result.getBuckets().get(1).getDocCount()); } ); + + Consumer verifyMissingFirst = (result) -> { + assertEquals(7, result.getBuckets().size()); + assertEquals("{keyword=null, long=null}", result.getBuckets().get(0).getKeyAsString()); + assertEquals("{keyword=null, long=100}", result.getBuckets().get(1).getKeyAsString()); + }; + + testSearchCase( + Arrays.asList(new MatchAllDocsQuery()), + dataset, + () -> new CompositeAggregationBuilder( + "name", + Arrays.asList( + new TermsValuesSourceBuilder("keyword").field("keyword").missingBucket(true).missingOrder(MissingOrder.FIRST), + new TermsValuesSourceBuilder("long").field("long").missingBucket(true).missingOrder(MissingOrder.FIRST) + ) + ), + verifyMissingFirst + ); + + testSearchCase( + Arrays.asList(new MatchAllDocsQuery()), + dataset, + () -> new CompositeAggregationBuilder( + "name", + Arrays.asList( + new TermsValuesSourceBuilder("keyword").field("keyword") + .order(SortOrder.DESC) + .missingBucket(true) + .missingOrder(MissingOrder.FIRST), + new TermsValuesSourceBuilder("long").field("long") + .order(SortOrder.DESC) + .missingBucket(true) + .missingOrder(MissingOrder.FIRST) + ) + ), + verifyMissingFirst + ); + + Consumer verifyMissingLast = (result) -> { + assertEquals(7, result.getBuckets().size()); + assertEquals("{keyword=null, long=100}", result.getBuckets().get(5).getKeyAsString()); + assertEquals("{keyword=null, long=null}", result.getBuckets().get(6).getKeyAsString()); + }; + + testSearchCase( + Arrays.asList(new MatchAllDocsQuery()), + dataset, + () -> new CompositeAggregationBuilder( + "name", + Arrays.asList( + new TermsValuesSourceBuilder("keyword").field("keyword").missingBucket(true).missingOrder(MissingOrder.LAST), + new TermsValuesSourceBuilder("long").field("long").missingBucket(true).missingOrder(MissingOrder.LAST) + ) + ), + verifyMissingLast + ); + + testSearchCase( + Arrays.asList(new MatchAllDocsQuery()), + dataset, + () -> new CompositeAggregationBuilder( + "name", + Arrays.asList( + new TermsValuesSourceBuilder("keyword").field("keyword") + .order(SortOrder.DESC) + .missingBucket(true) + .missingOrder(MissingOrder.LAST), + new TermsValuesSourceBuilder("long").field("long") + .order(SortOrder.DESC) + .missingBucket(true) + .missingOrder(MissingOrder.LAST) + ) + ), + verifyMissingLast + ); + } + + public void testMissingTermBucket() throws Exception { + List>> dataset = Arrays.asList( + createDocument("const", 1, "keyword", "a"), + createDocument("const", 1, "keyword", "b"), + createDocument("const", 1, "long", 1) + ); + + testMissingBucket(dataset, new TermsValuesSourceBuilder("keyword").field("keyword").order(SortOrder.ASC), null); + testMissingBucket(dataset, new TermsValuesSourceBuilder("keyword").field("keyword").missingBucket(true).order(SortOrder.ASC), 0); + testMissingBucket(dataset, new TermsValuesSourceBuilder("keyword").field("keyword").missingBucket(true).order(SortOrder.DESC), 2); + testMissingBucket( + dataset, + new TermsValuesSourceBuilder("keyword").field("keyword") + .missingBucket(true) + .missingOrder(MissingOrder.FIRST) + .order(randomFrom(SortOrder.DESC, SortOrder.ASC)), + 0 + ); + testMissingBucket( + dataset, + new TermsValuesSourceBuilder("keyword").field("keyword") + .missingBucket(true) + .missingOrder(MissingOrder.LAST) + .order(randomFrom(SortOrder.DESC, SortOrder.ASC)), + 2 + ); + } + + public void testMissingHistogramBucket() throws Exception { + List>> dataset = Arrays.asList( + createDocument("const", 1, "long", 1), + createDocument("const", 1, "long", 2), + createDocument("const", 1, "keyword", "a") + ); + + testMissingBucket( + dataset, + new HistogramValuesSourceBuilder("hist").interval(1).field("long").missingBucket(false).order(SortOrder.ASC), + null + ); + testMissingBucket( + dataset, + new HistogramValuesSourceBuilder("hist").interval(1).field("long").missingBucket(true).order(SortOrder.ASC), + 0 + ); + testMissingBucket( + dataset, + new HistogramValuesSourceBuilder("hist").interval(1).field("long").missingBucket(true).order(SortOrder.DESC), + 2 + ); + testMissingBucket( + dataset, + new HistogramValuesSourceBuilder("hist").interval(1) + .field("long") + .missingBucket(true) + .missingOrder(MissingOrder.FIRST) + .order(randomFrom(SortOrder.DESC, SortOrder.ASC)), + 0 + ); + testMissingBucket( + dataset, + new HistogramValuesSourceBuilder("hist").interval(1) + .field("long") + .missingBucket(true) + .missingOrder(MissingOrder.LAST) + .order(randomFrom(SortOrder.DESC, SortOrder.ASC)), + 2 + ); + } + + private void testMissingBucket( + List>> dataset, + CompositeValuesSourceBuilder sourceBuilder, + Integer expectedMissingIndex + ) throws IOException { + testSearchCase( + Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("const")), + dataset, + () -> new CompositeAggregationBuilder("name", Collections.singletonList(sourceBuilder)), + (result) -> { + if (expectedMissingIndex == null) { + for (InternalComposite.InternalBucket bucket : result.getBuckets()) { + assertFalse(bucket.getKey().containsValue(null)); + } + } else { + assertTrue(result.getBuckets().get(expectedMissingIndex).getKey().containsValue(null)); + assertEquals(1, result.getBuckets().get(expectedMissingIndex).getKey().size()); + assertEquals(1, result.getBuckets().get(expectedMissingIndex).getDocCount()); + } + } + ); + } + + public void testMissingTermBucketAfterKey() throws Exception { + List>> dataset = Arrays.asList( + createDocument("const", 1, "keyword", "a"), + createDocument("const", 1, "keyword", "b"), + createDocument("const", 1, "long", 1), + createDocument("const", 1, "long", 2) + ); + + testSearchCase( + Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("const")), + dataset, + () -> new CompositeAggregationBuilder( + "name", + Collections.singletonList( + new TermsValuesSourceBuilder("keyword").field("keyword") + .missingBucket(true) + .missingOrder(MissingOrder.FIRST) + .order(SortOrder.ASC) + ) + ).aggregateAfter(createAfterKey("keyword", null)), + (result) -> { + assertEquals(2, result.getBuckets().size()); + assertEquals("{keyword=a}", result.getBuckets().get(0).getKeyAsString()); + assertEquals("{keyword=b}", result.getBuckets().get(1).getKeyAsString()); + } + ); + + testSearchCase( + Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("const")), + dataset, + () -> new CompositeAggregationBuilder( + "name", + Arrays.asList( + new TermsValuesSourceBuilder("keyword").field("keyword") + .missingBucket(true) + .missingOrder(MissingOrder.FIRST) + .order(SortOrder.ASC), + new TermsValuesSourceBuilder("long").field("long") + ) + ).aggregateAfter(createAfterKey("keyword", null, "long", 1)), + (result) -> { + assertEquals(1, result.getBuckets().size()); + assertEquals("{keyword=null, long=2}", result.getBuckets().get(0).getKeyAsString()); + } + ); + + testSearchCase( + Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("const")), + dataset, + () -> new CompositeAggregationBuilder( + "name", + Collections.singletonList( + new TermsValuesSourceBuilder("keyword").field("keyword") + .missingBucket(true) + .missingOrder(MissingOrder.LAST) + .order(SortOrder.ASC) + ) + ).aggregateAfter(createAfterKey("keyword", null)), + (result) -> { assertEquals(0, result.getBuckets().size()); } + ); + + testSearchCase( + Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("const")), + dataset, + () -> new CompositeAggregationBuilder( + "name", + Arrays.asList( + new TermsValuesSourceBuilder("keyword").field("keyword") + .missingBucket(true) + .missingOrder(MissingOrder.LAST) + .order(SortOrder.ASC), + new TermsValuesSourceBuilder("long").field("long") + ) + ).aggregateAfter(createAfterKey("keyword", null, "long", 1)), + (result) -> { + assertEquals(1, result.getBuckets().size()); + assertEquals("{keyword=null, long=2}", result.getBuckets().get(0).getKeyAsString()); + } + ); + } + + public void testMissingHistogramBucketAfterKey() throws Exception { + List>> dataset = Arrays.asList( + createDocument("const", 1, "long", 1), + createDocument("const", 1, "long", 2), + createDocument("const", 1, "keyword", "a"), + createDocument("const", 1, "keyword", "b") + ); + + testSearchCase( + Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("const")), + dataset, + () -> new CompositeAggregationBuilder( + "name", + Collections.singletonList( + new HistogramValuesSourceBuilder("hist").interval(1) + .field("long") + .missingBucket(true) + .missingOrder(MissingOrder.FIRST) + .order(SortOrder.ASC) + ) + ).aggregateAfter(createAfterKey("hist", null)), + (result) -> { + assertEquals(2, result.getBuckets().size()); + assertEquals("{hist=1.0}", result.getBuckets().get(0).getKeyAsString()); + assertEquals("{hist=2.0}", result.getBuckets().get(1).getKeyAsString()); + } + ); + + testSearchCase( + Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("const")), + dataset, + () -> new CompositeAggregationBuilder( + "name", + Arrays.asList( + new HistogramValuesSourceBuilder("hist").interval(1) + .field("long") + .missingBucket(true) + .missingOrder(MissingOrder.FIRST) + .order(SortOrder.ASC), + new TermsValuesSourceBuilder("keyword").field("keyword") + ) + ).aggregateAfter(createAfterKey("hist", null, "keyword", "a")), + (result) -> { + assertEquals(1, result.getBuckets().size()); + assertEquals("{hist=null, keyword=b}", result.getBuckets().get(0).getKeyAsString()); + } + ); + + testSearchCase( + Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("const")), + dataset, + () -> new CompositeAggregationBuilder( + "name", + Collections.singletonList( + new HistogramValuesSourceBuilder("hist").interval(1) + .field("long") + .missingBucket(true) + .missingOrder(MissingOrder.LAST) + .order(SortOrder.ASC) + ) + ).aggregateAfter(createAfterKey("hist", null)), + (result) -> { assertEquals(0, result.getBuckets().size()); } + ); + + testSearchCase( + Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("const")), + dataset, + () -> new CompositeAggregationBuilder( + "name", + Arrays.asList( + new HistogramValuesSourceBuilder("hist").interval(1) + .field("long") + .missingBucket(true) + .missingOrder(MissingOrder.LAST) + .order(SortOrder.ASC), + new TermsValuesSourceBuilder("keyword").field("keyword") + ) + ).aggregateAfter(createAfterKey("hist", null, "keyword", "a")), + (result) -> { + assertEquals(1, result.getBuckets().size()); + assertEquals("{hist=null, keyword=b}", result.getBuckets().get(0).getKeyAsString()); + } + ); } public void testMultiValuedWithKeywordAndLong() throws Exception { diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesCollectorQueueTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesCollectorQueueTests.java index 9a5cdcbfcc70a..519889ef38c15 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesCollectorQueueTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesCollectorQueueTests.java @@ -253,6 +253,7 @@ private void testRandomCase(boolean forceMerge, boolean missingBucket, int index value -> value, DocValueFormat.RAW, missingBucket, + MissingOrder.DEFAULT, size, 1 ); @@ -263,6 +264,7 @@ private void testRandomCase(boolean forceMerge, boolean missingBucket, int index context -> FieldData.sortableLongBitsToDoubles(DocValues.getSortedNumeric(context.reader(), fieldType.name())), DocValueFormat.RAW, missingBucket, + MissingOrder.DEFAULT, size, 1 ); @@ -275,6 +277,7 @@ private void testRandomCase(boolean forceMerge, boolean missingBucket, int index context -> DocValues.getSortedSet(context.reader(), fieldType.name()), DocValueFormat.RAW, missingBucket, + MissingOrder.DEFAULT, size, 1 ); @@ -286,6 +289,7 @@ private void testRandomCase(boolean forceMerge, boolean missingBucket, int index context -> FieldData.toString(DocValues.getSortedSet(context.reader(), fieldType.name())), DocValueFormat.RAW, missingBucket, + MissingOrder.DEFAULT, size, 1 ); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/SingleDimensionValuesSourceTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/SingleDimensionValuesSourceTests.java index 1529236a4c2e1..4eb3020a7a3f9 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/SingleDimensionValuesSourceTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/SingleDimensionValuesSourceTests.java @@ -38,6 +38,7 @@ public void testBinarySorted() { context -> null, DocValueFormat.RAW, false, + MissingOrder.DEFAULT, 1, 1 ); @@ -55,6 +56,7 @@ public void testBinarySorted() { context -> null, DocValueFormat.RAW, true, + MissingOrder.DEFAULT, 1, 1 ); @@ -68,13 +70,24 @@ public void testBinarySorted() { context -> null, DocValueFormat.RAW, false, + MissingOrder.DEFAULT, 0, -1 ); assertNull(source.createSortedDocsProducerOrNull(reader, null)); MappedFieldType ip = new IpFieldMapper.IpFieldType("ip"); - source = new BinaryValuesSource(BigArrays.NON_RECYCLING_INSTANCE, (b) -> {}, ip, context -> null, DocValueFormat.RAW, false, 1, 1); + source = new BinaryValuesSource( + BigArrays.NON_RECYCLING_INSTANCE, + (b) -> {}, + ip, + context -> null, + DocValueFormat.RAW, + false, + MissingOrder.DEFAULT, + 1, + 1 + ); assertNull(source.createSortedDocsProducerOrNull(reader, null)); } @@ -87,6 +100,7 @@ public void testOrdinalsSorted() { context -> null, DocValueFormat.RAW, false, + MissingOrder.DEFAULT, 1, 1 ); @@ -104,6 +118,7 @@ public void testOrdinalsSorted() { context -> null, DocValueFormat.RAW, true, + MissingOrder.DEFAULT, 1, 1 ); @@ -118,6 +133,7 @@ public void testOrdinalsSorted() { context -> null, DocValueFormat.RAW, false, + MissingOrder.DEFAULT, 1, -1 ); @@ -125,7 +141,17 @@ public void testOrdinalsSorted() { assertNull(source.createSortedDocsProducerOrNull(reader, new TermQuery(new Term("foo", "bar")))); final MappedFieldType ip = new IpFieldMapper.IpFieldType("ip"); - source = new OrdinalValuesSource(BigArrays.NON_RECYCLING_INSTANCE, (b) -> {}, ip, context -> null, DocValueFormat.RAW, false, 1, 1); + source = new OrdinalValuesSource( + BigArrays.NON_RECYCLING_INSTANCE, + (b) -> {}, + ip, + context -> null, + DocValueFormat.RAW, + false, + MissingOrder.DEFAULT, + 1, + 1 + ); assertNull(source.createSortedDocsProducerOrNull(reader, null)); assertNull(source.createSortedDocsProducerOrNull(reader, new TermQuery(new Term("foo", "bar")))); } @@ -146,6 +172,7 @@ public void testNumericSorted() { value -> value, DocValueFormat.RAW, false, + MissingOrder.DEFAULT, 1, 1 ); @@ -179,6 +206,7 @@ public void testNumericSorted() { value -> value, DocValueFormat.RAW, true, + MissingOrder.DEFAULT, 1, 1 ); @@ -200,6 +228,7 @@ public void testNumericSorted() { value -> value, DocValueFormat.RAW, false, + MissingOrder.DEFAULT, 1, -1 ); @@ -218,6 +247,7 @@ public void testNumericSorted() { context -> null, DocValueFormat.RAW, false, + MissingOrder.DEFAULT, 1, 1 );