Skip to content

Commit

Permalink
[7.x] Support "_first" and "_last" ordering of missing values in comp…
Browse files Browse the repository at this point in the history
…osite 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
  • Loading branch information
Lukas Wegmann authored Sep 13, 2021
1 parent 934691e commit 5087b9f
Show file tree
Hide file tree
Showing 20 changed files with 726 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,11 @@ class BinaryValuesSource extends SingleDimensionValuesSource<BytesRef> {
CheckedFunction<LeafReaderContext, SortedBinaryDocValues, IOException> 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));
Expand Down Expand Up @@ -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));
Expand All @@ -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));
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public abstract class CompositeValuesSourceBuilder<AB extends CompositeValuesSou
private Script script = null;
private ValueType userValueTypeHint = null;
private boolean missingBucket = false;
private MissingOrder missingOrder = MissingOrder.DEFAULT;
private SortOrder order = SortOrder.ASC;
private String format = null;

Expand All @@ -62,6 +63,9 @@ public abstract class CompositeValuesSourceBuilder<AB extends CompositeValuesSou
// skip missing value for BWC
in.readGenericValue();
}
if (in.getVersion().onOrAfter(Version.V_7_16_0)) {
this.missingOrder = MissingOrder.readFromStream(in);
}
this.order = SortOrder.readFromStream(in);
if (in.getVersion().onOrAfter(Version.V_6_3_0)) {
this.format = in.readOptionalString();
Expand Down Expand Up @@ -91,6 +95,9 @@ public final void writeTo(StreamOutput out) throws IOException {
// write missing value for BWC
out.writeGenericValue(null);
}
if (out.getVersion().onOrAfter(Version.V_7_16_0)) {
missingOrder.writeTo(out);
}
order.writeTo(out);
if (out.getVersion().onOrAfter(Version.V_6_3_0)) {
out.writeOptionalString(format);
Expand All @@ -112,6 +119,9 @@ public final XContentBuilder toXContent(XContentBuilder builder, Params params)
builder.field("script", script);
}
builder.field("missing_bucket", missingBucket);
if (missingOrder != MissingOrder.DEFAULT) {
builder.field("missing_order", missingOrder.toString());
}
if (userValueTypeHint != null) {
builder.field("value_type", userValueTypeHint.getPreferredName());
}
Expand All @@ -126,7 +136,7 @@ public final XContentBuilder toXContent(XContentBuilder builder, Params params)

@Override
public int hashCode() {
return Objects.hash(field, missingBucket, script, userValueTypeHint, order, format);
return Objects.hash(field, missingBucket, missingOrder, script, userValueTypeHint, order, format);
}

@Override
Expand All @@ -140,6 +150,7 @@ public boolean equals(Object o) {
&& Objects.equals(script, that.script())
&& Objects.equals(userValueTypeHint, that.userValuetypeHint())
&& Objects.equals(missingBucket, that.missingBucket())
&& Objects.equals(missingOrder, that.missingOrder())
&& Objects.equals(order, that.order())
&& Objects.equals(format, that.format());
}
Expand Down Expand Up @@ -224,6 +235,34 @@ public boolean missingBucket() {
return missingBucket;
}

/**
* Sets the {@link MissingOrder} policy to use for ordering missing values.
*
* @param missingOrder One of "first", "last" or "default".
*/
public AB missingOrder(String missingOrder) {
return missingOrder(MissingOrder.fromString(missingOrder));
}

/**
* Sets the {@link MissingOrder} policy to use for ordering missing values.
*/
@SuppressWarnings("unchecked")
public AB missingOrder(MissingOrder missingOrder) {
if (missingOrder == null) {
throw new IllegalArgumentException("[missingOrder] must not be null");
}
this.missingOrder = missingOrder;
return (AB) this;
}

/**
* The {@link MissingOrder} policy used for ordering missing values.
*/
public MissingOrder missingOrder() {
return missingOrder;
}

/**
* Sets the {@link SortOrder} to use to sort values produced this source
*/
Expand Down Expand Up @@ -282,6 +321,10 @@ public String format() {
protected abstract ValuesSourceType getDefaultValuesSourceType();

public final CompositeValuesSourceConfig build(AggregationContext context) throws IOException {
if (missingBucket == false && missingOrder != MissingOrder.DEFAULT) {
throw new IllegalArgumentException("missingOrder can only be set if missingBucket is true");
}

ValuesSourceConfig config = ValuesSourceConfig.resolve(
context,
userValueTypeHint,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ SingleDimensionValuesSource<?> 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;

Expand All @@ -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 <code>true</code> an explicit <code>null</code> bucket will represent documents with missing values.
* @param missingOrder How to order missing buckets if missingBucket is <code>true</code>.
* @param hasScript <code>true</code> if the source contains a script that can change the value.
*/
CompositeValuesSourceConfig(
Expand All @@ -59,6 +61,7 @@ SingleDimensionValuesSource<?> createValuesSource(
DocValueFormat format,
SortOrder order,
boolean missingBucket,
MissingOrder missingOrder,
boolean hasScript,
SingleDimensionValuesSourceProvider singleDimensionValuesSourceProvider
) {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public class CompositeValuesSourceParserHelper {
static <VB extends CompositeValuesSourceBuilder<VB>, T> void declareValuesSourceFields(AbstractObjectParser<VB, T> 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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Loading

0 comments on commit 5087b9f

Please sign in to comment.