diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/WeightedAvgAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/WeightedAvgAggregationBuilder.java index ee8b77fa25f70..ddac2e1b6dae9 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/WeightedAvgAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/WeightedAvgAggregationBuilder.java @@ -31,19 +31,20 @@ import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.AggregatorFactories.Builder; import org.elasticsearch.search.aggregations.AggregatorFactory; +import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; import org.elasticsearch.search.aggregations.support.MultiValuesSourceAggregationBuilder; import org.elasticsearch.search.aggregations.support.MultiValuesSourceAggregatorFactory; import org.elasticsearch.search.aggregations.support.MultiValuesSourceFieldConfig; import org.elasticsearch.search.aggregations.support.MultiValuesSourceParseHelper; import org.elasticsearch.search.aggregations.support.ValueType; -import org.elasticsearch.search.aggregations.support.ValuesSource.Numeric; import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; +import org.elasticsearch.search.aggregations.support.ValuesSourceType; import java.io.IOException; import java.util.Map; import java.util.Objects; -public class WeightedAvgAggregationBuilder extends MultiValuesSourceAggregationBuilder.LeafOnly { +public class WeightedAvgAggregationBuilder extends MultiValuesSourceAggregationBuilder.LeafOnly { public static final String NAME = "weighted_avg"; public static final ParseField VALUE_FIELD = new ParseField("value"); public static final ParseField WEIGHT_FIELD = new ParseField("weight"); @@ -61,7 +62,7 @@ public static AggregationBuilder parse(String aggregationName, XContentParser pa } public WeightedAvgAggregationBuilder(String name) { - super(name, ValueType.NUMERIC); + super(name); } public WeightedAvgAggregationBuilder(WeightedAvgAggregationBuilder clone, Builder factoriesBuilder, Map metaData) { @@ -84,7 +85,7 @@ public WeightedAvgAggregationBuilder weight(MultiValuesSourceFieldConfig weightC * Read from a stream. */ public WeightedAvgAggregationBuilder(StreamInput in) throws IOException { - super(in, ValueType.NUMERIC); + super(in); } @Override @@ -92,17 +93,22 @@ protected AggregationBuilder shallowCopy(Builder factoriesBuilder, Map innerBuild(QueryShardContext queryShardContext, - Map configs, - DocValueFormat format, - AggregatorFactory parent, - Builder subFactoriesBuilder) throws IOException { + protected MultiValuesSourceAggregatorFactory innerBuild(QueryShardContext queryShardContext, + Map configs, + DocValueFormat format, + AggregatorFactory parent, + Builder subFactoriesBuilder) throws IOException { return new WeightedAvgAggregatorFactory(name, configs, format, queryShardContext, parent, subFactoriesBuilder, metaData); } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/WeightedAvgAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/WeightedAvgAggregatorFactory.java index fecfabba98252..a9fedbf0e7d3e 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/WeightedAvgAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/WeightedAvgAggregatorFactory.java @@ -27,7 +27,6 @@ import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; import org.elasticsearch.search.aggregations.support.MultiValuesSource; import org.elasticsearch.search.aggregations.support.MultiValuesSourceAggregatorFactory; -import org.elasticsearch.search.aggregations.support.ValuesSource.Numeric; import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; import org.elasticsearch.search.internal.SearchContext; @@ -35,7 +34,7 @@ import java.util.List; import java.util.Map; -class WeightedAvgAggregatorFactory extends MultiValuesSourceAggregatorFactory { +class WeightedAvgAggregatorFactory extends MultiValuesSourceAggregatorFactory { WeightedAvgAggregatorFactory(String name, Map configs, DocValueFormat format, QueryShardContext queryShardContext, AggregatorFactory parent, diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceAggregationBuilder.java index 937b3e77d743a..d622ff79f804c 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceAggregationBuilder.java @@ -40,18 +40,18 @@ * * A limitation of this class is that all the ValuesSource's being refereenced must be of the same type. */ -public abstract class MultiValuesSourceAggregationBuilder> +public abstract class MultiValuesSourceAggregationBuilder> extends AbstractAggregationBuilder { - public abstract static class LeafOnly> - extends MultiValuesSourceAggregationBuilder { + public abstract static class LeafOnly> + extends MultiValuesSourceAggregationBuilder { - protected LeafOnly(String name, ValueType targetValueType) { - super(name, targetValueType); + protected LeafOnly(String name) { + super(name); } - protected LeafOnly(LeafOnly clone, Builder factoriesBuilder, Map metaData) { + protected LeafOnly(LeafOnly clone, Builder factoriesBuilder, Map metaData) { super(clone, factoriesBuilder, metaData); if (factoriesBuilder.count() > 0) { throw new AggregationInitializationException("Aggregator [" + name + "] of type [" @@ -62,8 +62,8 @@ protected LeafOnly(LeafOnly clone, Builder factoriesBuilder, Map fields = new HashMap<>(); - private final ValueType targetValueType; - private ValueType valueType = null; + private ValueType userValueTypeHint = null; private String format = null; - protected MultiValuesSourceAggregationBuilder(String name, ValueType targetValueType) { + protected MultiValuesSourceAggregationBuilder(String name) { super(name); - this.targetValueType = targetValueType; } - protected MultiValuesSourceAggregationBuilder(MultiValuesSourceAggregationBuilder clone, + protected MultiValuesSourceAggregationBuilder(MultiValuesSourceAggregationBuilder clone, Builder factoriesBuilder, Map metaData) { super(clone, factoriesBuilder, metaData); this.fields = new HashMap<>(clone.fields); - this.targetValueType = clone.targetValueType; - this.valueType = clone.valueType; + this.userValueTypeHint = clone.userValueTypeHint; this.format = clone.format; } - protected MultiValuesSourceAggregationBuilder(StreamInput in, ValueType targetValueType) + /** + * Read from a stream. + */ + protected MultiValuesSourceAggregationBuilder(StreamInput in) throws IOException { super(in); - assert false == serializeTargetValueType() : "Wrong read constructor called for subclass that provides its targetValueType"; - this.targetValueType = targetValueType; read(in); } @@ -109,17 +107,14 @@ protected MultiValuesSourceAggregationBuilder(StreamInput in, ValueType targetVa @SuppressWarnings("unchecked") private void read(StreamInput in) throws IOException { fields = in.readMap(StreamInput::readString, MultiValuesSourceFieldConfig::new); - valueType = in.readOptionalWriteable(ValueType::readFromStream); + userValueTypeHint = in.readOptionalWriteable(ValueType::readFromStream); format = in.readOptionalString(); } @Override protected final void doWriteTo(StreamOutput out) throws IOException { - if (serializeTargetValueType()) { - out.writeOptionalWriteable(targetValueType); - } out.writeMap(fields, StreamOutput::writeString, (o, value) -> value.writeTo(o)); - out.writeOptionalWriteable(valueType); + out.writeOptionalWriteable(userValueTypeHint); out.writeOptionalString(format); innerWriteTo(out); } @@ -142,11 +137,11 @@ protected AB field(String propertyName, MultiValuesSourceFieldConfig config) { * Sets the {@link ValueType} for the value produced by this aggregation */ @SuppressWarnings("unchecked") - public AB valueType(ValueType valueType) { + public AB userValueTypeHint(ValueType valueType) { if (valueType == null) { - throw new IllegalArgumentException("[valueType] must not be null: [" + name + "]"); + throw new IllegalArgumentException("[userValueTypeHint] must not be null: [" + name + "]"); } - this.valueType = valueType; + this.userValueTypeHint = valueType; return (AB) this; } @@ -162,25 +157,34 @@ public AB format(String format) { return (AB) this; } - @Override - protected final MultiValuesSourceAggregatorFactory doBuild(QueryShardContext queryShardContext, AggregatorFactory parent, - Builder subFactoriesBuilder) throws IOException { - ValueType finalValueType = this.valueType != null ? this.valueType : targetValueType; + /** + * Aggregations should use this method to define a {@link ValuesSourceType} of last resort. This will only be used when the resolver + * can't find a field and the user hasn't provided a value type hint. + * + * @return The CoreValuesSourceType we expect this script to yield. + */ + protected abstract ValuesSourceType defaultValueSourceType(); + @Override + protected final MultiValuesSourceAggregatorFactory doBuild(QueryShardContext queryShardContext, AggregatorFactory parent, + Builder subFactoriesBuilder) throws IOException { Map configs = new HashMap<>(fields.size()); fields.forEach((key, value) -> { - ValuesSourceConfig config = ValuesSourceConfig.resolve(queryShardContext, finalValueType, - value.getFieldName(), value.getScript(), value.getMissing(), value.getTimeZone(), format, getType()); + ValuesSourceConfig config = ValuesSourceConfig.resolve(queryShardContext, userValueTypeHint, + value.getFieldName(), value.getScript(), value.getMissing(), value.getTimeZone(), format, defaultValueSourceType(), + getType()); configs.put(key, config); }); - DocValueFormat docValueFormat = resolveFormat(format, finalValueType); + DocValueFormat docValueFormat = resolveFormat(format, userValueTypeHint, defaultValueSourceType()); return innerBuild(queryShardContext, configs, docValueFormat, parent, subFactoriesBuilder); } - private static DocValueFormat resolveFormat(@Nullable String format, @Nullable ValueType valueType) { + private static DocValueFormat resolveFormat(@Nullable String format, @Nullable ValueType valueType, + ValuesSourceType defaultValuesSourceType) { if (valueType == null) { - return DocValueFormat.RAW; // we can't figure it out + // If the user didn't send a hint, all we can do is fall back to the default + return defaultValuesSourceType.getFormatter(format, null); } DocValueFormat valueFormat = valueType.defaultFormat; if (valueFormat instanceof DocValueFormat.Decimal && format != null) { @@ -189,19 +193,11 @@ private static DocValueFormat resolveFormat(@Nullable String format, @Nullable V return valueFormat; } - protected abstract MultiValuesSourceAggregatorFactory innerBuild(QueryShardContext queryShardContext, - Map configs, - DocValueFormat format, AggregatorFactory parent, - Builder subFactoriesBuilder) throws IOException; - + protected abstract MultiValuesSourceAggregatorFactory innerBuild(QueryShardContext queryShardContext, + Map configs, + DocValueFormat format, AggregatorFactory parent, + Builder subFactoriesBuilder) throws IOException; - /** - * Should this builder serialize its targetValueType? Defaults to false. All subclasses that override this to true - * should use the three argument read constructor rather than the four argument version. - */ - protected boolean serializeTargetValueType() { - return false; - } @Override public final XContentBuilder internalXContent(XContentBuilder builder, Params params) throws IOException { @@ -214,8 +210,8 @@ public final XContentBuilder internalXContent(XContentBuilder builder, Params pa if (format != null) { builder.field(CommonFields.FORMAT.getPreferredName(), format); } - if (valueType != null) { - builder.field(CommonFields.VALUE_TYPE.getPreferredName(), valueType.getPreferredName()); + if (userValueTypeHint != null) { + builder.field(CommonFields.VALUE_TYPE.getPreferredName(), userValueTypeHint.getPreferredName()); } doXContentBody(builder, params); builder.endObject(); @@ -226,7 +222,7 @@ public final XContentBuilder internalXContent(XContentBuilder builder, Params pa @Override public int hashCode() { - return Objects.hash(super.hashCode(), fields, format, targetValueType, valueType); + return Objects.hash(super.hashCode(), fields, format, userValueTypeHint); } @@ -239,6 +235,6 @@ public boolean equals(Object obj) { MultiValuesSourceAggregationBuilder other = (MultiValuesSourceAggregationBuilder) obj; return Objects.equals(this.fields, other.fields) && Objects.equals(this.format, other.format) - && Objects.equals(this.valueType, other.valueType); + && Objects.equals(this.userValueTypeHint, other.userValueTypeHint); } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceAggregatorFactory.java index e78029967862f..711c53f13d9df 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceAggregatorFactory.java @@ -31,8 +31,7 @@ import java.util.List; import java.util.Map; -public abstract class MultiValuesSourceAggregatorFactory - extends AggregatorFactory { +public abstract class MultiValuesSourceAggregatorFactory extends AggregatorFactory { protected final Map configs; protected final DocValueFormat format; diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceParseHelper.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceParseHelper.java index dc87efa7edbc8..3bbb8bfb702fc 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceParseHelper.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceParseHelper.java @@ -28,10 +28,10 @@ public final class MultiValuesSourceParseHelper { public static void declareCommon( - AbstractObjectParser, T> objectParser, boolean formattable, - ValueType expectedValueType) { + AbstractObjectParser, T> objectParser, boolean formattable, + ValueType expectedValueType) { - objectParser.declareField(MultiValuesSourceAggregationBuilder::valueType, p -> { + objectParser.declareField(MultiValuesSourceAggregationBuilder::userValueTypeHint, p -> { ValueType valueType = ValueType.resolveForScript(p.text()); if (expectedValueType != null && valueType.isNotA(expectedValueType)) { throw new ParsingException(p.getTokenLocation(), @@ -49,7 +49,7 @@ public static void declareCommon( } public static void declareField(String fieldName, - AbstractObjectParser, T> objectParser, + AbstractObjectParser, T> objectParser, boolean scriptable, boolean timezoneAware) { objectParser.declareField((o, fieldConfig) -> o.field(fieldName, fieldConfig.build()), diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValueType.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValueType.java index bfe642caaad20..c2fbab66028ce 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValueType.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValueType.java @@ -23,10 +23,6 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.index.fielddata.IndexFieldData; -import org.elasticsearch.index.fielddata.IndexGeoPointFieldData; -import org.elasticsearch.index.fielddata.IndexNumericFieldData; -import org.elasticsearch.index.fielddata.plain.BinaryDVIndexFieldData; import org.elasticsearch.index.mapper.DateFieldMapper; import org.elasticsearch.search.DocValueFormat; @@ -36,24 +32,23 @@ public enum ValueType implements Writeable { STRING((byte) 1, "string", "string", CoreValuesSourceType.BYTES, - IndexFieldData.class, DocValueFormat.RAW), + DocValueFormat.RAW), - LONG((byte) 2, "byte|short|integer|long", "long", CoreValuesSourceType.NUMERIC, IndexNumericFieldData.class, DocValueFormat.RAW), - DOUBLE((byte) 3, "float|double", "double", CoreValuesSourceType.NUMERIC, IndexNumericFieldData.class, DocValueFormat.RAW), - NUMBER((byte) 4, "number", "number", CoreValuesSourceType.NUMERIC, IndexNumericFieldData.class, DocValueFormat.RAW), - DATE((byte) 5, "date", "date", CoreValuesSourceType.DATE, IndexNumericFieldData.class, - new DocValueFormat.DateTime(DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER, ZoneOffset.UTC, + LONG((byte) 2, "byte|short|integer|long", "long", CoreValuesSourceType.NUMERIC, DocValueFormat.RAW), + DOUBLE((byte) 3, "float|double", "double", CoreValuesSourceType.NUMERIC, DocValueFormat.RAW), + NUMBER((byte) 4, "number", "number", CoreValuesSourceType.NUMERIC, DocValueFormat.RAW), + DATE((byte) 5, "date", "date", CoreValuesSourceType.DATE, + new DocValueFormat.DateTime(DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER, ZoneOffset.UTC, DateFieldMapper.Resolution.MILLISECONDS)), - IP((byte) 6, "ip", "ip", CoreValuesSourceType.IP, IndexFieldData.class, DocValueFormat.IP), + IP((byte) 6, "ip", "ip", CoreValuesSourceType.IP, DocValueFormat.IP), // TODO: what is the difference between "number" and "numeric"? - NUMERIC((byte) 7, "numeric", "numeric", CoreValuesSourceType.NUMERIC, IndexNumericFieldData.class, DocValueFormat.RAW), - GEOPOINT((byte) 8, "geo_point", "geo_point", CoreValuesSourceType.GEOPOINT, IndexGeoPointFieldData.class, DocValueFormat.GEOHASH), - BOOLEAN((byte) 9, "boolean", "boolean", CoreValuesSourceType.BOOLEAN, IndexNumericFieldData.class, DocValueFormat.BOOLEAN), - RANGE((byte) 10, "range", "range", CoreValuesSourceType.RANGE, BinaryDVIndexFieldData.class, DocValueFormat.RAW); + NUMERIC((byte) 7, "numeric", "numeric", CoreValuesSourceType.NUMERIC, DocValueFormat.RAW), + GEOPOINT((byte) 8, "geo_point", "geo_point", CoreValuesSourceType.GEOPOINT, DocValueFormat.GEOHASH), + BOOLEAN((byte) 9, "boolean", "boolean", CoreValuesSourceType.BOOLEAN, DocValueFormat.BOOLEAN), + RANGE((byte) 10, "range", "range", CoreValuesSourceType.RANGE, DocValueFormat.RAW); final String description; final ValuesSourceType valuesSourceType; - final Class fieldDataType; final DocValueFormat defaultFormat; private final byte id; private String preferredName; @@ -61,12 +56,11 @@ public enum ValueType implements Writeable { public static final ParseField VALUE_TYPE = new ParseField("value_type", "valueType"); ValueType(byte id, String description, String preferredName, ValuesSourceType valuesSourceType, - Class fieldDataType, DocValueFormat defaultFormat) { + DocValueFormat defaultFormat) { this.id = id; this.description = description; this.preferredName = preferredName; this.valuesSourceType = valuesSourceType; - this.fieldDataType = fieldDataType; this.defaultFormat = defaultFormat; } @@ -79,8 +73,7 @@ public ValuesSourceType getValuesSourceType() { } public boolean isA(ValueType valueType) { - return valueType.valuesSourceType.isCastableTo(valuesSourceType) && - valueType.fieldDataType.isAssignableFrom(fieldDataType); + return valueType.valuesSourceType.isCastableTo(valuesSourceType); } public boolean isNotA(ValueType valueType) { @@ -121,7 +114,7 @@ public static ValueType readFromStream(StreamInput in) throws IOException { return valueType; } } - throw new IOException("No valueType found for id [" + id + "]"); + throw new IOException("No ValueType found for id [" + id + "]"); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceAggregationBuilder.java index e0a7917165a93..6e6213938c754 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceAggregationBuilder.java @@ -208,7 +208,7 @@ public AB userValueTypeHint(ValueType valueType) { // TODO: This is nonsense. We allow the value to be null (via constructor), but don't allow it to be set to null. This means // thing looking to copy settings (like RollupRequestTranslator) need to check if userValueTypeHint is not null, and then // set it if and only if it is non-null. - throw new IllegalArgumentException("[valueType] must not be null: [" + name + "]"); + throw new IllegalArgumentException("[userValueTypeHint] must not be null: [" + name + "]"); } this.userValueTypeHint = valueType; return (AB) this; @@ -286,8 +286,8 @@ protected final ValuesSourceAggregatorFactory doBuild(QueryShardContext querySha } /** - * Provide a hook for aggregations to have finer grained control of the {@link ValuesSourceType } when {@link ValuesSourceConfig} - * can't resolve a type (i.e. this will be called before falling back to CoreValuesSourceType.ANY.) + * Aggregations should use this method to define a {@link ValuesSourceType} of last resort. This will only be used when the resolver + * can't find a field and the user hasn't provided a value type hint. * * @return The CoreValuesSourceType we expect this script to yield. */ diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceConfig.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceConfig.java index e1a4fec8adb96..100e1d912f9d8 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceConfig.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceConfig.java @@ -195,7 +195,7 @@ public ValuesSourceConfig script(AggregationScript.LeafFactory script) { return this; } - public ValuesSourceConfig scriptValueType(ValueType scriptValueType) { + private ValuesSourceConfig scriptValueType(ValueType scriptValueType) { this.scriptValueType = scriptValueType; return this; }