From 91cd7f8890a26ddec872dc6d7a631b3568f29e90 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Thu, 18 Nov 2021 08:35:32 -0500 Subject: [PATCH 01/19] VSAB now has string for hint --- .../ValuesSourceAggregationBuilder.java | 58 ++++++++++++++----- .../support/ValuesSourceRegistry.java | 5 ++ 2 files changed, 49 insertions(+), 14 deletions(-) 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 8cbedec6bad8c..2c7eee7f205cf 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 @@ -62,13 +62,12 @@ public static void declareFields( ObjectParser.ValueType.VALUE ); - objectParser.declareField(ValuesSourceAggregationBuilder::userValueTypeHint, p -> { - ValueType type = ValueType.lenientParse(p.text()); - if (type == null) { - throw new IllegalArgumentException("Unknown value type [" + p.text() + "]"); - } - return type; - }, ValueType.VALUE_TYPE, ObjectParser.ValueType.STRING); + objectParser.declareField( + ValuesSourceAggregationBuilder::userValueTypeHint, + XContentParser::text, + ValueType.VALUE_TYPE, + ObjectParser.ValueType.STRING + ); if (formattable) { objectParser.declareField( @@ -189,7 +188,7 @@ public Set metricNames() { private String field = null; private Script script = null; - private ValueType userValueTypeHint = null; + private String userValueTypeHint = null; private String format = null; private Object missing = null; private ZoneId timeZone = null; @@ -235,7 +234,12 @@ private void read(StreamInput in) throws IOException { script = new Script(in); } if (in.readBoolean()) { - userValueTypeHint = ValueType.readFromStream(in); + if (in.getVersion().before(Version.V_8_1_0)) { + // legacy version wrote a ValueType. Convert it to a string + userValueTypeHint = ValueType.readFromStream(in).getPreferredName(); + } else { + userValueTypeHint = in.readString(); + } } format = in.readOptionalString(); missing = in.readGenericValue(); @@ -257,7 +261,17 @@ protected final void doWriteTo(StreamOutput out) throws IOException { boolean hasValueType = userValueTypeHint != null; out.writeBoolean(hasValueType); if (hasValueType) { - userValueTypeHint.writeTo(out); + if (out.getVersion().before(Version.V_8_1_0)) { + // Legacy version writes a ValueType + ValueType toWrite = ValueType.lenientParse(userValueTypeHint); + if (toWrite != null) { + toWrite.writeTo(out); + } else { + throw new IOException("Value_type [" + userValueTypeHint + "] not compatible with pre 8.1 nodes"); + } + } else { + out.writeString(userValueTypeHint); + } } out.writeOptionalString(format); out.writeGenericValue(missing); @@ -327,7 +341,7 @@ public Script script() { * @return - The modified builder instance, for chaining. */ @SuppressWarnings("unchecked") - public AB userValueTypeHint(ValueType valueType) { + public AB userValueTypeHint(String valueType) { if (valueType == null) { // 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 @@ -338,7 +352,23 @@ public AB userValueTypeHint(ValueType valueType) { return (AB) this; } - public ValueType userValueTypeHint() { + /** + * We set the value type in tests a lot. This is a convenience method for not having to touch all of those places. + */ + @SuppressWarnings("unchecked") + @Deprecated + public AB userValueTypeHint(ValueType valueType) { + if (valueType == null) { + // 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("[userValueTypeHint] must not be null: [" + name + "]"); + } + this.userValueTypeHint = valueType.getPreferredName(); + return (AB) this; + } + + public String userValueTypeHint() { return userValueTypeHint; } @@ -444,7 +474,7 @@ protected final ValuesSourceAggregatorFactory doBuild(AggregationContext context protected ValuesSourceConfig resolveConfig(AggregationContext context) { return ValuesSourceConfig.resolve( context, - this.userValueTypeHint, + context.getValuesSourceRegistry().resolveTypeHint(this.userValueTypeHint), field, script, missing, @@ -480,7 +510,7 @@ public final XContentBuilder internalXContent(XContentBuilder builder, Params pa builder.field("time_zone", timeZone.toString()); } if (userValueTypeHint != null) { - builder.field("value_type", userValueTypeHint.getPreferredName()); + builder.field("value_type", userValueTypeHint); } doXContentBody(builder, params); builder.endObject(); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java index 3a43f147fefa3..c14d8ff213dd6 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java @@ -181,4 +181,9 @@ public T getAggregator(RegistryKey registryKey, ValuesSourceConfig values public AggregationUsageService getUsageService() { return usageService; } + + public ValueType resolveTypeHint(String typeHint) { + // NOCOMMIT: do the actual lookup thing here + return typeHint != null ? ValueType.lenientParse(typeHint) : null; + } } From d4a818aceefa007d47db475e9cd360dea27b5ac8 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Thu, 18 Nov 2021 09:28:25 -0500 Subject: [PATCH 02/19] Use strings for hints everywhere --- .../CompositeValuesSourceBuilder.java | 38 +++++++++++++++---- .../CompositeValuesSourceParserHelper.java | 6 +-- .../WeightedAvgAggregationBuilder.java | 3 +- .../MultiValuesSourceAggregationBuilder.java | 25 ++++++++---- .../support/MultiValuesSourceFieldConfig.java | 35 ++++++++++++----- .../support/MultiValuesSourceParseHelper.java | 26 ++++--------- .../MultiValuesSourceFieldConfigTests.java | 2 +- .../MultiTermsAggregationBuilder.java | 2 +- .../ttest/TTestAggregationBuilder.java | 3 +- .../multiterms/MultiTermsAggregatorTests.java | 2 +- .../GeoLineAggregationBuilder.java | 3 +- 11 files changed, 88 insertions(+), 57 deletions(-) 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 980fa1ca45517..c20711e1097cc 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 @@ -35,7 +35,7 @@ public abstract class CompositeValuesSourceBuilder, T> void declareValuesSource 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()); - return valueType; - }, new ParseField("value_type"), ObjectParser.ValueType.STRING); + objectParser.declareField(VB::userValuetypeHint,XContentParser::text, new ParseField("value_type"), ObjectParser.ValueType.STRING); objectParser.declareField( VB::script, 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 fe6997ad35fde..0c31c4ac98868 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 @@ -21,7 +21,6 @@ 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.ValuesSourceConfig; import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; import org.elasticsearch.search.aggregations.support.ValuesSourceType; @@ -44,7 +43,7 @@ public class WeightedAvgAggregationBuilder extends MultiValuesSourceAggregationB WeightedAvgAggregationBuilder::new ); static { - MultiValuesSourceParseHelper.declareCommon(PARSER, true, ValueType.NUMERIC); + MultiValuesSourceParseHelper.declareCommon(PARSER, true); MultiValuesSourceParseHelper.declareField(VALUE_FIELD.getPreferredName(), PARSER, true, false, false, false); MultiValuesSourceParseHelper.declareField(WEIGHT_FIELD.getPreferredName(), PARSER, true, false, false, false); } 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 034b23f7585fb..44e973100294d 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 @@ -7,6 +7,7 @@ */ package org.elasticsearch.search.aggregations.support; +import org.elasticsearch.Version; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.core.Nullable; @@ -64,7 +65,7 @@ public AB subAggregations(Builder subFactories) { } private Map fields = new HashMap<>(); - private ValueType userValueTypeHint = null; + private String userValueTypeHint = null; private String format = null; protected MultiValuesSourceAggregationBuilder(String name) { @@ -97,14 +98,23 @@ protected MultiValuesSourceAggregationBuilder(StreamInput in) throws IOException @SuppressWarnings("unchecked") private void read(StreamInput in) throws IOException { fields = in.readMap(StreamInput::readString, MultiValuesSourceFieldConfig::new); - userValueTypeHint = in.readOptionalWriteable(ValueType::readFromStream); + if (in.getVersion().before(Version.V_8_1_0)) { + ValueType valueType = in.readOptionalWriteable(ValueType::readFromStream); + this.userValueTypeHint = valueType == null ? null : valueType.getPreferredName(); + } else { + this.userValueTypeHint = in.readOptionalString(); + } format = in.readOptionalString(); } @Override protected final void doWriteTo(StreamOutput out) throws IOException { out.writeMap(fields, StreamOutput::writeString, (o, value) -> value.writeTo(o)); - out.writeOptionalWriteable(userValueTypeHint); + if (out.getVersion().before(Version.V_8_1_0)) { + out.writeOptionalWriteable(ValueType.lenientParse(userValueTypeHint)); + } else { + out.writeOptionalString(userValueTypeHint); + } out.writeOptionalString(format); innerWriteTo(out); } @@ -127,7 +137,7 @@ protected AB field(String propertyName, MultiValuesSourceFieldConfig config) { * Sets the {@link ValueType} for the value produced by this aggregation */ @SuppressWarnings("unchecked") - public AB userValueTypeHint(ValueType valueType) { + public AB userValueTypeHint(String valueType) { if (valueType == null) { throw new IllegalArgumentException("[userValueTypeHint] must not be null: [" + name + "]"); } @@ -163,10 +173,11 @@ protected final MultiValuesSourceAggregatorFactory doBuild( ) throws IOException { Map configs = new HashMap<>(fields.size()); Map filters = new HashMap<>(fields.size()); + final ValueType resolvedTypeHint = context.getValuesSourceRegistry().resolveTypeHint(userValueTypeHint); fields.forEach((key, value) -> { ValuesSourceConfig config = ValuesSourceConfig.resolveUnregistered( context, - userValueTypeHint, + resolvedTypeHint, value.getFieldName(), value.getScript(), value.getMissing(), @@ -177,7 +188,7 @@ protected final MultiValuesSourceAggregatorFactory doBuild( configs.put(key, config); filters.put(key, value.getFilter()); }); - DocValueFormat docValueFormat = resolveFormat(format, userValueTypeHint, defaultValueSourceType()); + DocValueFormat docValueFormat = resolveFormat(format, resolvedTypeHint, defaultValueSourceType()); return innerBuild(context, configs, filters, docValueFormat, parent, subFactoriesBuilder); } @@ -219,7 +230,7 @@ public final XContentBuilder internalXContent(XContentBuilder builder, Params pa builder.field(CommonFields.FORMAT.getPreferredName(), format); } if (userValueTypeHint != null) { - builder.field(CommonFields.VALUE_TYPE.getPreferredName(), userValueTypeHint.getPreferredName()); + builder.field(CommonFields.VALUE_TYPE.getPreferredName(), userValueTypeHint); } doXContentBody(builder, params); builder.endObject(); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java index 2033cdacb6f46..e433d712a0718 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java @@ -37,7 +37,7 @@ public class MultiValuesSourceFieldConfig implements Writeable, ToXContentObject // supported only if filtered == true private final QueryBuilder filter; // supported only if heterogeneous == true - private final ValueType userValueTypeHint; + private final String userValueTypeHint; private final String format; private static final String NAME = "field_config"; @@ -104,7 +104,7 @@ public static ObjectParser parserBu if (heterogeneous) { parser.declareField( MultiValuesSourceFieldConfig.Builder::setUserValueTypeHint, - p -> ValueType.lenientParse(p.text()), + XContentParser::text, ValueType.VALUE_TYPE, ObjectParser.ValueType.STRING ); @@ -125,7 +125,7 @@ protected MultiValuesSourceFieldConfig( Script script, ZoneId timeZone, QueryBuilder filter, - ValueType userValueTypeHint, + String userValueTypeHint, String format ) { this.fieldName = fieldName; @@ -152,7 +152,12 @@ public MultiValuesSourceFieldConfig(StreamInput in) throws IOException { this.filter = null; } if (in.getVersion().onOrAfter(Version.V_7_12_0)) { - this.userValueTypeHint = in.readOptionalWriteable(ValueType::readFromStream); + if (in.getVersion().before(Version.V_8_1_0)) { + ValueType valueType = in.readOptionalWriteable(ValueType::readFromStream); + this.userValueTypeHint = valueType == null ? null : valueType.getPreferredName(); + } else { + this.userValueTypeHint = in.readOptionalString(); + } this.format = in.readOptionalString(); } else { this.userValueTypeHint = null; @@ -180,7 +185,7 @@ public QueryBuilder getFilter() { return filter; } - public ValueType getUserValueTypeHint() { + public String getUserValueTypeHint() { return userValueTypeHint; } @@ -202,7 +207,11 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalNamedWriteable(filter); } if (out.getVersion().onOrAfter(Version.V_7_12_0)) { - out.writeOptionalWriteable(userValueTypeHint); + if (out.getVersion().before(Version.V_8_1_0)) { + out.writeOptionalWriteable(ValueType.lenientParse(userValueTypeHint)); + } else { + out.writeOptionalString(userValueTypeHint); + } out.writeOptionalString(format); } } @@ -227,7 +236,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws filter.toXContent(builder, params); } if (userValueTypeHint != null) { - builder.field(AggregationBuilder.CommonFields.VALUE_TYPE.getPreferredName(), userValueTypeHint.getPreferredName()); + builder.field(AggregationBuilder.CommonFields.VALUE_TYPE.getPreferredName(), userValueTypeHint); } if (format != null) { builder.field(AggregationBuilder.CommonFields.FORMAT.getPreferredName(), format); @@ -266,7 +275,7 @@ public static class Builder { private Script script = null; private ZoneId timeZone = null; private QueryBuilder filter = null; - private ValueType userValueTypeHint = null; + private String userValueTypeHint = null; private String format = null; public String getFieldName() { @@ -310,12 +319,18 @@ public Builder setFilter(QueryBuilder filter) { return this; } - public Builder setUserValueTypeHint(ValueType userValueTypeHint) { + public Builder setUserValueTypeHint(String userValueTypeHint) { this.userValueTypeHint = userValueTypeHint; return this; } - public ValueType getUserValueTypeHint() { + @Deprecated + public Builder setUserValueTypeHint(ValueType userValueTypeHint) { + this.userValueTypeHint = userValueTypeHint == null ? null : userValueTypeHint.getPreferredName(); + return this; + } + + public String getUserValueTypeHint() { return userValueTypeHint; } 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 4fe17985fdc02..1c93d099c1b6b 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 @@ -8,7 +8,6 @@ package org.elasticsearch.search.aggregations.support; -import org.elasticsearch.common.ParsingException; import org.elasticsearch.xcontent.AbstractObjectParser; import org.elasticsearch.xcontent.ObjectParser; import org.elasticsearch.xcontent.ParseField; @@ -18,26 +17,15 @@ public final class MultiValuesSourceParseHelper { public static void declareCommon( AbstractObjectParser, T> objectParser, - boolean formattable, - ValueType expectedValueType + boolean formattable ) { - objectParser.declareField(MultiValuesSourceAggregationBuilder::userValueTypeHint, p -> { - ValueType valueType = ValueType.lenientParse(p.text()); - if (expectedValueType != null && valueType.isNotA(expectedValueType)) { - throw new ParsingException( - p.getTokenLocation(), - "Aggregation [" - + objectParser.getName() - + "] was configured with an incompatible value type [" - + valueType - + "]. It can only work on value off type [" - + expectedValueType - + "]" - ); - } - return valueType; - }, ValueType.VALUE_TYPE, ObjectParser.ValueType.STRING); + objectParser.declareField( + MultiValuesSourceAggregationBuilder::userValueTypeHint, + XContentParser::text, + ValueType.VALUE_TYPE, + ObjectParser.ValueType.STRING + ); if (formattable) { objectParser.declareField( diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfigTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfigTests.java index 7fc07c5e78758..78fd58f2fc7ae 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfigTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfigTests.java @@ -49,7 +49,7 @@ protected MultiValuesSourceFieldConfig createTestInstance() { .setTimeZone(timeZone) .setFilter(filter) .setFormat(format) - .setUserValueTypeHint(userValueTypeHint) + .setUserValueTypeHint(userValueTypeHint.getPreferredName()) .build(); } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java index 5e2ec1f9aa813..1eee24d115941 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java @@ -374,7 +374,7 @@ protected List resolveConfig(AggregationContext context) { configs.add( ValuesSourceConfig.resolveUnregistered( context, - field.getUserValueTypeHint(), + context.getValuesSourceRegistry().resolveTypeHint(field.getUserValueTypeHint()), field.getFieldName(), field.getScript(), field.getMissing(), diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/ttest/TTestAggregationBuilder.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/ttest/TTestAggregationBuilder.java index 2fb36f0eaaa0b..6b039bff3f6f8 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/ttest/TTestAggregationBuilder.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/ttest/TTestAggregationBuilder.java @@ -21,7 +21,6 @@ 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.ValuesSourceConfig; import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; import org.elasticsearch.search.aggregations.support.ValuesSourceType; @@ -44,7 +43,7 @@ public class TTestAggregationBuilder extends MultiValuesSourceAggregationBuilder public static final ObjectParser PARSER = ObjectParser.fromBuilder(NAME, TTestAggregationBuilder::new); static { - MultiValuesSourceParseHelper.declareCommon(PARSER, true, ValueType.NUMERIC); + MultiValuesSourceParseHelper.declareCommon(PARSER, true); MultiValuesSourceParseHelper.declareField(A_FIELD.getPreferredName(), PARSER, true, false, true, false); MultiValuesSourceParseHelper.declareField(B_FIELD.getPreferredName(), PARSER, true, false, true, false); PARSER.declareString(TTestAggregationBuilder::testType, TYPE_FIELD); diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregatorTests.java index d14bccd0f6058..3ebf7df19b933 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregatorTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregatorTests.java @@ -330,7 +330,7 @@ public void testScripts() throws IOException { new MultiValuesSourceFieldConfig.Builder().setFieldName(KEYWORD_FIELD).build(), new MultiValuesSourceFieldConfig.Builder().setScript( new Script(ScriptType.INLINE, MockScriptEngine.NAME, ADD_ONE_SCRIPT, Collections.singletonMap("fieldname", INT_FIELD)) - ).setUserValueTypeHint(ValueType.LONG).build(), + ).setUserValueTypeHint(ValueType.LONG.getPreferredName()).build(), new MultiValuesSourceFieldConfig.Builder().setFieldName(INT_FIELD).build() ), null, diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/GeoLineAggregationBuilder.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/GeoLineAggregationBuilder.java index 4147ae66783fe..fa9c6154b52ac 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/GeoLineAggregationBuilder.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/GeoLineAggregationBuilder.java @@ -19,7 +19,6 @@ 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.ValuesSourceConfig; import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; import org.elasticsearch.search.aggregations.support.ValuesSourceType; @@ -48,7 +47,7 @@ public class GeoLineAggregationBuilder extends MultiValuesSourceAggregationBuild GeoLineAggregationBuilder::new ); static { - MultiValuesSourceParseHelper.declareCommon(PARSER, true, ValueType.NUMERIC); + MultiValuesSourceParseHelper.declareCommon(PARSER, true); MultiValuesSourceParseHelper.declareField(POINT_FIELD.getPreferredName(), PARSER, true, false, false, false); MultiValuesSourceParseHelper.declareField(SORT_FIELD.getPreferredName(), PARSER, true, false, false, false); PARSER.declareString((builder, order) -> builder.sortOrder(SortOrder.fromString(order)), ORDER_FIELD); From 9d39d8b02cc68c50185e8632c32077a4e6bb858a Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Mon, 15 Nov 2021 13:13:57 -0500 Subject: [PATCH 03/19] Split Numeric VST into Longs and Doubles The goal of this change is to let numeric script values sources know if they are floating point or not. Previously, this was done by plumbing the ValueType deep into the ValuesSource. Now, instead, pass in a boolean, but that in turn requires the VST to know if it's floating point or not. Thus this split. --- .../matrix/ArrayValuesSourceParser.java | 3 +- .../fielddata/DoubleScriptFieldData.java | 2 +- .../fielddata/IndexNumericFieldData.java | 14 ++-- .../index/fielddata/LongScriptFieldData.java | 2 +- .../DateHistogramValuesSourceBuilder.java | 2 +- .../HistogramValuesSourceBuilder.java | 4 +- .../composite/TermsValuesSourceBuilder.java | 2 +- .../AutoDateHistogramAggregatorFactory.java | 2 +- .../DateHistogramAggregatorFactory.java | 2 +- .../HistogramAggregationBuilder.java | 2 +- .../histogram/HistogramAggregatorFactory.java | 2 +- ...iableWidthHistogramAggregationBuilder.java | 2 +- ...riableWidthHistogramAggregatorFactory.java | 3 +- .../range/DateRangeAggregationBuilder.java | 7 +- .../bucket/range/InternalRange.java | 2 +- .../bucket/range/RangeAggregationBuilder.java | 2 +- .../sampler/DiversifiedAggregatorFactory.java | 2 +- .../terms/RareTermsAggregatorFactory.java | 2 +- .../SignificantTermsAggregatorFactory.java | 2 +- .../bucket/terms/TermsAggregatorFactory.java | 2 +- .../metrics/AvgAggregationBuilder.java | 2 +- .../metrics/AvgAggregatorFactory.java | 2 +- .../ExtendedStatsAggregationBuilder.java | 2 +- .../ExtendedStatsAggregatorFactory.java | 2 +- .../metrics/MaxAggregationBuilder.java | 2 +- .../metrics/MaxAggregatorFactory.java | 2 +- ...anAbsoluteDeviationAggregationBuilder.java | 2 +- ...ianAbsoluteDeviationAggregatorFactory.java | 3 +- .../metrics/MinAggregationBuilder.java | 2 +- .../metrics/MinAggregatorFactory.java | 2 +- .../PercentileRanksAggregationBuilder.java | 2 +- .../PercentileRanksAggregatorFactory.java | 2 +- .../PercentilesAggregationBuilder.java | 2 +- .../metrics/PercentilesAggregatorFactory.java | 2 +- .../metrics/StatsAggregationBuilder.java | 2 +- .../metrics/StatsAggregatorFactory.java | 2 +- .../metrics/SumAggregationBuilder.java | 2 +- .../metrics/SumAggregatorFactory.java | 2 +- .../WeightedAvgAggregationBuilder.java | 4 +- .../support/CoreValuesSourceType.java | 78 ++++++++++++++++--- .../aggregations/support/ValueType.java | 8 +- .../aggregations/support/ValuesSource.java | 8 +- .../support/ValuesSourceConfig.java | 3 +- .../missing/MissingAggregatorTests.java | 3 +- .../SignificantTermsAggregatorTests.java | 3 +- .../bucket/terms/TermsAggregatorTests.java | 3 +- .../metrics/AvgAggregatorTests.java | 2 +- .../HDRPercentileRanksAggregatorTests.java | 2 +- .../HDRPercentilesAggregatorTests.java | 2 +- .../metrics/MaxAggregatorTests.java | 2 +- ...edianAbsoluteDeviationAggregatorTests.java | 3 +- .../metrics/StatsAggregatorTests.java | 2 +- .../metrics/SumAggregatorTests.java | 2 +- ...TDigestPercentileRanksAggregatorTests.java | 2 +- .../TDigestPercentilesAggregatorTests.java | 2 +- .../metrics/ValueCountAggregatorTests.java | 3 +- .../support/CoreValuesSourceTypeTests.java | 3 +- .../aggregations/AggregatorTestCase.java | 3 +- .../boxplot/BoxplotAggregationBuilder.java | 2 +- .../boxplot/BoxplotAggregatorFactory.java | 2 +- .../MultiTermsAggregationBuilder.java | 8 +- .../rate/RateAggregationBuilder.java | 4 +- .../analytics/rate/RateAggregatorFactory.java | 3 +- .../TopMetricsAggregationBuilder.java | 7 +- .../TopMetricsAggregatorFactory.java | 2 +- .../ttest/TTestAggregationBuilder.java | 4 +- ...regatedPercentileRanksAggregatorTests.java | 3 +- ...eAggregatedPercentilesAggregatorTests.java | 3 +- .../HistoBackedAvgAggregatorTests.java | 3 +- .../HistoBackedMaxAggregatorTests.java | 3 +- .../HistoBackedMinAggregatorTests.java | 3 +- .../HistoBackedSumAggregatorTests.java | 3 +- .../HistoBackedValueCountAggregatorTests.java | 3 +- ...regatedPercentileRanksAggregatorTests.java | 3 +- ...eAggregatedPercentilesAggregatorTests.java | 3 +- .../boxplot/BoxplotAggregatorTests.java | 2 +- .../multiterms/MultiTermsAggregatorTests.java | 3 +- .../TopMetricsAggregatorMetricsTests.java | 13 ++-- .../analytics/ttest/TTestAggregatorTests.java | 2 +- ...gregateMetricBackedAvgAggregatorTests.java | 3 +- ...gregateMetricBackedMaxAggregatorTests.java | 3 +- ...gregateMetricBackedMinAggregatorTests.java | 3 +- ...gregateMetricBackedSumAggregatorTests.java | 3 +- ...MetricBackedValueCountAggregatorTests.java | 3 +- .../GeoLineAggregationBuilder.java | 2 +- 85 files changed, 215 insertions(+), 117 deletions(-) diff --git a/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/ArrayValuesSourceParser.java b/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/ArrayValuesSourceParser.java index 68daf3e8dbb6d..c19f8a5641eff 100644 --- a/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/ArrayValuesSourceParser.java +++ b/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/ArrayValuesSourceParser.java @@ -31,8 +31,9 @@ public abstract class ArrayValuesSourceParser implement public abstract static class NumericValuesSourceParser extends ArrayValuesSourceParser { + // NOCOMMIT: Long Support? IDK what we should do here. protected NumericValuesSourceParser(boolean formattable) { - super(formattable, CoreValuesSourceType.NUMERIC, ValueType.NUMERIC); + super(formattable, CoreValuesSourceType.DOUBLE, ValueType.NUMERIC); } } diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/DoubleScriptFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/DoubleScriptFieldData.java index 287e041e0764d..162b1936b7c81 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/DoubleScriptFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/DoubleScriptFieldData.java @@ -58,7 +58,7 @@ public String getFieldName() { @Override public ValuesSourceType getValuesSourceType() { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } @Override diff --git a/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java b/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java index dce94649e0088..445905a04aa32 100644 --- a/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java +++ b/server/src/main/java/org/elasticsearch/index/fielddata/IndexNumericFieldData.java @@ -39,15 +39,15 @@ public abstract class IndexNumericFieldData implements IndexFieldData { 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 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 cdd4906058c84..517450918d3e5 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 @@ -68,7 +68,7 @@ static HistogramValuesSourceBuilder parse(String name, XContentParser parser) th public static void register(ValuesSourceRegistry.Builder builder) { builder.register( REGISTRY_KEY, - List.of(CoreValuesSourceType.DATE, CoreValuesSourceType.NUMERIC), + List.of(CoreValuesSourceType.DATE, CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG), (valuesSourceConfig, interval, name, hasScript, format, missingBucket, missingOrder, order) -> { ValuesSource.Numeric numeric = (ValuesSource.Numeric) valuesSourceConfig.getValuesSource(); final HistogramValuesSource vs = new HistogramValuesSource(numeric, interval); @@ -166,7 +166,7 @@ public HistogramValuesSourceBuilder interval(double interval) { @Override protected ValuesSourceType getDefaultValuesSourceType() { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } @Override 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 dcbaed78ba063..e7c800ec33b4f 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 @@ -87,7 +87,7 @@ public String type() { static void register(ValuesSourceRegistry.Builder builder) { builder.register( REGISTRY_KEY, - List.of(CoreValuesSourceType.DATE, CoreValuesSourceType.NUMERIC, CoreValuesSourceType.BOOLEAN), + List.of(CoreValuesSourceType.DATE, CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.BOOLEAN), (valuesSourceConfig, name, hasScript, format, missingBucket, missingOrder, order) -> { final DocValueFormat docValueFormat; if (format == null && valuesSourceConfig.valueSourceType() == CoreValuesSourceType.DATE) { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/AutoDateHistogramAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/AutoDateHistogramAggregatorFactory.java index a19ed7bf7fede..2b7f5d8c8f1df 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/AutoDateHistogramAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/AutoDateHistogramAggregatorFactory.java @@ -32,7 +32,7 @@ public final class AutoDateHistogramAggregatorFactory extends ValuesSourceAggreg public static void registerAggregators(ValuesSourceRegistry.Builder builder) { builder.register( AutoDateHistogramAggregationBuilder.REGISTRY_KEY, - List.of(CoreValuesSourceType.DATE, CoreValuesSourceType.NUMERIC), + List.of(CoreValuesSourceType.DATE, CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG), AutoDateHistogramAggregator::build, true ); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java index 60aec42f26b3b..a9fa55112b785 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregatorFactory.java @@ -32,7 +32,7 @@ public final class DateHistogramAggregatorFactory extends ValuesSourceAggregator public static void registerAggregators(ValuesSourceRegistry.Builder builder) { builder.register( DateHistogramAggregationBuilder.REGISTRY_KEY, - List.of(CoreValuesSourceType.DATE, CoreValuesSourceType.NUMERIC), + List.of(CoreValuesSourceType.DATE, CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG), DateHistogramAggregator::build, true ); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramAggregationBuilder.java index 7e29e8d2dd11e..b10f20ed0cd83 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramAggregationBuilder.java @@ -103,7 +103,7 @@ public static void registerAggregators(ValuesSourceRegistry.Builder builder) { @Override protected ValuesSourceType defaultValueSourceType() { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } /** Create a new builder with the given name. */ diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramAggregatorFactory.java index 873cfa33648dc..45620868f6fed 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramAggregatorFactory.java @@ -42,7 +42,7 @@ static void registerAggregators(ValuesSourceRegistry.Builder builder) { builder.register( HistogramAggregationBuilder.REGISTRY_KEY, - List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), + List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), NumericHistogramAggregator::new, true ); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/VariableWidthHistogramAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/VariableWidthHistogramAggregationBuilder.java index 6cd3425934b22..698eede320249 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/VariableWidthHistogramAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/VariableWidthHistogramAggregationBuilder.java @@ -84,7 +84,7 @@ protected VariableWidthHistogramAggregationBuilder( @Override protected ValuesSourceType defaultValueSourceType() { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } public VariableWidthHistogramAggregationBuilder setNumBuckets(int numBuckets) { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/VariableWidthHistogramAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/VariableWidthHistogramAggregatorFactory.java index 17214e67ea2f8..84fd241644304 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/VariableWidthHistogramAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/VariableWidthHistogramAggregatorFactory.java @@ -19,6 +19,7 @@ import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; import java.io.IOException; +import java.util.List; import java.util.Map; public class VariableWidthHistogramAggregatorFactory extends ValuesSourceAggregatorFactory { @@ -26,7 +27,7 @@ public class VariableWidthHistogramAggregatorFactory extends ValuesSourceAggrega public static void registerAggregators(ValuesSourceRegistry.Builder builder) { builder.register( VariableWidthHistogramAggregationBuilder.REGISTRY_KEY, - CoreValuesSourceType.NUMERIC, + List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG), VariableWidthHistogramAggregator::new, true ); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/DateRangeAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/DateRangeAggregationBuilder.java index d78aa5dde1611..002c8d9c63314 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/DateRangeAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/DateRangeAggregationBuilder.java @@ -55,7 +55,12 @@ public class DateRangeAggregationBuilder extends AbstractRangeBuilder> { public ValuesSourceType getValueSourceType() { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } public ValueType getValueType() { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregationBuilder.java index e3c3c56729600..523a8a3b703c2 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregationBuilder.java @@ -48,7 +48,7 @@ public class RangeAggregationBuilder extends AbstractRangeBuilder metricNames() { @Override protected ValuesSourceType defaultValueSourceType() { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsAggregatorFactory.java index 23292bf67db16..892a99abdc804 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ExtendedStatsAggregatorFactory.java @@ -45,7 +45,7 @@ class ExtendedStatsAggregatorFactory extends ValuesSourceAggregatorFactory { static void registerAggregators(ValuesSourceRegistry.Builder builder) { builder.register( ExtendedStatsAggregationBuilder.REGISTRY_KEY, - List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), + List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), ExtendedStatsAggregator::new, true ); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregationBuilder.java index 4c79c427404b0..103db061c6ebf 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregationBuilder.java @@ -58,7 +58,7 @@ protected MaxAggregationBuilder( @Override protected ValuesSourceType defaultValueSourceType() { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorFactory.java index 4effad4e687d2..5a0c60b4e1809 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorFactory.java @@ -29,7 +29,7 @@ class MaxAggregatorFactory extends ValuesSourceAggregatorFactory { static void registerAggregators(ValuesSourceRegistry.Builder builder) { builder.register( MaxAggregationBuilder.REGISTRY_KEY, - List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), + List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), MaxAggregator::new, true ); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregationBuilder.java index e43c61400cb35..d4534938a9d88 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregationBuilder.java @@ -100,7 +100,7 @@ protected AggregationBuilder shallowCopy(AggregatorFactories.Builder factoriesBu @Override protected ValuesSourceType defaultValueSourceType() { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregatorFactory.java index 80082d83b2c88..b0ddcc0915095 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregatorFactory.java @@ -19,6 +19,7 @@ import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; import java.io.IOException; +import java.util.List; import java.util.Map; public class MedianAbsoluteDeviationAggregatorFactory extends ValuesSourceAggregatorFactory { @@ -45,7 +46,7 @@ public class MedianAbsoluteDeviationAggregatorFactory extends ValuesSourceAggreg static void registerAggregators(ValuesSourceRegistry.Builder builder) { builder.register( MedianAbsoluteDeviationAggregationBuilder.REGISTRY_KEY, - CoreValuesSourceType.NUMERIC, + List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG), MedianAbsoluteDeviationAggregator::new, true ); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregationBuilder.java index 6c47fd5364846..b4ba067aeb178 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregationBuilder.java @@ -70,7 +70,7 @@ public MinAggregationBuilder(StreamInput in) throws IOException { @Override protected ValuesSourceType defaultValueSourceType() { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorFactory.java index c6e7f5eee0890..3d472e325be44 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregatorFactory.java @@ -29,7 +29,7 @@ class MinAggregatorFactory extends ValuesSourceAggregatorFactory { static void registerAggregators(ValuesSourceRegistry.Builder builder) { builder.register( MinAggregationBuilder.REGISTRY_KEY, - List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), + List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), MinAggregator::new, true ); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregationBuilder.java index b0addd861c578..67030606283a5 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregationBuilder.java @@ -69,7 +69,7 @@ protected AggregationBuilder shallowCopy(AggregatorFactories.Builder factoriesBu @Override protected ValuesSourceType defaultValueSourceType() { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } /** diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregatorFactory.java index 058ebce142e18..9baeef461f459 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentileRanksAggregatorFactory.java @@ -32,7 +32,7 @@ class PercentileRanksAggregatorFactory extends ValuesSourceAggregatorFactory { static void registerAggregators(ValuesSourceRegistry.Builder builder) { builder.register( PercentileRanksAggregationBuilder.REGISTRY_KEY, - List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), + List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), (name, valuesSource, context, parent, percents, percentilesConfig, keyed, formatter, metadata) -> percentilesConfig .createPercentileRanksAggregator(name, valuesSource, context, parent, percents, keyed, formatter, metadata), true diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregationBuilder.java index f3604e41e59fc..0f45451cd57b1 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregationBuilder.java @@ -74,7 +74,7 @@ protected AggregationBuilder shallowCopy(AggregatorFactories.Builder factoriesBu @Override protected ValuesSourceType defaultValueSourceType() { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } /** diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregatorFactory.java index b28b33cc462e0..67cd99909f364 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/PercentilesAggregatorFactory.java @@ -36,7 +36,7 @@ class PercentilesAggregatorFactory extends ValuesSourceAggregatorFactory { static void registerAggregators(ValuesSourceRegistry.Builder builder) { builder.register( PercentilesAggregationBuilder.REGISTRY_KEY, - List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), + List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), (name, valuesSource, context, parent, percents, percentilesConfig, keyed, formatter, metadata) -> percentilesConfig .createPercentilesAggregator(name, valuesSource, context, parent, percents, keyed, formatter, metadata), true diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/StatsAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/StatsAggregationBuilder.java index 64e54f37b2f02..c0921ec94bd51 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/StatsAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/StatsAggregationBuilder.java @@ -77,7 +77,7 @@ public Set metricNames() { @Override protected ValuesSourceType defaultValueSourceType() { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/StatsAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/StatsAggregatorFactory.java index 2d311c9e05408..5cb8f986d5680 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/StatsAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/StatsAggregatorFactory.java @@ -43,7 +43,7 @@ class StatsAggregatorFactory extends ValuesSourceAggregatorFactory { static void registerAggregators(ValuesSourceRegistry.Builder builder) { builder.register( StatsAggregationBuilder.REGISTRY_KEY, - List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), + List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), StatsAggregator::new, true ); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/SumAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/SumAggregationBuilder.java index 9bc74f57cee0b..e329ebcedc60e 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/SumAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/SumAggregationBuilder.java @@ -70,7 +70,7 @@ public SumAggregationBuilder(StreamInput in) throws IOException { @Override protected ValuesSourceType defaultValueSourceType() { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorFactory.java index a14764a5ca25e..d5af6790e5392 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorFactory.java @@ -43,7 +43,7 @@ class SumAggregatorFactory extends ValuesSourceAggregatorFactory { static void registerAggregators(ValuesSourceRegistry.Builder builder) { builder.register( SumAggregationBuilder.REGISTRY_KEY, - List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), + List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN), SumAggregator::new, true ); 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 0c31c4ac98868..b0118fdd78924 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 @@ -49,7 +49,7 @@ public class WeightedAvgAggregationBuilder extends MultiValuesSourceAggregationB } public static void registerUsage(ValuesSourceRegistry.Builder builder) { - builder.registerUsage(NAME, CoreValuesSourceType.NUMERIC); + builder.registerUsage(NAME, CoreValuesSourceType.DOUBLE); } public WeightedAvgAggregationBuilder(String name) { @@ -86,7 +86,7 @@ protected AggregationBuilder shallowCopy(Builder factoriesBuilder, Map indexFieldData = fieldContext.indexFieldData(); if (indexFieldData instanceof IndexNumericFieldData) { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } else if (indexFieldData instanceof IndexGeoPointFieldData) { return CoreValuesSourceType.GEOPOINT; } else if (fieldContext.fieldType() instanceof RangeFieldMapper.RangeFieldType) { diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/missing/MissingAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/missing/MissingAggregatorTests.java index b96c5e8f0d355..be2c6e8f533d3 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/missing/MissingAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/missing/MissingAggregatorTests.java @@ -338,7 +338,8 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.KEYWORD, CoreValuesSourceType.GEOPOINT, CoreValuesSourceType.RANGE, diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/SignificantTermsAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/SignificantTermsAggregatorTests.java index a6c1adcc6f9e9..e469eb3fd889c 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/SignificantTermsAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/SignificantTermsAggregatorTests.java @@ -77,7 +77,8 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.KEYWORD, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE, diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java index 0eb3aba58283c..dd875b1108e05 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java @@ -194,7 +194,8 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.KEYWORD, CoreValuesSourceType.IP, CoreValuesSourceType.DATE, diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java index 55a344dd8c1e8..068721bddbb79 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/AvgAggregatorTests.java @@ -700,7 +700,7 @@ public void testScriptCaching() throws IOException { @Override protected List getSupportedValuesSourceTypes() { - return List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE); + return List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE); } @Override diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksAggregatorTests.java index eddc12a095ab0..c817d8a0d54f3 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentileRanksAggregatorTests.java @@ -40,7 +40,7 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { - return List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN); + return List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN); } public void testEmpty() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesAggregatorTests.java index 0a82a569e8c55..8b6552e2f7446 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/HDRPercentilesAggregatorTests.java @@ -53,7 +53,7 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { - return List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN); + return List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN); } public void testNoDocs() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java index 52652893ecc1d..1eff5fc51b824 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MaxAggregatorTests.java @@ -153,7 +153,7 @@ protected ScriptService getMockScriptService() { @Override protected List getSupportedValuesSourceTypes() { - return List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE); + return List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE); } @Override diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregatorTests.java index 22275cfc4876a..9270c49718866 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/MedianAbsoluteDeviationAggregatorTests.java @@ -46,7 +46,6 @@ import java.util.stream.IntStream; import static java.util.Collections.singleton; -import static java.util.Collections.singletonList; import static org.elasticsearch.search.aggregations.metrics.MedianAbsoluteDeviationAggregatorTests.ExactMedianAbsoluteDeviation.calculateMAD; import static org.elasticsearch.search.aggregations.metrics.MedianAbsoluteDeviationAggregatorTests.IsCloseToRelative.closeToRelative; import static org.hamcrest.Matchers.equalTo; @@ -307,7 +306,7 @@ private static double calculateMedian(double[] sample) { @Override protected List getSupportedValuesSourceTypes() { - return singletonList(CoreValuesSourceType.NUMERIC); + return List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG); } @Override diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsAggregatorTests.java index 05c1667a5cc37..9a9c2af343634 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/StatsAggregatorTests.java @@ -426,7 +426,7 @@ void add(double value) { @Override protected List getSupportedValuesSourceTypes() { - return List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE); + return List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE); } @Override diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java index 8cd8358cae043..7564b3a177398 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/SumAggregatorTests.java @@ -384,7 +384,7 @@ private void testAggregation( @Override protected List getSupportedValuesSourceTypes() { - return List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE); + return List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE); } @Override diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksAggregatorTests.java index 6b99e20f4a399..fbe7f2299e36c 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentileRanksAggregatorTests.java @@ -40,7 +40,7 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { - return List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN); + return List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN); } public void testEmpty() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesAggregatorTests.java index c50aee1b430b5..bccbb92619af6 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TDigestPercentilesAggregatorTests.java @@ -46,7 +46,7 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { - return List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN); + return List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN); } public void testNoDocs() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountAggregatorTests.java index 63e8b0ce7e651..31d05ef47892a 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/ValueCountAggregatorTests.java @@ -75,7 +75,8 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.KEYWORD, CoreValuesSourceType.GEOPOINT, CoreValuesSourceType.RANGE, diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/support/CoreValuesSourceTypeTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/support/CoreValuesSourceTypeTests.java index 9655a353da8e3..0f8950503c5e6 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/support/CoreValuesSourceTypeTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/support/CoreValuesSourceTypeTests.java @@ -25,7 +25,8 @@ public class CoreValuesSourceTypeTests extends MapperServiceTestCase { public void testFromString() { - assertThat(CoreValuesSourceType.fromString("numeric"), equalTo(CoreValuesSourceType.NUMERIC)); + assertThat(CoreValuesSourceType.fromString("double"), equalTo(CoreValuesSourceType.DOUBLE)); + assertThat(CoreValuesSourceType.fromString("long"), equalTo(CoreValuesSourceType.LONG)); assertThat(CoreValuesSourceType.fromString("keyword"), equalTo(CoreValuesSourceType.KEYWORD)); assertThat(CoreValuesSourceType.fromString("geopoint"), equalTo(CoreValuesSourceType.GEOPOINT)); assertThat(CoreValuesSourceType.fromString("range"), equalTo(CoreValuesSourceType.RANGE)); diff --git a/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java b/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java index 533e56e6936ef..f360cbbd28134 100644 --- a/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java @@ -981,7 +981,8 @@ private void writeTestDoc(MappedFieldType fieldType, String fieldName, RandomInd Document doc = new Document(); String json; - if (vst.equals(CoreValuesSourceType.NUMERIC)) { + // NOCOMMI: Break out long and double logic? + if (vst.equals(CoreValuesSourceType.DOUBLE) || vst.equals(CoreValuesSourceType.LONG)) { long v; if (typeName.equals(NumberFieldMapper.NumberType.DOUBLE.typeName())) { double d = Math.abs(randomDouble()); diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregationBuilder.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregationBuilder.java index 8ad0379815dbd..1531c67eadfc8 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregationBuilder.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregationBuilder.java @@ -93,7 +93,7 @@ protected void innerWriteTo(StreamOutput out) throws IOException { @Override protected ValuesSourceType defaultValueSourceType() { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } /** diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregatorFactory.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregatorFactory.java index d3bb7f4031836..2bd096bb663ab 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregatorFactory.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregatorFactory.java @@ -30,7 +30,7 @@ public class BoxplotAggregatorFactory extends ValuesSourceAggregatorFactory { static void registerAggregators(ValuesSourceRegistry.Builder builder) { builder.register( BoxplotAggregationBuilder.REGISTRY_KEY, - List.of(CoreValuesSourceType.NUMERIC, AnalyticsValuesSourceType.HISTOGRAM), + List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, AnalyticsValuesSourceType.HISTOGRAM), BoxplotAggregator::new, true ); diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java index 1eee24d115941..5f7ff90dc5b2a 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java @@ -112,7 +112,13 @@ interface MultiTermValuesSupplier { public static void registerAggregators(ValuesSourceRegistry.Builder registry) { registry.registerUsage(NAME); - registry.register(REGISTRY_KEY, List.of(CoreValuesSourceType.NUMERIC), MultiTermsAggregator::buildNumericTermValues, false); + // NOCOMMIT: Should Longs use the LongTermsValuesSource? This mirrors the numeric behavior, but might be wrong. + registry.register( + REGISTRY_KEY, + List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG), + MultiTermsAggregator::buildNumericTermValues, + false + ); registry.register( REGISTRY_KEY, List.of(CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE), diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/rate/RateAggregationBuilder.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/rate/RateAggregationBuilder.java index eb1f9815e1875..3d4dc776915b2 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/rate/RateAggregationBuilder.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/rate/RateAggregationBuilder.java @@ -94,7 +94,7 @@ public RateAggregationBuilder(StreamInput in) throws IOException { @Override protected ValuesSourceType defaultValueSourceType() { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } @Override @@ -191,7 +191,7 @@ static Rounding.DateTimeUnit parse(String rateUnit) { @Override protected ValuesSourceConfig resolveConfig(AggregationContext context) { if (field() == null && script() == null) { - return new ValuesSourceConfig(CoreValuesSourceType.NUMERIC, null, true, null, null, 1.0, null, DocValueFormat.RAW, context); + return new ValuesSourceConfig(CoreValuesSourceType.DOUBLE, null, true, null, null, 1.0, null, DocValueFormat.RAW, context); } else { return super.resolveConfig(context); } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/rate/RateAggregatorFactory.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/rate/RateAggregatorFactory.java index e5a243012395b..ff01fdb8c1e72 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/rate/RateAggregatorFactory.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/rate/RateAggregatorFactory.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.util.Collections; +import java.util.List; import java.util.Map; class RateAggregatorFactory extends ValuesSourceAggregatorFactory { @@ -54,7 +55,7 @@ class RateAggregatorFactory extends ValuesSourceAggregatorFactory { static void registerAggregators(ValuesSourceRegistry.Builder builder) { builder.register( RateAggregationBuilder.REGISTRY_KEY, - Collections.singletonList(CoreValuesSourceType.NUMERIC), + List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG), NumericRateAggregator::new, true ); diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregationBuilder.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregationBuilder.java index bdf28c33786fe..8a7992de0e6db 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregationBuilder.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregationBuilder.java @@ -48,7 +48,12 @@ public class TopMetricsAggregationBuilder extends AbstractAggregationBuilder getSupportedValuesSourceTypes() { // Note: this is the same list as Core, plus Analytics return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN, AnalyticsValuesSourceType.HISTOGRAM diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HDRPreAggregatedPercentilesAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HDRPreAggregatedPercentilesAggregatorTests.java index fec2fe1a33a19..9c0528e9e083a 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HDRPreAggregatedPercentilesAggregatorTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HDRPreAggregatedPercentilesAggregatorTests.java @@ -59,7 +59,8 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy protected List getSupportedValuesSourceTypes() { // Note: this is the same list as Core, plus Analytics return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN, AnalyticsValuesSourceType.HISTOGRAM diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedAvgAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedAvgAggregatorTests.java index d7296aef1b401..8457d00b9bf76 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedAvgAggregatorTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedAvgAggregatorTests.java @@ -123,7 +123,8 @@ protected List getSearchPlugins() { protected List getSupportedValuesSourceTypes() { // Note: this is the same list as Core, plus Analytics return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE, AnalyticsValuesSourceType.HISTOGRAM diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMaxAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMaxAggregatorTests.java index 88ba9e9234799..dfa195470ca85 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMaxAggregatorTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMaxAggregatorTests.java @@ -123,7 +123,8 @@ protected List getSearchPlugins() { protected List getSupportedValuesSourceTypes() { // Note: this is the same list as Core, plus Analytics return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE, AnalyticsValuesSourceType.HISTOGRAM diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMinAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMinAggregatorTests.java index 3066225b6f629..0f9ddf3950c2b 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMinAggregatorTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMinAggregatorTests.java @@ -123,7 +123,8 @@ protected List getSearchPlugins() { protected List getSupportedValuesSourceTypes() { // Note: this is the same list as Core, plus Analytics return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE, AnalyticsValuesSourceType.HISTOGRAM diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedSumAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedSumAggregatorTests.java index 52b2119b56242..4ca48e29d9bb7 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedSumAggregatorTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedSumAggregatorTests.java @@ -123,7 +123,8 @@ protected List getSearchPlugins() { protected List getSupportedValuesSourceTypes() { // Note: this is the same list as Core, plus Analytics return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE, AnalyticsValuesSourceType.HISTOGRAM diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedValueCountAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedValueCountAggregatorTests.java index 039e5f474007b..cd99c5af2ae8f 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedValueCountAggregatorTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedValueCountAggregatorTests.java @@ -123,7 +123,8 @@ protected List getSearchPlugins() { protected List getSupportedValuesSourceTypes() { // Note: this is the same list as Core, plus Analytics return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.KEYWORD, CoreValuesSourceType.GEOPOINT, CoreValuesSourceType.RANGE, diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/TDigestPreAggregatedPercentileRanksAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/TDigestPreAggregatedPercentileRanksAggregatorTests.java index a7662e0358b48..3a62bd0e865d0 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/TDigestPreAggregatedPercentileRanksAggregatorTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/TDigestPreAggregatedPercentileRanksAggregatorTests.java @@ -54,7 +54,8 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy protected List getSupportedValuesSourceTypes() { // Note: this is the same list as Core, plus Analytics return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN, AnalyticsValuesSourceType.HISTOGRAM diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/TDigestPreAggregatedPercentilesAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/TDigestPreAggregatedPercentilesAggregatorTests.java index 024212c5d8c64..b62242f9f4aa2 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/TDigestPreAggregatedPercentilesAggregatorTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/TDigestPreAggregatedPercentilesAggregatorTests.java @@ -55,7 +55,8 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy protected List getSupportedValuesSourceTypes() { // Note: this is the same list as Core, plus Analytics return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN, AnalyticsValuesSourceType.HISTOGRAM diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregatorTests.java index 0ad234c425891..3348aacdb2b7d 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregatorTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/boxplot/BoxplotAggregatorTests.java @@ -67,7 +67,7 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { - return List.of(CoreValuesSourceType.NUMERIC, AnalyticsValuesSourceType.HISTOGRAM); + return List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, AnalyticsValuesSourceType.HISTOGRAM); } @Override diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregatorTests.java index 3ebf7df19b933..4e3a6aa3988ec 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregatorTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregatorTests.java @@ -90,7 +90,8 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.KEYWORD, diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregatorMetricsTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregatorMetricsTests.java index 0ab41367b6d50..edd8c99e7ee73 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregatorMetricsTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregatorMetricsTests.java @@ -56,7 +56,8 @@ public class TopMetricsAggregatorMetricsTests extends ESTestCase { } public void testNoNumbers() throws IOException { - assertNoValues(toConfig(null, CoreValuesSourceType.NUMERIC, DocValueFormat.RAW, false)); + assertNoValues(toConfig(null, CoreValuesSourceType.DOUBLE, DocValueFormat.RAW, false)); + assertNoValues(toConfig(null, CoreValuesSourceType.LONG, DocValueFormat.RAW, false)); } public void testNoDates() throws IOException { @@ -203,7 +204,8 @@ private ValuesSourceConfig toConfig(SortedNumericDocValues values) throws IOExce when(source.isFloatingPoint()).thenReturn(false); when(source.longValues(null)).thenReturn(values); if (randomBoolean()) { - return toConfig(source, CoreValuesSourceType.NUMERIC, randomWholeNumberDocValuesFormat(), true); + // NOCOMMIT: should we have a case for CVST.LONG here? + return toConfig(source, CoreValuesSourceType.DOUBLE, randomWholeNumberDocValuesFormat(), true); } DocValueFormat dateFormatter = new DocValueFormat.DateTime( DateFormatter.forPattern(randomDateFormatterPattern()), @@ -217,12 +219,7 @@ private ValuesSourceConfig toConfig(SortedNumericDoubleValues values) throws IOE ValuesSource.Numeric source = mock(ValuesSource.Numeric.class); when(source.isFloatingPoint()).thenReturn(true); when(source.doubleValues(null)).thenReturn(values); - return toConfig( - source, - CoreValuesSourceType.NUMERIC, - randomFrom(DocValueFormat.RAW, new DocValueFormat.Decimal("####.####")), - true - ); + return toConfig(source, CoreValuesSourceType.DOUBLE, randomFrom(DocValueFormat.RAW, new DocValueFormat.Decimal("####.####")), true); } private ValuesSourceConfig toConfig(SortedSetDocValues values) throws IOException { diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/ttest/TTestAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/ttest/TTestAggregatorTests.java index 3ef0f4b64a3b4..21e405495bd25 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/ttest/TTestAggregatorTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/ttest/TTestAggregatorTests.java @@ -110,7 +110,7 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { - return List.of(CoreValuesSourceType.NUMERIC, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE); + return List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG, CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE); } @Override diff --git a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedAvgAggregatorTests.java b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedAvgAggregatorTests.java index 7abe41574c92e..843af8141ba2d 100644 --- a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedAvgAggregatorTests.java +++ b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedAvgAggregatorTests.java @@ -151,7 +151,8 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN, AggregateMetricsValuesSourceType.AGGREGATE_METRIC diff --git a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMaxAggregatorTests.java b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMaxAggregatorTests.java index d4840a8a6a73a..7dbba90e7d72b 100644 --- a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMaxAggregatorTests.java +++ b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMaxAggregatorTests.java @@ -151,7 +151,8 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN, AggregateMetricsValuesSourceType.AGGREGATE_METRIC diff --git a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMinAggregatorTests.java b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMinAggregatorTests.java index 4d28508c6b35c..4fbaafe425484 100644 --- a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMinAggregatorTests.java +++ b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedMinAggregatorTests.java @@ -151,7 +151,8 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN, AggregateMetricsValuesSourceType.AGGREGATE_METRIC diff --git a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedSumAggregatorTests.java b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedSumAggregatorTests.java index c1c607e051a4f..6699c9941dbcb 100644 --- a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedSumAggregatorTests.java +++ b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedSumAggregatorTests.java @@ -151,7 +151,8 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.DATE, CoreValuesSourceType.BOOLEAN, AggregateMetricsValuesSourceType.AGGREGATE_METRIC diff --git a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedValueCountAggregatorTests.java b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedValueCountAggregatorTests.java index 9fd3a7eb7c4c5..4d0d45460f249 100644 --- a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedValueCountAggregatorTests.java +++ b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/aggregations/metrics/AggregateMetricBackedValueCountAggregatorTests.java @@ -150,7 +150,8 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { return List.of( - CoreValuesSourceType.NUMERIC, + CoreValuesSourceType.DOUBLE, + CoreValuesSourceType.LONG, CoreValuesSourceType.KEYWORD, CoreValuesSourceType.GEOPOINT, CoreValuesSourceType.RANGE, diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/GeoLineAggregationBuilder.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/GeoLineAggregationBuilder.java index fa9c6154b52ac..e3e2b161cc5e0 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/GeoLineAggregationBuilder.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/GeoLineAggregationBuilder.java @@ -123,7 +123,7 @@ protected void innerWriteTo(StreamOutput out) throws IOException { @Override protected ValuesSourceType defaultValueSourceType() { - return CoreValuesSourceType.NUMERIC; + return CoreValuesSourceType.DOUBLE; } @Override From f642826c524828de0a0732923cc1dc2072588a45 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Thu, 18 Nov 2021 10:53:56 -0500 Subject: [PATCH 04/19] VSConfig#resolve takes a VST --- .../ArrayValuesSourceAggregationBuilder.java | 2 +- .../CompositeValuesSourceParserHelper.java | 2 +- .../support/CoreValuesSourceType.java | 22 +++--- .../MultiValuesSourceAggregationBuilder.java | 6 +- .../support/ValuesSourceConfig.java | 34 +++----- .../support/ValuesSourceRegistry.java | 30 ++++++-- .../support/ValuesSourceType.java | 3 +- .../support/ValuesSourceConfigTests.java | 77 +++++++++++++++++-- .../support/ValuesSourceRegistryTests.java | 2 +- .../support/AnalyticsValuesSourceType.java | 3 +- .../rate/RateAggregationBuilder.java | 2 +- .../AggregateMetricsValuesSourceType.java | 3 +- .../support/GeoShapeValuesSourceType.java | 3 +- .../xpack/spatial/SpatialPluginTests.java | 4 +- 14 files changed, 130 insertions(+), 63 deletions(-) diff --git a/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/ArrayValuesSourceAggregationBuilder.java b/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/ArrayValuesSourceAggregationBuilder.java index 1d945d1265fb9..c6d673138478b 100644 --- a/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/ArrayValuesSourceAggregationBuilder.java +++ b/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/ArrayValuesSourceAggregationBuilder.java @@ -201,7 +201,7 @@ protected Map resolveConfig(AggregationContext conte for (String field : fields) { ValuesSourceConfig config = ValuesSourceConfig.resolveUnregistered( context, - userValueTypeHint, + userValueTypeHint.getValuesSourceType(), field, null, missingMap.get(field), 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 8b0e00ad7bc77..de2d3e71ace98 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 @@ -31,7 +31,7 @@ static , T> void declareValuesSource objectParser.declareBoolean(VB::missingBucket, new ParseField("missing_bucket")); objectParser.declareString(VB::missingOrder, new ParseField("missing_order")); - objectParser.declareField(VB::userValuetypeHint,XContentParser::text, new ParseField("value_type"), ObjectParser.ValueType.STRING); + objectParser.declareField(VB::userValuetypeHint, XContentParser::text, new ParseField("value_type"), ObjectParser.ValueType.STRING); objectParser.declareField( VB::script, diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/CoreValuesSourceType.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/CoreValuesSourceType.java index cd130f0b1ff7f..463548bde59ed 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/CoreValuesSourceType.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/CoreValuesSourceType.java @@ -51,7 +51,7 @@ public ValuesSource getEmpty() { } @Override - public ValuesSource getScript(AggregationScript.LeafFactory script, ValueType scriptValueType) { + public ValuesSource getScript(AggregationScript.LeafFactory script) { return new ValuesSource.Numeric.Script(script, true); } @@ -111,7 +111,7 @@ public ValuesSource getEmpty() { } @Override - public ValuesSource getScript(AggregationScript.LeafFactory script, ValueType scriptValueType) { + public ValuesSource getScript(AggregationScript.LeafFactory script) { return new ValuesSource.Numeric.Script(script, false); } @@ -171,7 +171,7 @@ public ValuesSource getEmpty() { } @Override - public ValuesSource getScript(AggregationScript.LeafFactory script, ValueType scriptValueType) { + public ValuesSource getScript(AggregationScript.LeafFactory script) { return new ValuesSource.Bytes.Script(script); } @@ -213,7 +213,7 @@ public ValuesSource getEmpty() { } @Override - public ValuesSource getScript(AggregationScript.LeafFactory script, ValueType scriptValueType) { + public ValuesSource getScript(AggregationScript.LeafFactory script) { throw new AggregationExecutionException("value source of type [" + this.value() + "] is not supported by scripts"); } @@ -252,7 +252,7 @@ public ValuesSource getEmpty() { } @Override - public ValuesSource getScript(AggregationScript.LeafFactory script, ValueType scriptValueType) { + public ValuesSource getScript(AggregationScript.LeafFactory script) { throw new AggregationExecutionException("value source of type [" + this.value() + "] is not supported by scripts"); } @@ -284,8 +284,8 @@ public ValuesSource getEmpty() { } @Override - public ValuesSource getScript(AggregationScript.LeafFactory script, ValueType scriptValueType) { - return KEYWORD.getScript(script, scriptValueType); + public ValuesSource getScript(AggregationScript.LeafFactory script) { + return KEYWORD.getScript(script); } @Override @@ -315,8 +315,8 @@ public ValuesSource getEmpty() { } @Override - public ValuesSource getScript(AggregationScript.LeafFactory script, ValueType scriptValueType) { - return DOUBLE.getScript(script, scriptValueType); + public ValuesSource getScript(AggregationScript.LeafFactory script) { + return DOUBLE.getScript(script); } @Override @@ -436,8 +436,8 @@ public ValuesSource getEmpty() { } @Override - public ValuesSource getScript(AggregationScript.LeafFactory script, ValueType scriptValueType) { - return DOUBLE.getScript(script, scriptValueType); + public ValuesSource getScript(AggregationScript.LeafFactory script) { + return DOUBLE.getScript(script); } @Override 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 44e973100294d..12643c758bfe2 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 @@ -173,7 +173,7 @@ protected final MultiValuesSourceAggregatorFactory doBuild( ) throws IOException { Map configs = new HashMap<>(fields.size()); Map filters = new HashMap<>(fields.size()); - final ValueType resolvedTypeHint = context.getValuesSourceRegistry().resolveTypeHint(userValueTypeHint); + final ValuesSourceType resolvedTypeHint = context.getValuesSourceRegistry().resolveTypeHint(userValueTypeHint); fields.forEach((key, value) -> { ValuesSourceConfig config = ValuesSourceConfig.resolveUnregistered( context, @@ -195,14 +195,14 @@ protected final MultiValuesSourceAggregatorFactory doBuild( public static DocValueFormat resolveFormat( @Nullable String format, - @Nullable ValueType valueType, + @Nullable ValuesSourceType valueType, ValuesSourceType defaultValuesSourceType ) { if (valueType == null) { // 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; + DocValueFormat valueFormat = valueType.getFormatter(format, null); if (valueFormat instanceof DocValueFormat.Decimal && format != null) { valueFormat = new DocValueFormat.Decimal(format); } 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 2f80758b316dc..eda47e9bcb7b0 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 @@ -47,7 +47,7 @@ public class ValuesSourceConfig { */ public static ValuesSourceConfig resolve( AggregationContext context, - ValueType userValueTypeHint, + ValuesSourceType userValueTypeHint, String field, Script script, Object missing, @@ -87,7 +87,7 @@ public static ValuesSourceConfig resolve( */ public static ValuesSourceConfig resolveUnregistered( AggregationContext context, - ValueType userValueTypeHint, + ValuesSourceType userValueTypeHint, String field, Script script, Object missing, @@ -110,7 +110,7 @@ public static ValuesSourceConfig resolveUnregistered( private static ValuesSourceConfig internalResolve( AggregationContext context, - ValueType userValueTypeHint, + ValuesSourceType userValueTypeHint, String field, Script script, Object missing, @@ -121,13 +121,12 @@ private static ValuesSourceConfig internalResolve( ) { ValuesSourceConfig config; ValuesSourceType valuesSourceType = null; - ValueType scriptValueType = userValueTypeHint; FieldContext fieldContext = null; AggregationScript.LeafFactory aggregationScript = createScript(script, context); // returns null if script is null boolean unmapped = false; if (userValueTypeHint != null) { // If the user gave us a type hint, respect that. - valuesSourceType = userValueTypeHint.getValuesSourceType(); + valuesSourceType = userValueTypeHint; } if (field == null) { if (script == null) { @@ -160,7 +159,6 @@ private static ValuesSourceConfig internalResolve( fieldContext, unmapped, aggregationScript, - scriptValueType, missing, timeZone, docValueFormat, @@ -173,7 +171,7 @@ private static ValuesSourceConfig internalResolve( private interface FieldResolver { ValuesSourceType getValuesSourceType( FieldContext fieldContext, - ValueType userValueTypeHint, + ValuesSourceType userValueTypeHint, ValuesSourceType defaultValuesSourceType ); @@ -181,16 +179,15 @@ ValuesSourceType getValuesSourceType( private static ValuesSourceType getMappingFromRegistry( FieldContext fieldContext, - ValueType userValueTypeHint, + ValuesSourceType userValueTypeHint, ValuesSourceType defaultValuesSourceType ) { return fieldContext.indexFieldData().getValuesSourceType(); } - // NOCOMMIT: This whole method should probably get removed private static ValuesSourceType getLegacyMapping( FieldContext fieldContext, - ValueType userValueTypeHint, + ValuesSourceType userValueTypeHint, ValuesSourceType defaultValuesSourceType ) { IndexFieldData indexFieldData = fieldContext.indexFieldData(); @@ -204,7 +201,7 @@ private static ValuesSourceType getLegacyMapping( if (userValueTypeHint == null) { return defaultValuesSourceType; } else { - return userValueTypeHint.getValuesSourceType(); + return userValueTypeHint; } } } @@ -239,20 +236,19 @@ private static DocValueFormat resolveFormat( public static ValuesSourceConfig resolveFieldOnly(MappedFieldType fieldType, AggregationContext context) { FieldContext fieldContext = context.buildFieldContext(fieldType); ValuesSourceType vstype = fieldContext.indexFieldData().getValuesSourceType(); - return new ValuesSourceConfig(vstype, fieldContext, false, null, null, null, null, null, context); + return new ValuesSourceConfig(vstype, fieldContext, false, null, null, null, null, context); } /** * Convenience method for creating unmapped configs */ public static ValuesSourceConfig resolveUnmapped(ValuesSourceType valuesSourceType, AggregationContext context) { - return new ValuesSourceConfig(valuesSourceType, null, true, null, null, null, null, null, context); + return new ValuesSourceConfig(valuesSourceType, null, true, null, null, null, null, context); } private final ValuesSourceType valuesSourceType; private final FieldContext fieldContext; private final AggregationScript.LeafFactory script; - private final ValueType scriptValueType; private final boolean unmapped; private final DocValueFormat format; private final Object missing; @@ -268,7 +264,6 @@ public ValuesSourceConfig( FieldContext fieldContext, boolean unmapped, AggregationScript.LeafFactory script, - ValueType scriptValueType, Object missing, ZoneId timeZone, DocValueFormat format, @@ -281,7 +276,6 @@ public ValuesSourceConfig( this.fieldContext = fieldContext; this.unmapped = unmapped; this.script = script; - this.scriptValueType = scriptValueType; this.missing = missing; this.timeZone = timeZone; this.format = format == null ? DocValueFormat.RAW : format; @@ -302,7 +296,7 @@ private ValuesSource constructValuesSource(Object missing, DocValueFormat format } else { if (fieldContext() == null) { // Script case - vs = valueSourceType().getScript(script(), scriptValueType()); + vs = valueSourceType().getScript(script()); } else { // Field or Value Script case vs = valueSourceType().getField(fieldContext(), script(), context); @@ -359,10 +353,6 @@ public boolean valid() { return fieldContext != null || script != null || unmapped; } - public ValueType scriptValueType() { - return this.scriptValueType; - } - public Object missing() { return this.missing; } @@ -424,7 +414,7 @@ public boolean alignesWithSearchIndex() { */ public String getDescription() { if (script != null) { - return "Script yielding [" + (scriptValueType != null ? scriptValueType.getPreferredName() : "unknown type") + "]"; + return "Script"; } MappedFieldType fieldType = fieldType(); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java index c14d8ff213dd6..6b463e432ee35 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java @@ -13,10 +13,13 @@ import java.util.AbstractMap; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; /** * {@link ValuesSourceRegistry} holds the mapping from {@link ValuesSourceType}s to functions for building aggregation components. DO NOT @@ -59,6 +62,8 @@ public int hashCode() { public static class Builder { private final AggregationUsageService.Builder usageServiceBuilder; private Map, List>> aggregatorRegistry = new HashMap<>(); + private Map valueTypeLookup = new HashMap<>(); + private Set duplicateRegistrationCheck = new HashSet<>(); public Builder() { this.usageServiceBuilder = new AggregationUsageService.Builder(); @@ -120,8 +125,19 @@ public void registerUsage(String aggregationName) { usageServiceBuilder.registerAggregationUsage(aggregationName); } + public void registerValuesSourceType(ValuesSourceType valuesSourceType, Collection alternateNames) { + if (duplicateRegistrationCheck.contains(valuesSourceType)) { + throw new IllegalStateException("Duplicate registration of ValuesSourceType [" + valuesSourceType.typeName() + "]"); + } + valueTypeLookup.put(valuesSourceType.typeName(), valuesSourceType); + for (String name : alternateNames) { + valueTypeLookup.put(name, valuesSourceType); + } + duplicateRegistrationCheck.add(valuesSourceType); + } + public ValuesSourceRegistry build() { - return new ValuesSourceRegistry(aggregatorRegistry, usageServiceBuilder.build()); + return new ValuesSourceRegistry(aggregatorRegistry, usageServiceBuilder.build(), valueTypeLookup); } } @@ -151,13 +167,16 @@ public ValuesSourceRegistry build() { /** Maps Aggregation names to (ValuesSourceType, Supplier) pairs, keyed by ValuesSourceType */ private final AggregationUsageService usageService; private Map, Map> aggregatorRegistry; + private Map valueTypeLookup; public ValuesSourceRegistry( Map, List>> aggregatorRegistry, - AggregationUsageService usageService - ) { + AggregationUsageService usageService, + Map valueTypeLookup) { this.aggregatorRegistry = copyMap(aggregatorRegistry); this.usageService = usageService; + // TODO: Make an immutable copy blah blah blah + this.valueTypeLookup = valueTypeLookup; } public boolean isRegistered(RegistryKey registryKey) { @@ -182,8 +201,7 @@ public AggregationUsageService getUsageService() { return usageService; } - public ValueType resolveTypeHint(String typeHint) { - // NOCOMMIT: do the actual lookup thing here - return typeHint != null ? ValueType.lenientParse(typeHint) : null; + public ValuesSourceType resolveTypeHint(String typeHint) { + return typeHint != null ? ValueType.lenientParse(typeHint).getValuesSourceType() : null; } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceType.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceType.java index ed7e786b10b34..ad9b20b6bce06 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceType.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceType.java @@ -46,10 +46,9 @@ public interface ValuesSourceType { * operating without an underlying field. Scripts operating over fields are handled by the script argument to getField below. * * @param script - The script being wrapped - * @param scriptValueType - The expected output type of the script * @return - Script specialization of the base {@link ValuesSource} */ - ValuesSource getScript(AggregationScript.LeafFactory script, ValueType scriptValueType); + ValuesSource getScript(AggregationScript.LeafFactory script); /** * Return a {@link ValuesSource} wrapping a field for the given type. All {@link ValuesSource}s must implement this method. diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/support/ValuesSourceConfigTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/support/ValuesSourceConfigTests.java index 027c0a9a3cc26..0d9a17731f56c 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/support/ValuesSourceConfigTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/support/ValuesSourceConfigTests.java @@ -58,13 +58,31 @@ public void testUnmappedKeyword() throws Exception { MapperService mapperService = createMapperService(mapping(b -> {})); withAggregationContext(mapperService, List.of(source(b -> {})), context -> { ValuesSourceConfig config; - config = ValuesSourceConfig.resolve(context, ValueType.STRING, "field", null, null, null, null, CoreValuesSourceType.KEYWORD); + config = ValuesSourceConfig.resolve( + context, + CoreValuesSourceType.KEYWORD, + "field", + null, + null, + null, + null, + CoreValuesSourceType.KEYWORD + ); ValuesSource.Bytes valuesSource = (ValuesSource.Bytes) config.getValuesSource(); assertNotNull(valuesSource); assertFalse(config.hasValues()); assertFalse(config.alignesWithSearchIndex()); - config = ValuesSourceConfig.resolve(context, ValueType.STRING, "field", null, "abc", null, null, CoreValuesSourceType.KEYWORD); + config = ValuesSourceConfig.resolve( + context, + CoreValuesSourceType.KEYWORD, + "field", + null, + "abc", + null, + null, + CoreValuesSourceType.KEYWORD + ); valuesSource = (ValuesSource.Bytes) config.getValuesSource(); LeafReaderContext ctx = context.searcher().getIndexReader().leaves().get(0); SortedBinaryDocValues values = valuesSource.bytesValues(ctx); @@ -115,13 +133,31 @@ public void testUnmappedLong() throws Exception { MapperService mapperService = createMapperService(mapping(b -> {})); withAggregationContext(mapperService, List.of(source(b -> {})), context -> { ValuesSourceConfig config; - config = ValuesSourceConfig.resolve(context, ValueType.NUMBER, "field", null, null, null, null, CoreValuesSourceType.KEYWORD); + config = ValuesSourceConfig.resolve( + context, + CoreValuesSourceType.LONG, + "field", + null, + null, + null, + null, + CoreValuesSourceType.KEYWORD + ); ValuesSource.Numeric valuesSource = (ValuesSource.Numeric) config.getValuesSource(); assertNotNull(valuesSource); assertFalse(config.hasValues()); assertFalse(config.alignesWithSearchIndex()); - config = ValuesSourceConfig.resolve(context, ValueType.NUMBER, "field", null, 42, null, null, CoreValuesSourceType.KEYWORD); + config = ValuesSourceConfig.resolve( + context, + CoreValuesSourceType.LONG, + "field", + null, + 42, + null, + null, + CoreValuesSourceType.KEYWORD + ); valuesSource = (ValuesSource.Numeric) config.getValuesSource(); LeafReaderContext ctx = context.searcher().getIndexReader().leaves().get(0); SortedNumericDocValues values = valuesSource.longValues(ctx); @@ -172,13 +208,31 @@ public void testUnmappedBoolean() throws Exception { MapperService mapperService = createMapperService(mapping(b -> {})); withAggregationContext(mapperService, List.of(source(b -> {})), context -> { ValuesSourceConfig config; - config = ValuesSourceConfig.resolve(context, ValueType.BOOLEAN, "field", null, null, null, null, CoreValuesSourceType.KEYWORD); + config = ValuesSourceConfig.resolve( + context, + CoreValuesSourceType.BOOLEAN, + "field", + null, + null, + null, + null, + CoreValuesSourceType.KEYWORD + ); ValuesSource.Numeric valuesSource = (ValuesSource.Numeric) config.getValuesSource(); assertNotNull(valuesSource); assertFalse(config.hasValues()); assertFalse(config.alignesWithSearchIndex()); - config = ValuesSourceConfig.resolve(context, ValueType.BOOLEAN, "field", null, true, null, null, CoreValuesSourceType.KEYWORD); + config = ValuesSourceConfig.resolve( + context, + CoreValuesSourceType.BOOLEAN, + "field", + null, + true, + null, + null, + CoreValuesSourceType.KEYWORD + ); valuesSource = (ValuesSource.Numeric) config.getValuesSource(); LeafReaderContext ctx = context.searcher().getIndexReader().leaves().get(0); SortedNumericDocValues values = valuesSource.longValues(ctx); @@ -196,7 +250,16 @@ public void testFieldAlias() throws Exception { })); withAggregationContext(mapperService, List.of(source(b -> b.field("field", "value"))), context -> { ValuesSourceConfig config; - config = ValuesSourceConfig.resolve(context, ValueType.STRING, "alias", null, null, null, null, CoreValuesSourceType.KEYWORD); + config = ValuesSourceConfig.resolve( + context, + CoreValuesSourceType.KEYWORD, + "alias", + null, + null, + null, + null, + CoreValuesSourceType.KEYWORD + ); ValuesSource.Bytes valuesSource = (ValuesSource.Bytes) config.getValuesSource(); LeafReaderContext ctx = context.searcher().getIndexReader().leaves().get(0); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistryTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistryTests.java index 5bbee314d5f22..41fdb26e8dee4 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistryTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistryTests.java @@ -52,7 +52,7 @@ public void testAggregatorNotFoundException() { "bogus", HistogramAggregatorSupplier.class ); - ValuesSourceRegistry registry = new ValuesSourceRegistry(Map.of(key, List.of()), null); + ValuesSourceRegistry registry = new ValuesSourceRegistry(Map.of(key, List.of()), null, ); expectThrows(IllegalArgumentException.class, () -> registry.getAggregator(key, fieldOnly)); expectThrows(IllegalArgumentException.class, () -> registry.getAggregator(key, scriptOnly)); } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/support/AnalyticsValuesSourceType.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/support/AnalyticsValuesSourceType.java index aebb2698b0870..b3eb870551f58 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/support/AnalyticsValuesSourceType.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/aggregations/support/AnalyticsValuesSourceType.java @@ -13,7 +13,6 @@ import org.elasticsearch.search.aggregations.AggregationExecutionException; import org.elasticsearch.search.aggregations.support.AggregationContext; import org.elasticsearch.search.aggregations.support.FieldContext; -import org.elasticsearch.search.aggregations.support.ValueType; import org.elasticsearch.search.aggregations.support.ValuesSource; import org.elasticsearch.search.aggregations.support.ValuesSourceType; @@ -28,7 +27,7 @@ public ValuesSource getEmpty() { } @Override - public ValuesSource getScript(AggregationScript.LeafFactory script, ValueType scriptValueType) { + public ValuesSource getScript(AggregationScript.LeafFactory script) { throw new AggregationExecutionException("value source of type [" + this.value() + "] is not supported by scripts"); } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/rate/RateAggregationBuilder.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/rate/RateAggregationBuilder.java index 3d4dc776915b2..4f0a11579f3c3 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/rate/RateAggregationBuilder.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/rate/RateAggregationBuilder.java @@ -191,7 +191,7 @@ static Rounding.DateTimeUnit parse(String rateUnit) { @Override protected ValuesSourceConfig resolveConfig(AggregationContext context) { if (field() == null && script() == null) { - return new ValuesSourceConfig(CoreValuesSourceType.DOUBLE, null, true, null, null, 1.0, null, DocValueFormat.RAW, context); + return new ValuesSourceConfig(CoreValuesSourceType.DOUBLE, null, true, null, 1.0, null, DocValueFormat.RAW, context); } else { return super.resolveConfig(context); } diff --git a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/aggregations/support/AggregateMetricsValuesSourceType.java b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/aggregations/support/AggregateMetricsValuesSourceType.java index dd3f6d694664b..ddf759a31fab7 100644 --- a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/aggregations/support/AggregateMetricsValuesSourceType.java +++ b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/aggregations/support/AggregateMetricsValuesSourceType.java @@ -12,7 +12,6 @@ import org.elasticsearch.search.aggregations.AggregationExecutionException; import org.elasticsearch.search.aggregations.support.AggregationContext; import org.elasticsearch.search.aggregations.support.FieldContext; -import org.elasticsearch.search.aggregations.support.ValueType; import org.elasticsearch.search.aggregations.support.ValuesSource; import org.elasticsearch.search.aggregations.support.ValuesSourceType; import org.elasticsearch.xpack.aggregatemetric.fielddata.IndexAggregateDoubleMetricFieldData; @@ -28,7 +27,7 @@ public ValuesSource getEmpty() { } @Override - public ValuesSource getScript(AggregationScript.LeafFactory script, ValueType scriptValueType) { + public ValuesSource getScript(AggregationScript.LeafFactory script) { throw new AggregationExecutionException("Value source of type [" + this.value() + "] is not supported by scripts"); } diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/support/GeoShapeValuesSourceType.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/support/GeoShapeValuesSourceType.java index 139f462575349..648bc860852a4 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/support/GeoShapeValuesSourceType.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/support/GeoShapeValuesSourceType.java @@ -18,7 +18,6 @@ import org.elasticsearch.search.aggregations.support.AggregationContext; import org.elasticsearch.search.aggregations.support.FieldContext; import org.elasticsearch.search.aggregations.support.MissingValues; -import org.elasticsearch.search.aggregations.support.ValueType; import org.elasticsearch.search.aggregations.support.ValuesSource; import org.elasticsearch.search.aggregations.support.ValuesSourceType; import org.elasticsearch.xpack.spatial.index.fielddata.GeoShapeValues; @@ -40,7 +39,7 @@ public ValuesSource getEmpty() { } @Override - public ValuesSource getScript(AggregationScript.LeafFactory script, ValueType scriptValueType) { + public ValuesSource getScript(AggregationScript.LeafFactory script) { // TODO (support scripts) throw new UnsupportedOperationException("geo_shape"); } diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/SpatialPluginTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/SpatialPluginTests.java index 2ea9a4205ba5a..57f1d89de524d 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/SpatialPluginTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/SpatialPluginTests.java @@ -38,7 +38,7 @@ public void testGeoCentroidLicenseCheck() { ValuesSourceRegistry registry = registryBuilder.build(); MetricAggregatorSupplier centroidSupplier = registry.getAggregator( GeoCentroidAggregationBuilder.REGISTRY_KEY, - new ValuesSourceConfig(GeoShapeValuesSourceType.instance(), null, true, null, null, null, null, null, null) + new ValuesSourceConfig(GeoShapeValuesSourceType.instance(), null, true, null, null, null, null, null) ); if (License.OperationMode.TRIAL != operationMode && License.OperationMode.compare(operationMode, License.OperationMode.GOLD) < 0) { @@ -67,7 +67,7 @@ public void testGeoGridLicenseCheck() { ValuesSourceRegistry registry = registryBuilder.build(); GeoGridAggregatorSupplier supplier = registry.getAggregator( registryKey, - new ValuesSourceConfig(GeoShapeValuesSourceType.instance(), null, true, null, null, null, null, null, null) + new ValuesSourceConfig(GeoShapeValuesSourceType.instance(), null, true, null, null, null, null, null) ); if (License.OperationMode.TRIAL != operationMode && License.OperationMode.compare(operationMode, License.OperationMode.GOLD) < 0) { From 669381fba3830b28c1cf0785f728b832aaf27454 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Thu, 18 Nov 2021 12:25:34 -0500 Subject: [PATCH 05/19] Register Values Sources --- .../elasticsearch/plugins/SearchPlugin.java | 8 +++++++ .../elasticsearch/search/SearchModule.java | 9 ++++++++ .../support/CoreValuesSourceType.java | 21 +++++++++++++++++++ .../support/ValuesSourceRegistry.java | 13 ++++++++---- .../support/ValuesSourceType.java | 17 ++++++++++++++- .../support/ValuesSourceRegistryTests.java | 2 +- .../xpack/analytics/AnalyticsPlugin.java | 7 +++++++ .../AggregateMetricMapperPlugin.java | 7 +++++++ .../xpack/spatial/SpatialPlugin.java | 6 ++++++ 9 files changed, 84 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/plugins/SearchPlugin.java b/server/src/main/java/org/elasticsearch/plugins/SearchPlugin.java index 6067768715aa1..26bdc472175cb 100644 --- a/server/src/main/java/org/elasticsearch/plugins/SearchPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/SearchPlugin.java @@ -31,6 +31,7 @@ import org.elasticsearch.search.aggregations.bucket.terms.heuristic.SignificanceHeuristic; import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator; import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; +import org.elasticsearch.search.aggregations.support.ValuesSourceType; import org.elasticsearch.search.fetch.FetchSubPhase; import org.elasticsearch.search.fetch.subphase.highlight.Highlighter; import org.elasticsearch.search.internal.ShardSearchRequest; @@ -115,6 +116,13 @@ default List getAggregations() { return emptyList(); } + /** + * The new {@link ValuesSourceType}s added by this plugin + */ + default List getValuesSourceTypes() { + return emptyList(); + } + /** * Allows plugins to register new aggregations using aggregation names that are already defined * in Core, as long as the new aggregations target different ValuesSourceTypes diff --git a/server/src/main/java/org/elasticsearch/search/SearchModule.java b/server/src/main/java/org/elasticsearch/search/SearchModule.java index f7858e536edf9..b335eeb9a50ef 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/server/src/main/java/org/elasticsearch/search/SearchModule.java @@ -207,7 +207,9 @@ import org.elasticsearch.search.aggregations.pipeline.SerialDiffPipelineAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.StatsBucketPipelineAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.SumBucketPipelineAggregationBuilder; +import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; +import org.elasticsearch.search.aggregations.support.ValuesSourceType; import org.elasticsearch.search.fetch.FetchPhase; import org.elasticsearch.search.fetch.FetchSubPhase; import org.elasticsearch.search.fetch.subphase.ExplainPhase; @@ -343,6 +345,10 @@ public Map getHighlighters() { private ValuesSourceRegistry registerAggregations(List plugins) { ValuesSourceRegistry.Builder builder = new ValuesSourceRegistry.Builder(); + for (ValuesSourceType coreVST : CoreValuesSourceType.values()) { + + } + registerAggregation( new AggregationSpec(AvgAggregationBuilder.NAME, AvgAggregationBuilder::new, AvgAggregationBuilder.PARSER).addResultReader( InternalAvg::new @@ -645,6 +651,9 @@ private ValuesSourceRegistry registerAggregations(List plugins) { ); } + // Before registering plugin aggregations, register plugin values sources + registerFromPlugin(plugins, SearchPlugin::getValuesSourceTypes, builder::registerValuesSourceType); + registerFromPlugin(plugins, SearchPlugin::getAggregations, (agg) -> this.registerAggregation(agg, builder)); // after aggs have been registered, see if there are any new VSTypes that need to be linked to core fields diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/CoreValuesSourceType.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/CoreValuesSourceType.java index 463548bde59ed..25d8478ccb092 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/CoreValuesSourceType.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/CoreValuesSourceType.java @@ -35,6 +35,7 @@ import java.time.ZoneId; import java.time.ZoneOffset; import java.util.Arrays; +import java.util.Collection; import java.util.List; import java.util.Locale; import java.util.function.Function; @@ -103,6 +104,11 @@ public DocValueFormat getFormatter(String format, ZoneId tz) { } } + + @Override + public Collection alternateNames() { + return List.of("float", "numeric", "number"); + } }, LONG() { @Override @@ -163,6 +169,11 @@ public DocValueFormat getFormatter(String format, ZoneId tz) { } } + + @Override + public Collection alternateNames() { + return List.of("byte", "short", "integer"); + } }, KEYWORD() { @Override @@ -205,6 +216,11 @@ public ValuesSource replaceMissing( return MissingValues.replaceMissing((ValuesSource.Bytes) valuesSource, missing); } } + + @Override + public Collection alternateNames() { + return List.of("string"); + } }, GEOPOINT() { @Override @@ -244,6 +260,11 @@ public ValuesSource replaceMissing( public DocValueFormat getFormatter(String format, ZoneId tz) { return DocValueFormat.GEOHASH; } + + @Override + public Collection alternateNames() { + return List.of("geo_point"); + } }, RANGE() { @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java index 6b463e432ee35..59d2c892deb68 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java @@ -13,7 +13,6 @@ import java.util.AbstractMap; import java.util.ArrayList; -import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -125,12 +124,18 @@ public void registerUsage(String aggregationName) { usageServiceBuilder.registerAggregationUsage(aggregationName); } - public void registerValuesSourceType(ValuesSourceType valuesSourceType, Collection alternateNames) { + public void registerValuesSourceType(ValuesSourceType valuesSourceType) { if (duplicateRegistrationCheck.contains(valuesSourceType)) { throw new IllegalStateException("Duplicate registration of ValuesSourceType [" + valuesSourceType.typeName() + "]"); } + if (valueTypeLookup.containsKey(valuesSourceType.typeName())) { + throw new IllegalStateException("Duplicate type name detected: [" + valuesSourceType.typeName() + "]"); + } valueTypeLookup.put(valuesSourceType.typeName(), valuesSourceType); - for (String name : alternateNames) { + for (String name : valuesSourceType.alternateNames()) { + if (valueTypeLookup.containsKey(name)) { + throw new IllegalStateException("Duplicate type name detected: [" + name + "]"); + } valueTypeLookup.put(name, valuesSourceType); } duplicateRegistrationCheck.add(valuesSourceType); @@ -202,6 +207,6 @@ public AggregationUsageService getUsageService() { } public ValuesSourceType resolveTypeHint(String typeHint) { - return typeHint != null ? ValueType.lenientParse(typeHint).getValuesSourceType() : null; + return typeHint != null ? valueTypeLookup.get(typeHint) : null; } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceType.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceType.java index ad9b20b6bce06..0ec6538fa0cd9 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceType.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceType.java @@ -12,6 +12,8 @@ import org.elasticsearch.search.DocValueFormat; import java.time.ZoneId; +import java.util.Collection; +import java.util.List; /** * {@link ValuesSourceType} represents a collection of fields that share a common set of operations, for example all numeric fields. @@ -87,8 +89,21 @@ default DocValueFormat getFormatter(String format, ZoneId tz) { } /** - * Returns the name of the Values Source Type for stats purposes + * Returns the name of the Values Source Type for stats purposes. This name can also be used for a type hint via the + * {@link ValueType#VALUE_TYPE} field when specifying an aggregation. This can help resolve issues with unmapped fields and similar. + * + * The returned value should be unique. + * * @return the name of the Values Source Type */ String typeName(); + + /** + * Optionally, specify a list of alternative names that can be used in the {@link ValueType#VALUE_TYPE} field to specify this type. + * Generally, new types shouldn't need to define this, but we have some legacy mappings that need to be supported still. + * @return a collection of unique strings that can be used to identify this type + */ + default Collection alternateNames() { + return List.of(); + } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistryTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistryTests.java index 41fdb26e8dee4..b9350aaa60109 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistryTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistryTests.java @@ -52,7 +52,7 @@ public void testAggregatorNotFoundException() { "bogus", HistogramAggregatorSupplier.class ); - ValuesSourceRegistry registry = new ValuesSourceRegistry(Map.of(key, List.of()), null, ); + ValuesSourceRegistry registry = new ValuesSourceRegistry(Map.of(key, List.of()), null, null); expectThrows(IllegalArgumentException.class, () -> registry.getAggregator(key, fieldOnly)); expectThrows(IllegalArgumentException.class, () -> registry.getAggregator(key, scriptOnly)); } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsPlugin.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsPlugin.java index 1382ac60a2dcf..25f4f36083f73 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsPlugin.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/AnalyticsPlugin.java @@ -23,6 +23,7 @@ import org.elasticsearch.repositories.RepositoriesService; import org.elasticsearch.script.ScriptService; import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; +import org.elasticsearch.search.aggregations.support.ValuesSourceType; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.watcher.ResourceWatcherService; import org.elasticsearch.xcontent.NamedXContentRegistry; @@ -30,6 +31,7 @@ import org.elasticsearch.xpack.analytics.action.AnalyticsUsageTransportAction; import org.elasticsearch.xpack.analytics.action.TransportAnalyticsStatsAction; import org.elasticsearch.xpack.analytics.aggregations.AnalyticsAggregatorFactory; +import org.elasticsearch.xpack.analytics.aggregations.support.AnalyticsValuesSourceType; import org.elasticsearch.xpack.analytics.boxplot.BoxplotAggregationBuilder; import org.elasticsearch.xpack.analytics.boxplot.InternalBoxplot; import org.elasticsearch.xpack.analytics.cumulativecardinality.CumulativeCardinalityPipelineAggregationBuilder; @@ -98,6 +100,11 @@ public List getPipelineAggregations() { return pipelineAggs; } + @Override + public List getValuesSourceTypes() { + return Arrays.asList(AnalyticsValuesSourceType.values()); + } + @Override public List getAggregations() { return Arrays.asList( diff --git a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/AggregateMetricMapperPlugin.java b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/AggregateMetricMapperPlugin.java index e44b65ee2212b..0d3f1346cee1a 100644 --- a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/AggregateMetricMapperPlugin.java +++ b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/AggregateMetricMapperPlugin.java @@ -15,7 +15,9 @@ import org.elasticsearch.plugins.Plugin; import org.elasticsearch.plugins.SearchPlugin; import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; +import org.elasticsearch.search.aggregations.support.ValuesSourceType; import org.elasticsearch.xpack.aggregatemetric.aggregations.metrics.AggregateMetricsAggregatorsRegistrar; +import org.elasticsearch.xpack.aggregatemetric.aggregations.support.AggregateMetricsValuesSourceType; import org.elasticsearch.xpack.aggregatemetric.mapper.AggregateDoubleMetricFieldMapper; import org.elasticsearch.xpack.core.action.XPackInfoFeatureAction; import org.elasticsearch.xpack.core.action.XPackUsageFeatureAction; @@ -42,6 +44,11 @@ public Map getMappers() { ); } + @Override + public List getValuesSourceTypes() { + return Arrays.asList(AggregateMetricsValuesSourceType.values()); + } + @Override public List> getAggregationExtentions() { return List.of( diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java index 064e43e2b9e90..307b768a186db 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java @@ -32,6 +32,7 @@ import org.elasticsearch.search.aggregations.metrics.ValueCountAggregationBuilder; import org.elasticsearch.search.aggregations.metrics.ValueCountAggregator; import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; +import org.elasticsearch.search.aggregations.support.ValuesSourceType; import org.elasticsearch.xcontent.ContextParser; import org.elasticsearch.xpack.core.XPackPlugin; import org.elasticsearch.xpack.core.action.XPackInfoFeatureAction; @@ -121,6 +122,11 @@ public List> getQueries() { return singletonList(new QuerySpec<>(ShapeQueryBuilder.NAME, ShapeQueryBuilder::new, ShapeQueryBuilder::fromXContent)); } + @Override + public List getValuesSourceTypes() { + return List.of(GeoShapeValuesSourceType.instance()); + } + @Override public List> getAggregationExtentions() { return List.of( From 188cad9d59c47f1c1d69443cf1bb6385cc13abfa Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Thu, 18 Nov 2021 12:33:18 -0500 Subject: [PATCH 06/19] Change remaining nocommits todos so tests will run --- .../search/aggregations/matrix/ArrayValuesSourceParser.java | 2 +- .../analytics/multiterms/MultiTermsAggregationBuilder.java | 5 ++--- .../topmetrics/TopMetricsAggregatorMetricsTests.java | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/ArrayValuesSourceParser.java b/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/ArrayValuesSourceParser.java index c19f8a5641eff..c4ca9b2663918 100644 --- a/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/ArrayValuesSourceParser.java +++ b/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/ArrayValuesSourceParser.java @@ -31,7 +31,7 @@ public abstract class ArrayValuesSourceParser implement public abstract static class NumericValuesSourceParser extends ArrayValuesSourceParser { - // NOCOMMIT: Long Support? IDK what we should do here. + // TODO: Long Support? IDK what we should do here. protected NumericValuesSourceParser(boolean formattable) { super(formattable, CoreValuesSourceType.DOUBLE, ValueType.NUMERIC); } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java index 5f7ff90dc5b2a..23b2da6b7d96f 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java @@ -112,16 +112,15 @@ interface MultiTermValuesSupplier { public static void registerAggregators(ValuesSourceRegistry.Builder registry) { registry.registerUsage(NAME); - // NOCOMMIT: Should Longs use the LongTermsValuesSource? This mirrors the numeric behavior, but might be wrong. registry.register( REGISTRY_KEY, - List.of(CoreValuesSourceType.DOUBLE, CoreValuesSourceType.LONG), + List.of(CoreValuesSourceType.DOUBLE), MultiTermsAggregator::buildNumericTermValues, false ); registry.register( REGISTRY_KEY, - List.of(CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.DATE), + List.of(CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.LONG, CoreValuesSourceType.DATE), MultiTermsAggregator.LongTermValuesSource::new, false ); diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregatorMetricsTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregatorMetricsTests.java index edd8c99e7ee73..1c49f2d1a88ee 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregatorMetricsTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregatorMetricsTests.java @@ -204,7 +204,7 @@ private ValuesSourceConfig toConfig(SortedNumericDocValues values) throws IOExce when(source.isFloatingPoint()).thenReturn(false); when(source.longValues(null)).thenReturn(values); if (randomBoolean()) { - // NOCOMMIT: should we have a case for CVST.LONG here? + // TODO: should we have a case for CVST.LONG here? return toConfig(source, CoreValuesSourceType.DOUBLE, randomWholeNumberDocValuesFormat(), true); } DocValueFormat dateFormatter = new DocValueFormat.DateTime( From c3d4bb6321f49b4196741a782f2993813b1ed714 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Thu, 18 Nov 2021 12:38:59 -0500 Subject: [PATCH 07/19] spotless apply --- .../search/aggregations/support/ValuesSourceRegistry.java | 3 ++- .../analytics/multiterms/MultiTermsAggregationBuilder.java | 7 +------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java index 59d2c892deb68..bcfb33a46d6ee 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java @@ -177,7 +177,8 @@ public ValuesSourceRegistry build() { public ValuesSourceRegistry( Map, List>> aggregatorRegistry, AggregationUsageService usageService, - Map valueTypeLookup) { + Map valueTypeLookup + ) { this.aggregatorRegistry = copyMap(aggregatorRegistry); this.usageService = usageService; // TODO: Make an immutable copy blah blah blah diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java index 23b2da6b7d96f..e9f1a3d9553ad 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/multiterms/MultiTermsAggregationBuilder.java @@ -112,12 +112,7 @@ interface MultiTermValuesSupplier { public static void registerAggregators(ValuesSourceRegistry.Builder registry) { registry.registerUsage(NAME); - registry.register( - REGISTRY_KEY, - List.of(CoreValuesSourceType.DOUBLE), - MultiTermsAggregator::buildNumericTermValues, - false - ); + registry.register(REGISTRY_KEY, List.of(CoreValuesSourceType.DOUBLE), MultiTermsAggregator::buildNumericTermValues, false); registry.register( REGISTRY_KEY, List.of(CoreValuesSourceType.BOOLEAN, CoreValuesSourceType.LONG, CoreValuesSourceType.DATE), From fb42a7caa6e05b53e2c9efdc6bc94ba56491a9ca Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Thu, 18 Nov 2021 12:51:46 -0500 Subject: [PATCH 08/19] Fix MatrixStats when hint is null --- .../matrix/ArrayValuesSourceAggregationBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/ArrayValuesSourceAggregationBuilder.java b/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/ArrayValuesSourceAggregationBuilder.java index c6d673138478b..288a07aed09a2 100644 --- a/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/ArrayValuesSourceAggregationBuilder.java +++ b/modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/ArrayValuesSourceAggregationBuilder.java @@ -201,7 +201,7 @@ protected Map resolveConfig(AggregationContext conte for (String field : fields) { ValuesSourceConfig config = ValuesSourceConfig.resolveUnregistered( context, - userValueTypeHint.getValuesSourceType(), + userValueTypeHint == null ? null : userValueTypeHint.getValuesSourceType(), field, null, missingMap.get(field), From 33bf6ff480adec4e07065caf06d7a525806d90c9 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Thu, 18 Nov 2021 14:03:29 -0500 Subject: [PATCH 09/19] actually register the core types --- .../src/main/java/org/elasticsearch/search/SearchModule.java | 2 +- .../search/aggregations/support/ValuesSourceRegistry.java | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/search/SearchModule.java b/server/src/main/java/org/elasticsearch/search/SearchModule.java index b335eeb9a50ef..23d10a9695058 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/server/src/main/java/org/elasticsearch/search/SearchModule.java @@ -346,7 +346,7 @@ private ValuesSourceRegistry registerAggregations(List plugins) { ValuesSourceRegistry.Builder builder = new ValuesSourceRegistry.Builder(); for (ValuesSourceType coreVST : CoreValuesSourceType.values()) { - + builder.registerValuesSourceType(coreVST); } registerAggregation( diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java index bcfb33a46d6ee..23c590f4631eb 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java @@ -85,6 +85,9 @@ public void register( T aggregatorSupplier, boolean registerUsage ) { + if (duplicateRegistrationCheck.contains(valuesSourceType) == false) { + throw new IllegalStateException("Attempt to register aggregation mapping before registering ValuesSourceType"); + } if (aggregatorRegistry.containsKey(registryKey) == false) { aggregatorRegistry.put(registryKey, new ArrayList<>()); } From d9a991febd2054bfa161305a3182c6076aca865a Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Thu, 18 Nov 2021 14:22:30 -0500 Subject: [PATCH 10/19] Register values sources for top metrics tests --- .../search/aggregations/support/ValuesSourceRegistry.java | 8 +++++++- .../topmetrics/TopMetricsAggregatorMetricsTests.java | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java index 23c590f4631eb..4500059eac5a0 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java @@ -86,7 +86,13 @@ public void register( boolean registerUsage ) { if (duplicateRegistrationCheck.contains(valuesSourceType) == false) { - throw new IllegalStateException("Attempt to register aggregation mapping before registering ValuesSourceType"); + throw new IllegalStateException( + "Attempt to register aggregation [" + + aggregatorSupplier.toString() + + "] before registering ValuesSourceType [" + + valuesSourceType.typeName() + + "]" + ); } if (aggregatorRegistry.containsKey(registryKey) == false) { aggregatorRegistry.put(registryKey, new ArrayList<>()); diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregatorMetricsTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregatorMetricsTests.java index 1c49f2d1a88ee..88fc041a18da7 100644 --- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregatorMetricsTests.java +++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregatorMetricsTests.java @@ -51,6 +51,9 @@ public class TopMetricsAggregatorMetricsTests extends ESTestCase { private static final ValuesSourceRegistry REGISTRY; static { ValuesSourceRegistry.Builder registry = new ValuesSourceRegistry.Builder(); + for (ValuesSourceType vst : CoreValuesSourceType.values()) { + registry.registerValuesSourceType(vst); + } TopMetricsAggregationBuilder.registerAggregators(registry); REGISTRY = registry.build(); } From c26b1a871ad48fc1484113ea079498bec0a4afa5 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Thu, 18 Nov 2021 14:31:06 -0500 Subject: [PATCH 11/19] More tests that need to register stuff --- .../search/aggregations/AdaptingAggregatorTests.java | 5 +++++ .../support/MultiValuesSourceFieldConfigTests.java | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/AdaptingAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/AdaptingAggregatorTests.java index da74f2d4e0b22..82813a5e5785a 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/AdaptingAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/AdaptingAggregatorTests.java @@ -15,7 +15,9 @@ import org.elasticsearch.search.aggregations.bucket.histogram.SizedBucketAggregator; import org.elasticsearch.search.aggregations.metrics.MaxAggregationBuilder; import org.elasticsearch.search.aggregations.support.AggregationContext; +import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; +import org.elasticsearch.search.aggregations.support.ValuesSourceType; import java.io.IOException; import java.util.List; @@ -35,6 +37,9 @@ public class AdaptingAggregatorTests extends MapperServiceTestCase { public void testParent() throws IOException { MapperService mapperService = createMapperService(mapping(b -> {})); ValuesSourceRegistry.Builder registry = new ValuesSourceRegistry.Builder(); + for (ValuesSourceType vst : CoreValuesSourceType.values()) { + registry.registerValuesSourceType(vst); + } MaxAggregationBuilder.registerAggregators(registry); withAggregationContext(registry.build(), mapperService, List.of(), null, context -> { AggregatorFactories.Builder sub = AggregatorFactories.builder(); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfigTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfigTests.java index 78fd58f2fc7ae..72bab65667b3c 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfigTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfigTests.java @@ -49,7 +49,7 @@ protected MultiValuesSourceFieldConfig createTestInstance() { .setTimeZone(timeZone) .setFilter(filter) .setFormat(format) - .setUserValueTypeHint(userValueTypeHint.getPreferredName()) + .setUserValueTypeHint(userValueTypeHint == null ? null : userValueTypeHint.getPreferredName()) .build(); } From 5811cff3d98b65fb6119a6d9161cde33ec88b7ff Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Thu, 18 Nov 2021 15:14:20 -0500 Subject: [PATCH 12/19] Clean up GeoShapeValuesSource --- .../xpack/spatial/SpatialPlugin.java | 14 +- .../index/fielddata/GeoShapeValues.java | 2 +- .../LatLonShapeDVAtomicShapeFieldData.java | 2 +- .../GeoShapeWithDocValuesFieldMapper.java | 2 +- .../bucket/geogrid/GeoShapeCellIdSource.java | 2 +- .../support/GeoShapeValuesSourceType.java | 168 ++++++++---------- .../xpack/spatial/SpatialPluginTests.java | 4 +- .../bucket/geogrid/GeoGridTilerTestCase.java | 2 +- .../geogrid/GeoShapeGeoGridTestCase.java | 2 +- .../GeoShapeBoundsAggregatorTests.java | 2 +- .../GeoShapeCentroidAggregatorTests.java | 2 +- 11 files changed, 95 insertions(+), 107 deletions(-) diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java index 307b768a186db..aea8e87dd217d 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java @@ -124,7 +124,7 @@ public List> getQueries() { @Override public List getValuesSourceTypes() { - return List.of(GeoShapeValuesSourceType.instance()); + return List.of(GeoShapeValuesSourceType.GEOSHAPE); } @Override @@ -157,7 +157,7 @@ public Map getProcessors(Processor.Parameters paramet private static void registerGeoShapeBoundsAggregator(ValuesSourceRegistry.Builder builder) { builder.register( GeoBoundsAggregationBuilder.REGISTRY_KEY, - GeoShapeValuesSourceType.instance(), + GeoShapeValuesSourceType.GEOSHAPE, GeoShapeBoundsAggregator::new, true ); @@ -166,7 +166,7 @@ private static void registerGeoShapeBoundsAggregator(ValuesSourceRegistry.Builde private void registerGeoShapeCentroidAggregator(ValuesSourceRegistry.Builder builder) { builder.register( GeoCentroidAggregationBuilder.REGISTRY_KEY, - GeoShapeValuesSourceType.instance(), + GeoShapeValuesSourceType.GEOSHAPE, (name, valuesSourceConfig, context, parent, metadata) -> { if (GEO_CENTROID_AGG_FEATURE.check(getLicenseState())) { return new GeoShapeCentroidAggregator(name, context, parent, valuesSourceConfig, metadata); @@ -180,7 +180,7 @@ private void registerGeoShapeCentroidAggregator(ValuesSourceRegistry.Builder bui private void registerGeoShapeGridAggregators(ValuesSourceRegistry.Builder builder) { builder.register( GeoHashGridAggregationBuilder.REGISTRY_KEY, - GeoShapeValuesSourceType.instance(), + GeoShapeValuesSourceType.GEOSHAPE, ( name, factories, @@ -223,7 +223,7 @@ private void registerGeoShapeGridAggregators(ValuesSourceRegistry.Builder builde builder.register( GeoTileGridAggregationBuilder.REGISTRY_KEY, - GeoShapeValuesSourceType.instance(), + GeoShapeValuesSourceType.GEOSHAPE, ( name, factories, @@ -266,11 +266,11 @@ private void registerGeoShapeGridAggregators(ValuesSourceRegistry.Builder builde } private static void registerValueCountAggregator(ValuesSourceRegistry.Builder builder) { - builder.register(ValueCountAggregationBuilder.REGISTRY_KEY, GeoShapeValuesSourceType.instance(), ValueCountAggregator::new, true); + builder.register(ValueCountAggregationBuilder.REGISTRY_KEY, GeoShapeValuesSourceType.GEOSHAPE, ValueCountAggregator::new, true); } private static void registerCardinalityAggregator(ValuesSourceRegistry.Builder builder) { - builder.register(CardinalityAggregationBuilder.REGISTRY_KEY, GeoShapeValuesSourceType.instance(), CardinalityAggregator::new, true); + builder.register(CardinalityAggregationBuilder.REGISTRY_KEY, GeoShapeValuesSourceType.GEOSHAPE, CardinalityAggregator::new, true); } private ContextParser checkLicense(ContextParser realParser, LicensedFeature.Momentary feature) { diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java index 781a062c4b065..2e783d38dc4bf 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/GeoShapeValues.java @@ -41,7 +41,7 @@ public abstract class GeoShapeValues { public static GeoShapeValues EMPTY = new GeoShapeValues() { - private final GeoShapeValuesSourceType DEFAULT_VALUES_SOURCE_TYPE = GeoShapeValuesSourceType.instance(); + private final GeoShapeValuesSourceType DEFAULT_VALUES_SOURCE_TYPE = GeoShapeValuesSourceType.GEOSHAPE; @Override public boolean advanceExact(int doc) { diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/LatLonShapeDVAtomicShapeFieldData.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/LatLonShapeDVAtomicShapeFieldData.java index b41918ffdcc51..7b3a418569973 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/LatLonShapeDVAtomicShapeFieldData.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/fielddata/plain/LatLonShapeDVAtomicShapeFieldData.java @@ -58,7 +58,7 @@ public boolean advanceExact(int doc) throws IOException { @Override public ValuesSourceType valuesSourceType() { - return GeoShapeValuesSourceType.instance(); + return GeoShapeValuesSourceType.GEOSHAPE; } @Override diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java index 86669cd871a20..2a9997b75fa48 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java @@ -173,7 +173,7 @@ public GeoShapeWithDocValuesFieldType( public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName, Supplier searchLookup) { failIfNoDocValues(); - return new AbstractLatLonShapeIndexFieldData.Builder(name(), GeoShapeValuesSourceType.instance()); + return new AbstractLatLonShapeIndexFieldData.Builder(name(), GeoShapeValuesSourceType.GEOSHAPE); } @Override diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoShapeCellIdSource.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoShapeCellIdSource.java index cfd1bce97ab24..acf3594c331b2 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoShapeCellIdSource.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoShapeCellIdSource.java @@ -48,7 +48,7 @@ public boolean isFloatingPoint() { public SortedNumericDocValues longValues(LeafReaderContext ctx) { GeoShapeValues geoValues = valuesSource.geoShapeValues(ctx); ValuesSourceType vs = geoValues.valuesSourceType(); - if (GeoShapeValuesSourceType.instance() == vs) { + if (GeoShapeValuesSourceType.GEOSHAPE == vs) { // docValues are geo shapes return new GeoShapeCellValues(geoValues, encoder, circuitBreakerConsumer); } else { diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/support/GeoShapeValuesSourceType.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/support/GeoShapeValuesSourceType.java index 648bc860852a4..57f6d0848edd0 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/support/GeoShapeValuesSourceType.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/support/GeoShapeValuesSourceType.java @@ -9,8 +9,6 @@ import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.util.BytesRef; -import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.index.fielddata.IndexGeoPointFieldData; import org.elasticsearch.index.fielddata.SortedBinaryDocValues; import org.elasticsearch.script.AggregationScript; @@ -25,100 +23,90 @@ import java.io.IOException; -public class GeoShapeValuesSourceType implements Writeable, ValuesSourceType { - - static GeoShapeValuesSourceType INSTANCE = new GeoShapeValuesSourceType(); - - public static GeoShapeValuesSourceType instance() { - return INSTANCE; - } - - @Override - public ValuesSource getEmpty() { - return GeoShapeValuesSource.EMPTY; - } - - @Override - public ValuesSource getScript(AggregationScript.LeafFactory script) { - // TODO (support scripts) - throw new UnsupportedOperationException("geo_shape"); - } - - @Override - public ValuesSource getField(FieldContext fieldContext, AggregationScript.LeafFactory script, AggregationContext context) { - boolean isGeoPoint = fieldContext.indexFieldData() instanceof IndexGeoPointFieldData; - boolean isGeoShape = fieldContext.indexFieldData() instanceof IndexGeoShapeFieldData; - if (isGeoPoint == false && isGeoShape == false) { - throw new IllegalArgumentException( - "Expected geo_point or geo_shape type on field [" - + fieldContext.field() - + "], but got [" - + fieldContext.fieldType().typeName() - + "]" - ); - } - if (isGeoPoint) { - return new ValuesSource.GeoPoint.Fielddata((IndexGeoPointFieldData) fieldContext.indexFieldData()); +public enum GeoShapeValuesSourceType implements ValuesSourceType { + GEOSHAPE { + @Override + public ValuesSource getEmpty() { + return GeoShapeValuesSource.EMPTY; } - return new GeoShapeValuesSource.Fielddata((IndexGeoShapeFieldData) fieldContext.indexFieldData()); - } - - @Override - public ValuesSource replaceMissing( - ValuesSource valuesSource, - Object rawMissing, - DocValueFormat docValueFormat, - AggregationContext context - ) { - GeoShapeValuesSource geoShapeValuesSource = (GeoShapeValuesSource) valuesSource; - final GeoShapeValues.GeoShapeValue missing = GeoShapeValues.GeoShapeValue.missing(rawMissing.toString()); - return new GeoShapeValuesSource() { - @Override - public GeoShapeValues geoShapeValues(LeafReaderContext context) { - GeoShapeValues values = geoShapeValuesSource.geoShapeValues(context); - return new GeoShapeValues() { - private boolean exists; - - @Override - public boolean advanceExact(int doc) throws IOException { - exists = values.advanceExact(doc); - // always return true because we want to return a value even if - // the document does not have a value - return true; - } - - @Override - public ValuesSourceType valuesSourceType() { - return values.valuesSourceType(); - } - - @Override - public GeoShapeValue value() throws IOException { - return exists ? values.value() : missing; - } + @Override + public ValuesSource getScript(AggregationScript.LeafFactory script) { + // TODO (support scripts) + throw new UnsupportedOperationException("geo_shape"); + } - @Override - public String toString() { - return "anon MultiGeoShapeValues of [" + super.toString() + "]"; - } - }; + @Override + public ValuesSource getField(FieldContext fieldContext, AggregationScript.LeafFactory script, AggregationContext context) { + boolean isGeoPoint = fieldContext.indexFieldData() instanceof IndexGeoPointFieldData; + boolean isGeoShape = fieldContext.indexFieldData() instanceof IndexGeoShapeFieldData; + if (isGeoPoint == false && isGeoShape == false) { + throw new IllegalArgumentException( + "Expected geo_point or geo_shape type on field [" + + fieldContext.field() + + "], but got [" + + fieldContext.fieldType().typeName() + + "]" + ); } - - @Override - public SortedBinaryDocValues bytesValues(LeafReaderContext context) throws IOException { - return MissingValues.replaceMissing(geoShapeValuesSource.bytesValues(context), new BytesRef(missing.toString())); + if (isGeoPoint) { + return new ValuesSource.GeoPoint.Fielddata((IndexGeoPointFieldData) fieldContext.indexFieldData()); } - }; - } - - @Override - public String typeName() { - return "geoshape"; - } + return new GeoShapeValuesSource.Fielddata((IndexGeoShapeFieldData) fieldContext.indexFieldData()); + } - @Override - public void writeTo(StreamOutput out) throws IOException { + @Override + public ValuesSource replaceMissing( + ValuesSource valuesSource, + Object rawMissing, + DocValueFormat docValueFormat, + AggregationContext context + ) { + GeoShapeValuesSource geoShapeValuesSource = (GeoShapeValuesSource) valuesSource; + final GeoShapeValues.GeoShapeValue missing = GeoShapeValues.GeoShapeValue.missing(rawMissing.toString()); + return new GeoShapeValuesSource() { + @Override + public GeoShapeValues geoShapeValues(LeafReaderContext context) { + GeoShapeValues values = geoShapeValuesSource.geoShapeValues(context); + return new GeoShapeValues() { + + private boolean exists; + + @Override + public boolean advanceExact(int doc) throws IOException { + exists = values.advanceExact(doc); + // always return true because we want to return a value even if + // the document does not have a value + return true; + } + + @Override + public ValuesSourceType valuesSourceType() { + return values.valuesSourceType(); + } + + @Override + public GeoShapeValue value() throws IOException { + return exists ? values.value() : missing; + } + + @Override + public String toString() { + return "anon MultiGeoShapeValues of [" + super.toString() + "]"; + } + }; + } + + @Override + public SortedBinaryDocValues bytesValues(LeafReaderContext context) throws IOException { + return MissingValues.replaceMissing(geoShapeValuesSource.bytesValues(context), new BytesRef(missing.toString())); + } + }; + } + @Override + public String typeName() { + return "geoshape"; + } } } diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/SpatialPluginTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/SpatialPluginTests.java index 57f1d89de524d..32fb74045616f 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/SpatialPluginTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/SpatialPluginTests.java @@ -38,7 +38,7 @@ public void testGeoCentroidLicenseCheck() { ValuesSourceRegistry registry = registryBuilder.build(); MetricAggregatorSupplier centroidSupplier = registry.getAggregator( GeoCentroidAggregationBuilder.REGISTRY_KEY, - new ValuesSourceConfig(GeoShapeValuesSourceType.instance(), null, true, null, null, null, null, null) + new ValuesSourceConfig(GeoShapeValuesSourceType.GEOSHAPE, null, true, null, null, null, null, null) ); if (License.OperationMode.TRIAL != operationMode && License.OperationMode.compare(operationMode, License.OperationMode.GOLD) < 0) { @@ -67,7 +67,7 @@ public void testGeoGridLicenseCheck() { ValuesSourceRegistry registry = registryBuilder.build(); GeoGridAggregatorSupplier supplier = registry.getAggregator( registryKey, - new ValuesSourceConfig(GeoShapeValuesSourceType.instance(), null, true, null, null, null, null, null) + new ValuesSourceConfig(GeoShapeValuesSourceType.GEOSHAPE, null, true, null, null, null, null, null) ); if (License.OperationMode.TRIAL != operationMode && License.OperationMode.compare(operationMode, License.OperationMode.GOLD) < 0) { diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoGridTilerTestCase.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoGridTilerTestCase.java index 15dd9383e5022..f47edcff8f064 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoGridTilerTestCase.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoGridTilerTestCase.java @@ -265,7 +265,7 @@ public boolean advanceExact(int doc) { @Override public ValuesSourceType valuesSourceType() { - return GeoShapeValuesSourceType.instance(); + return GeoShapeValuesSourceType.GEOSHAPE; } @Override diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoShapeGeoGridTestCase.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoShapeGeoGridTestCase.java index d448c2a9320c2..bbce0a0f1a343 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoShapeGeoGridTestCase.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/bucket/geogrid/GeoShapeGeoGridTestCase.java @@ -99,7 +99,7 @@ protected List getSearchPlugins() { @Override protected List getSupportedValuesSourceTypes() { - return List.of(GeoShapeValuesSourceType.instance(), CoreValuesSourceType.GEOPOINT); + return List.of(GeoShapeValuesSourceType.GEOSHAPE, CoreValuesSourceType.GEOPOINT); } @Override diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/metrics/GeoShapeBoundsAggregatorTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/metrics/GeoShapeBoundsAggregatorTests.java index afa37276b9b8b..616509fa36c43 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/metrics/GeoShapeBoundsAggregatorTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/metrics/GeoShapeBoundsAggregatorTests.java @@ -250,6 +250,6 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { - return List.of(CoreValuesSourceType.GEOPOINT, GeoShapeValuesSourceType.instance()); + return List.of(CoreValuesSourceType.GEOPOINT, GeoShapeValuesSourceType.GEOSHAPE); } } diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/metrics/GeoShapeCentroidAggregatorTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/metrics/GeoShapeCentroidAggregatorTests.java index a52cfd1881808..9738e5aa93a0d 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/metrics/GeoShapeCentroidAggregatorTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/metrics/GeoShapeCentroidAggregatorTests.java @@ -222,6 +222,6 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy @Override protected List getSupportedValuesSourceTypes() { - return List.of(CoreValuesSourceType.GEOPOINT, GeoShapeValuesSourceType.instance()); + return List.of(CoreValuesSourceType.GEOPOINT, GeoShapeValuesSourceType.GEOSHAPE); } } From d7ea0d5e228957aa36d26124696c61d930717e07 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Thu, 18 Nov 2021 15:35:49 -0500 Subject: [PATCH 13/19] some test fixes --- .../org/elasticsearch/xpack/spatial/SpatialPluginTests.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/SpatialPluginTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/SpatialPluginTests.java index 32fb74045616f..d263db3a1925c 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/SpatialPluginTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/SpatialPluginTests.java @@ -33,6 +33,7 @@ public void testGeoCentroidLicenseCheck() { for (License.OperationMode operationMode : License.OperationMode.values()) { SpatialPlugin plugin = getPluginWithOperationMode(operationMode); ValuesSourceRegistry.Builder registryBuilder = new ValuesSourceRegistry.Builder(); + registryBuilder.registerValuesSourceType(GeoShapeValuesSourceType.GEOSHAPE); List> registrar = plugin.getAggregationExtentions(); registrar.forEach(c -> c.accept(registryBuilder)); ValuesSourceRegistry registry = registryBuilder.build(); @@ -62,6 +63,7 @@ public void testGeoGridLicenseCheck() { for (License.OperationMode operationMode : License.OperationMode.values()) { SpatialPlugin plugin = getPluginWithOperationMode(operationMode); ValuesSourceRegistry.Builder registryBuilder = new ValuesSourceRegistry.Builder(); + registryBuilder.registerValuesSourceType(GeoShapeValuesSourceType.GEOSHAPE); List> registrar = plugin.getAggregationExtentions(); registrar.forEach(c -> c.accept(registryBuilder)); ValuesSourceRegistry registry = registryBuilder.build(); From 659e0aadccaa3c01091449088e61855759960acc Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Fri, 19 Nov 2021 09:15:11 -0500 Subject: [PATCH 14/19] fix usage tracking test --- .../java/org/elasticsearch/test/rest/NodeRestUsageIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/distribution/archives/integ-test-zip/src/test/java/org/elasticsearch/test/rest/NodeRestUsageIT.java b/distribution/archives/integ-test-zip/src/test/java/org/elasticsearch/test/rest/NodeRestUsageIT.java index de8bcdd9d6b4f..516bdb78a8c4d 100644 --- a/distribution/archives/integ-test-zip/src/test/java/org/elasticsearch/test/rest/NodeRestUsageIT.java +++ b/distribution/archives/integ-test-zip/src/test/java/org/elasticsearch/test/rest/NodeRestUsageIT.java @@ -180,10 +180,10 @@ public void testAggregationUsage() throws IOException { Map> afterCombinedAggsUsage = getTotalUsage(nodesMap); - assertDiff(beforeCombinedAggsUsage, afterCombinedAggsUsage, "terms", "numeric", 1L); + assertDiff(beforeCombinedAggsUsage, afterCombinedAggsUsage, "terms", "long", 1L); assertDiff(beforeCombinedAggsUsage, afterCombinedAggsUsage, "terms", "date", 1L); assertDiff(beforeCombinedAggsUsage, afterCombinedAggsUsage, "terms", "keyword", 2L); - assertDiff(beforeCombinedAggsUsage, afterCombinedAggsUsage, "avg", "numeric", 3L); + assertDiff(beforeCombinedAggsUsage, afterCombinedAggsUsage, "avg", "long", 3L); } private void assertDiff( From aca10ba8cda4447dfbbea3f33e0dc1e91a1244b8 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Fri, 19 Nov 2021 09:43:53 -0500 Subject: [PATCH 15/19] Throw on unknown value types --- .../aggregations/bucket/terms/StringTermsIT.java | 16 +++++++++------- .../support/ValuesSourceRegistry.java | 8 +++++++- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java index 3735537c3fa36..bc844fbd5c1c6 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java @@ -33,7 +33,6 @@ import org.elasticsearch.search.aggregations.support.ValueType; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.test.ESIntegTestCase; -import org.elasticsearch.xcontent.XContentParseException; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.json.JsonXContent; import org.junit.After; @@ -1389,16 +1388,19 @@ public void testScriptWithValueType() throws Exception { assertThat(terms.getBuckets().size(), equalTo(1)); } - String invalidValueType = source.replaceAll("\"value_type\":\"n.*\"", "\"value_type\":\"foobar\""); + String invalidValueType = source.replaceAll("\"value_type\":\"n.*?\"", "\"value_type\":\"foobar\""); try (XContentParser parser = createParser(JsonXContent.jsonXContent, invalidValueType)) { - XContentParseException ex = expectThrows( - XContentParseException.class, - () -> client().prepareSearch("idx").setSource(SearchSourceBuilder.fromXContent(parser)).get() - ); - assertThat(ex.getCause(), instanceOf(IllegalArgumentException.class)); + SearchPhaseExecutionException ex = expectThrows( + SearchPhaseExecutionException.class, + () -> client().prepareSearch("idx").setSource(SearchSourceBuilder.fromXContent(parser)).get()); + + assertThat(ex.getCause(), instanceOf(ElasticsearchException.class)); assertThat(ex.getCause().getMessage(), containsString("Unknown value type [foobar]")); + assertThat(ex.getCause().getCause(), instanceOf(IllegalArgumentException.class)); + assertThat(ex.getCause().getCause().getMessage(), containsString("Unknown value type [foobar]")); + } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java index 4500059eac5a0..d44e834e096b9 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/ValuesSourceRegistry.java @@ -217,6 +217,12 @@ public AggregationUsageService getUsageService() { } public ValuesSourceType resolveTypeHint(String typeHint) { - return typeHint != null ? valueTypeLookup.get(typeHint) : null; + if (typeHint == null) { + return null; + } + if (valueTypeLookup.containsKey(typeHint)) { + return valueTypeLookup.get(typeHint); + } + throw new IllegalArgumentException("Unknown value type [" + typeHint + "]"); } } From 1f9fe3383f9aa02fc53b8ae3e94f0ee97c009dcd Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Fri, 19 Nov 2021 09:54:06 -0500 Subject: [PATCH 16/19] spotless apply --- .../search/aggregations/bucket/terms/StringTermsIT.java | 3 ++- .../org/elasticsearch/xpack/spatial/SpatialPlugin.java | 7 +------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java index bc844fbd5c1c6..06ad6eb3295a9 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/terms/StringTermsIT.java @@ -1393,7 +1393,8 @@ public void testScriptWithValueType() throws Exception { try (XContentParser parser = createParser(JsonXContent.jsonXContent, invalidValueType)) { SearchPhaseExecutionException ex = expectThrows( SearchPhaseExecutionException.class, - () -> client().prepareSearch("idx").setSource(SearchSourceBuilder.fromXContent(parser)).get()); + () -> client().prepareSearch("idx").setSource(SearchSourceBuilder.fromXContent(parser)).get() + ); assertThat(ex.getCause(), instanceOf(ElasticsearchException.class)); assertThat(ex.getCause().getMessage(), containsString("Unknown value type [foobar]")); diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java index aea8e87dd217d..fb2fe448f1eac 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialPlugin.java @@ -155,12 +155,7 @@ public Map getProcessors(Processor.Parameters paramet } private static void registerGeoShapeBoundsAggregator(ValuesSourceRegistry.Builder builder) { - builder.register( - GeoBoundsAggregationBuilder.REGISTRY_KEY, - GeoShapeValuesSourceType.GEOSHAPE, - GeoShapeBoundsAggregator::new, - true - ); + builder.register(GeoBoundsAggregationBuilder.REGISTRY_KEY, GeoShapeValuesSourceType.GEOSHAPE, GeoShapeBoundsAggregator::new, true); } private void registerGeoShapeCentroidAggregator(ValuesSourceRegistry.Builder builder) { From 78fdad5de1e9d25a2f1130123b4a96542ca1619d Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Fri, 19 Nov 2021 10:20:44 -0500 Subject: [PATCH 17/19] Fix NPE --- .../aggregations/support/MultiValuesSourceFieldConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java b/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java index e433d712a0718..cdb64df1186db 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/support/MultiValuesSourceFieldConfig.java @@ -208,7 +208,7 @@ public void writeTo(StreamOutput out) throws IOException { } if (out.getVersion().onOrAfter(Version.V_7_12_0)) { if (out.getVersion().before(Version.V_8_1_0)) { - out.writeOptionalWriteable(ValueType.lenientParse(userValueTypeHint)); + out.writeOptionalWriteable(userValueTypeHint == null ? null : ValueType.lenientParse(userValueTypeHint)); } else { out.writeOptionalString(userValueTypeHint); } From 099f08642e0ecd0a66fb322d0231871b62d1e1ea Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Fri, 19 Nov 2021 11:26:06 -0500 Subject: [PATCH 18/19] Fix another NPE --- .../support/MultiValuesSourceAggregationBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 12643c758bfe2..264fade17c9da 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 @@ -111,7 +111,7 @@ private void read(StreamInput in) throws IOException { protected final void doWriteTo(StreamOutput out) throws IOException { out.writeMap(fields, StreamOutput::writeString, (o, value) -> value.writeTo(o)); if (out.getVersion().before(Version.V_8_1_0)) { - out.writeOptionalWriteable(ValueType.lenientParse(userValueTypeHint)); + out.writeOptionalWriteable(userValueTypeHint == null ? null : ValueType.lenientParse(userValueTypeHint)); } else { out.writeOptionalString(userValueTypeHint); } From f4fa34d72f8efa4be1bcaba19e33f60b998cda04 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Fri, 19 Nov 2021 12:25:49 -0500 Subject: [PATCH 19/19] version output in composite sources correctly --- .../composite/CompositeValuesSourceBuilder.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) 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 c20711e1097cc..b5fdddd00689a 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 @@ -79,12 +79,16 @@ public final void writeTo(StreamOutput out) throws IOException { boolean hasValueType = userValueTypeHint != null; out.writeBoolean(hasValueType); if (hasValueType) { - // Legacy version writes a ValueType - ValueType toWrite = ValueType.lenientParse(userValueTypeHint); - if (toWrite != null) { - toWrite.writeTo(out); + if (out.getVersion().before(Version.V_8_1_0)) { + // Legacy version writes a ValueType + ValueType toWrite = ValueType.lenientParse(userValueTypeHint); + if (toWrite != null) { + toWrite.writeTo(out); + } else { + throw new IOException("Value_type [" + userValueTypeHint + "] not compatible with pre 8.1 nodes"); + } } else { - throw new IOException("Value_type [" + userValueTypeHint + "] not compatible with pre 8.1 nodes"); + out.writeString(userValueTypeHint); } } out.writeBoolean(missingBucket);