Skip to content

Comprehensively test supported/unsupported field type:agg combinations #52493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 20, 2020
Merged

Comprehensively test supported/unsupported field type:agg combinations #52493

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5ad9a96
Put values source types on fields (#51503)
not-napoleon Jan 28, 2020
e5340d5
Comprehensively test supported/unsupported field type:aggs combinations
polyfractal Feb 18, 2020
197b1b0
Cleanup, javadocs, review comments
polyfractal Feb 19, 2020
f0973f9
Tweak for date_nanos
polyfractal Feb 19, 2020
24a62aa
Update test/framework/src/main/java/org/elasticsearch/search/aggregat…
polyfractal Feb 20, 2020
5b587e4
Skip fields that are not aggregatable
polyfractal Feb 20, 2020
6df733f
Merge remote-tracking branch 'origin/master' into agg_test_fieldmappers
polyfractal Feb 20, 2020
ce07da7
Merge remote-tracking branch 'origin/master' into agg_test_fieldmappers
polyfractal Feb 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions ...es/mapper-extras/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@
import org.elasticsearch.search.MultiValueMode;
import org.elasticsearch.search.sort.BucketedSort;
import org.elasticsearch.search.sort.SortOrder;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.math.BigDecimal;
Expand Down Expand Up @@ -295,6 +297,11 @@ public IndexFieldData<?> build(IndexSettings indexSettings, MappedFieldType fiel
};
}

@Override
public ValuesSourceType getValuesSourceType() {
return CoreValuesSourceType.NUMERIC;
}

@Override
public Object valueForDisplay(Object value) {
if (value == null) {
Expand Down
7 changes: 7 additions & 0 deletions modules/parent-join/src/main/java/org/elasticsearch/join/mapper/MetaJoinFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import org.elasticsearch.index.mapper.ParseContext;
import org.elasticsearch.index.mapper.StringFieldType;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.util.List;
Expand Down Expand Up @@ -95,6 +97,11 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
return new DocValuesIndexFieldData.Builder();
}

@Override
public ValuesSourceType getValuesSourceType() {
return CoreValuesSourceType.BYTES;
}

@Override
public Object valueForDisplay(Object value) {
if (value == null) {
Expand Down
7 changes: 7 additions & 0 deletions modules/parent-join/src/main/java/org/elasticsearch/join/mapper/ParentIdFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
import org.elasticsearch.index.mapper.ParseContext;
import org.elasticsearch.index.mapper.StringFieldType;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.util.Collection;
Expand Down Expand Up @@ -118,6 +120,11 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
return new DocValuesIndexFieldData.Builder();
}

@Override
public ValuesSourceType getValuesSourceType() {
return CoreValuesSourceType.BYTES;
}

@Override
public Object valueForDisplay(Object value) {
if (value == null) {
Expand Down
7 changes: 7 additions & 0 deletions modules/parent-join/src/main/java/org/elasticsearch/join/mapper/ParentJoinFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
import org.elasticsearch.index.mapper.ParseContext;
import org.elasticsearch.index.mapper.StringFieldType;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -227,6 +229,11 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
return new DocValuesIndexFieldData.Builder();
}

@Override
public ValuesSourceType getValuesSourceType() {
return CoreValuesSourceType.BYTES;
}

@Override
public Object valueForDisplay(Object value) {
if (value == null) {
Expand Down
7 changes: 7 additions & 0 deletions ...ysis-icu/src/main/java/org/elasticsearch/index/mapper/ICUCollationKeywordFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
import org.elasticsearch.index.fielddata.plain.DocValuesIndexFieldData;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.time.ZoneId;
Expand Down Expand Up @@ -139,6 +141,11 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
return new DocValuesIndexFieldData.Builder();
}

@Override
public ValuesSourceType getValuesSourceType() {
return CoreValuesSourceType.BYTES;
}

@Override
protected BytesRef indexedValueForSearch(Object value) {
if (value == null) {
Expand Down
7 changes: 7 additions & 0 deletions ...pper-murmur3/src/main/java/org/elasticsearch/index/mapper/murmur3/Murmur3FieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import org.elasticsearch.index.mapper.TypeParsers;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.index.query.QueryShardException;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.util.List;
Expand Down Expand Up @@ -124,6 +126,11 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
return new DocValuesIndexFieldData.Builder().numericType(NumericType.LONG);
}

@Override
public ValuesSourceType getValuesSourceType() {
return CoreValuesSourceType.NUMERIC;
}

@Override
public Query existsQuery(QueryShardContext context) {
return new DocValuesFieldExistsQuery(name());
Expand Down
7 changes: 7 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/BinaryFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.index.query.QueryShardException;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.time.ZoneId;
Expand Down Expand Up @@ -136,6 +138,11 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
return new BytesBinaryDVIndexFieldData.Builder();
}

@Override
public ValuesSourceType getValuesSourceType() {
return CoreValuesSourceType.BYTES;
}

@Override
public Query existsQuery(QueryShardContext context) {
if (hasDocValues()) {
Expand Down
7 changes: 7 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/BooleanFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
import org.elasticsearch.index.fielddata.plain.DocValuesIndexFieldData;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.time.ZoneId;
Expand Down Expand Up @@ -189,6 +191,11 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
return new DocValuesIndexFieldData.Builder().numericType(NumericType.BOOLEAN);
}

@Override
public ValuesSourceType getValuesSourceType() {
return CoreValuesSourceType.NUMERIC;
}

@Override
public DocValueFormat docValueFormat(@Nullable String format, ZoneId timeZone) {
if (format != null) {
Expand Down
10 changes: 9 additions & 1 deletion server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
import org.elasticsearch.index.query.QueryRewriteContext;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.time.DateTimeException;
Expand All @@ -71,6 +73,7 @@
public final class DateFieldMapper extends FieldMapper {

public static final String CONTENT_TYPE = "date";
public static final String DATE_NANOS_CONTENT_TYPE = "date_nanos";
public static final DateFormatter DEFAULT_DATE_TIME_FORMATTER = DateFormatter.forPattern("strict_date_optional_time||epoch_millis");

public static class Defaults {
Expand All @@ -94,7 +97,7 @@ public Instant clampToValidRange(Instant instant) {
return instant;
}
},
NANOSECONDS("date_nanos", NumericType.DATE_NANOSECONDS) {
NANOSECONDS(DATE_NANOS_CONTENT_TYPE, NumericType.DATE_NANOSECONDS) {
@Override
public long convert(Instant instant) {
return toLong(instant);
Expand Down Expand Up @@ -505,6 +508,11 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
return new DocValuesIndexFieldData.Builder().numericType(resolution.numericType());
}

@Override
public ValuesSourceType getValuesSourceType() {
return CoreValuesSourceType.NUMERIC;
}

@Override
public Object valueForDisplay(Object value) {
Long val = (Long) value;
Expand Down
7 changes: 7 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
import org.elasticsearch.index.fielddata.plain.AbstractLatLonPointDVIndexFieldData;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.index.query.QueryShardException;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.util.ArrayList;
Expand Down Expand Up @@ -235,6 +237,11 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
return new AbstractLatLonPointDVIndexFieldData.Builder();
}

@Override
public ValuesSourceType getValuesSourceType() {
return CoreValuesSourceType.GEOPOINT;
}

@Override
public Query existsQuery(QueryShardContext context) {
if (hasDocValues()) {
Expand Down
8 changes: 8 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/IdFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
import org.elasticsearch.search.MultiValueMode;
import org.elasticsearch.search.sort.BucketedSort;
import org.elasticsearch.search.sort.SortOrder;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.util.Arrays;
Expand Down Expand Up @@ -157,6 +159,12 @@ public Query termsQuery(List<?> values, QueryShardContext context) {
return new TermInSetQuery(name(), bytesRefs);
}

@Override
public ValuesSourceType getValuesSourceType() {
// TODO: should this even exist? Is aggregating on the ID field valid?
return CoreValuesSourceType.BYTES;
}

@Override
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
if (indexOptions() == IndexOptions.NONE) {
Expand Down
8 changes: 8 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/IndexFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import org.elasticsearch.index.fielddata.IndexFieldData;
import org.elasticsearch.index.fielddata.plain.ConstantIndexFieldData;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.util.List;
Expand Down Expand Up @@ -189,6 +191,12 @@ public Query wildcardQuery(String value,
public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
return new ConstantIndexFieldData.Builder(mapperService -> fullyQualifiedIndexName);
}

@Override
public ValuesSourceType getValuesSourceType() {
// TODO: Should Index fields be aggregatable? What even is an IndexField?
return CoreValuesSourceType.BYTES;
}
}

private IndexFieldMapper(Settings indexSettings, MappedFieldType existing) {
Expand Down
7 changes: 7 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
import org.elasticsearch.index.fielddata.plain.DocValuesIndexFieldData;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.net.InetAddress;
Expand Down Expand Up @@ -294,6 +296,11 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
return new DocValuesIndexFieldData.Builder().scriptFunction(IpScriptDocValues::new);
}

@Override
public ValuesSourceType getValuesSourceType() {
return CoreValuesSourceType.BYTES;
}

@Override
public Object valueForDisplay(Object value) {
if (value == null) {
Expand Down
7 changes: 7 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
import org.elasticsearch.index.fielddata.IndexFieldData;
import org.elasticsearch.index.fielddata.plain.DocValuesIndexFieldData;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.util.Iterator;
Expand Down Expand Up @@ -271,6 +273,11 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
return new DocValuesIndexFieldData.Builder();
}

@Override
public ValuesSourceType getValuesSourceType() {
return CoreValuesSourceType.BYTES;
}

@Override
public Object valueForDisplay(Object value) {
if (value == null) {
Expand Down
11 changes: 11 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import org.elasticsearch.index.query.QueryShardException;
import org.elasticsearch.index.similarity.SimilarityProvider;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.time.ZoneId;
Expand Down Expand Up @@ -117,6 +118,16 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
throw new IllegalArgumentException("Fielddata is not supported on field [" + name() + "] of type [" + typeName() + "]");
}

/**
* Returns the {@link ValuesSourceType} which supports this field type. This is tightly coupled to field data and aggregations support,
* so any implementation that returns a value from {@link MappedFieldType#fielddataBuilder} should also return a value from here.
*
* @return The appropriate {@link ValuesSourceType} for this field type.
*/
public ValuesSourceType getValuesSourceType() {
throw new IllegalArgumentException("Aggregations are not supported on field [" + name() + "] of type [" + typeName() + "]");
}

@Override
public boolean equals(Object o) {
if (!super.equals(o)) return false;
Expand Down
7 changes: 7 additions & 0 deletions server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@
import org.elasticsearch.index.fielddata.plain.DocValuesIndexFieldData;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.time.ZoneId;
Expand Down Expand Up @@ -958,6 +960,11 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
return new DocValuesIndexFieldData.Builder().numericType(type.numericType());
}

@Override
public ValuesSourceType getValuesSourceType() {
return CoreValuesSourceType.NUMERIC;
}

@Override
public Object valueForDisplay(Object value) {
if (value == null) {
Expand Down
15 changes: 7 additions & 8 deletions server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
import org.elasticsearch.index.fielddata.plain.DocValuesIndexFieldData;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

import java.io.IOException;
import java.net.InetAddress;
Expand Down Expand Up @@ -92,14 +94,6 @@ public RangeFieldType fieldType() {
return (RangeFieldType)fieldType;
}

@Override
public Builder docValues(boolean docValues) {
Copy link
Contributor Author

@polyfractal polyfractal Feb 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: we think this is a bug, I'll open a separate PR fixing this

if (docValues) {
throw new IllegalArgumentException("field [" + name + "] does not currently support " + TypeParsers.DOC_VALUES);
}
return super.docValues(docValues);
}

public Builder coerce(boolean coerce) {
this.coerce = coerce;
return builder;
Expand Down Expand Up @@ -242,6 +236,11 @@ public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {
return new DocValuesIndexFieldData.Builder().setRangeType(rangeType);
}

@Override
public ValuesSourceType getValuesSourceType() {
return CoreValuesSourceType.RANGE;
}

@Override
public String typeName() {
return rangeType.name;
Expand Down
Loading