Skip to content

Commit 1276d73

Browse files
not-napoleonnot-napoleon
authored
Remove genereics & hard coded ValuesSource references from Matrix Stats (#51131)
1 parent 98d5b22 commit 1276d73

File tree

3 files changed

+35
-163
lines changed

3 files changed

+35
-163
lines changed

‎modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/stats/MatrixStatsAggregationBuilder.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,22 +28,19 @@
2828
import org.elasticsearch.search.aggregations.AggregatorFactories;
2929
import org.elasticsearch.search.aggregations.AggregatorFactory;
3030
import org.elasticsearch.search.aggregations.support.ArrayValuesSourceAggregationBuilder;
31-
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
32-
import org.elasticsearch.search.aggregations.support.ValueType;
33-
import org.elasticsearch.search.aggregations.support.ValuesSource;
3431
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
3532

3633
import java.io.IOException;
3734
import java.util.Map;
3835

3936
public class MatrixStatsAggregationBuilder
40-
extends ArrayValuesSourceAggregationBuilder.LeafOnly<ValuesSource.Numeric, MatrixStatsAggregationBuilder> {
37+
extends ArrayValuesSourceAggregationBuilder.LeafOnly<MatrixStatsAggregationBuilder> {
4138
public static final String NAME = "matrix_stats";
4239

4340
private MultiValueMode multiValueMode = MultiValueMode.AVG;
4441

4542
public MatrixStatsAggregationBuilder(String name) {
46-
super(name, CoreValuesSourceType.NUMERIC, ValueType.NUMERIC);
43+
super(name);
4744
}
4845

4946
protected MatrixStatsAggregationBuilder(MatrixStatsAggregationBuilder clone,
@@ -61,7 +58,7 @@ protected AggregationBuilder shallowCopy(AggregatorFactories.Builder factoriesBu
6158
* Read from a stream.
6259
*/
6360
public MatrixStatsAggregationBuilder(StreamInput in) throws IOException {
64-
super(in, CoreValuesSourceType.NUMERIC, ValueType.NUMERIC);
61+
super(in);
6562
}
6663

6764
@Override

‎modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/support/ArrayValuesSourceAggregationBuilder.java‎

Lines changed: 26 additions & 151 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,11 @@
1818
*/
1919
package org.elasticsearch.search.aggregations.support;
2020

21-
import org.elasticsearch.common.Nullable;
2221
import org.elasticsearch.common.ParseField;
2322
import org.elasticsearch.common.io.stream.StreamInput;
2423
import org.elasticsearch.common.io.stream.StreamOutput;
2524
import org.elasticsearch.common.xcontent.XContentBuilder;
26-
import org.elasticsearch.index.fielddata.IndexFieldData;
27-
import org.elasticsearch.index.fielddata.IndexGeoPointFieldData;
28-
import org.elasticsearch.index.fielddata.IndexNumericFieldData;
29-
import org.elasticsearch.index.mapper.MappedFieldType;
3025
import org.elasticsearch.index.query.QueryShardContext;
31-
import org.elasticsearch.script.Script;
32-
import org.elasticsearch.search.DocValueFormat;
3326
import org.elasticsearch.search.aggregations.AbstractAggregationBuilder;
3427
import org.elasticsearch.search.aggregations.AggregationInitializationException;
3528
import org.elasticsearch.search.aggregations.AggregatorFactories;
@@ -44,19 +37,19 @@
4437
import java.util.Map;
4538
import java.util.Objects;
4639

47-
public abstract class ArrayValuesSourceAggregationBuilder<VS extends ValuesSource, AB extends ArrayValuesSourceAggregationBuilder<VS, AB>>
40+
public abstract class ArrayValuesSourceAggregationBuilder<AB extends ArrayValuesSourceAggregationBuilder<AB>>
4841
extends AbstractAggregationBuilder<AB> {
4942

5043
public static final ParseField MULTIVALUE_MODE_FIELD = new ParseField("mode");
5144

52-
public abstract static class LeafOnly<VS extends ValuesSource, AB extends ArrayValuesSourceAggregationBuilder<VS, AB>>
53-
extends ArrayValuesSourceAggregationBuilder<VS, AB> {
45+
public abstract static class LeafOnly<AB extends ArrayValuesSourceAggregationBuilder<AB>>
46+
extends ArrayValuesSourceAggregationBuilder<AB> {
5447

55-
protected LeafOnly(String name, ValuesSourceType valuesSourceType, ValueType targetValueType) {
56-
super(name, valuesSourceType, targetValueType);
48+
protected LeafOnly(String name) {
49+
super(name);
5750
}
5851

59-
protected LeafOnly(LeafOnly<VS, AB> clone, Builder factoriesBuilder, Map<String, Object> metaData) {
52+
protected LeafOnly(LeafOnly<AB> clone, Builder factoriesBuilder, Map<String, Object> metaData) {
6053
super(clone, factoriesBuilder, metaData);
6154
if (factoriesBuilder.count() > 0) {
6255
throw new AggregationInitializationException("Aggregator [" + name + "] of type ["
@@ -65,18 +58,10 @@ protected LeafOnly(LeafOnly<VS, AB> clone, Builder factoriesBuilder, Map<String,
6558
}
6659

6760
/**
68-
* Read from a stream that does not serialize its targetValueType. This should be used by most subclasses.
61+
* Read from a stream
6962
*/
70-
protected LeafOnly(StreamInput in, ValuesSourceType valuesSourceType, ValueType targetValueType) throws IOException {
71-
super(in, valuesSourceType, targetValueType);
72-
}
73-
74-
/**
75-
* Read an aggregation from a stream that serializes its targetValueType. This should only be used by subclasses that override
76-
* {@link #serializeTargetValueType()} to return true.
77-
*/
78-
protected LeafOnly(StreamInput in, ValuesSourceType valuesSourceType) throws IOException {
79-
super(in, valuesSourceType);
63+
protected LeafOnly(StreamInput in) throws IOException {
64+
super(in);
8065
}
8166

8267
@Override
@@ -86,49 +71,32 @@ public AB subAggregations(Builder subFactories) {
8671
}
8772
}
8873

89-
private final ValuesSourceType valuesSourceType;
90-
private final ValueType targetValueType;
9174
private List<String> fields = Collections.emptyList();
92-
private ValueType valueType = null;
75+
/* The parser doesn't support setting userValueTypeHint (aka valueType), but we do serialize and deserialize it, so keeping it around
76+
for now so as to not break BWC. Future refactors should feel free to remove this field. --Tozzi 2020-01-16
77+
*/
78+
private ValueType userValueTypeHint = null;
9379
private String format = null;
9480
private Object missing = null;
9581
private Map<String, Object> missingMap = Collections.emptyMap();
9682

97-
protected ArrayValuesSourceAggregationBuilder(String name, ValuesSourceType valuesSourceType, ValueType targetValueType) {
83+
protected ArrayValuesSourceAggregationBuilder(String name) {
9884
super(name);
99-
if (valuesSourceType == null) {
100-
throw new IllegalArgumentException("[valuesSourceType] must not be null: [" + name + "]");
101-
}
102-
this.valuesSourceType = valuesSourceType;
103-
this.targetValueType = targetValueType;
10485
}
10586

106-
protected ArrayValuesSourceAggregationBuilder(ArrayValuesSourceAggregationBuilder<VS, AB> clone,
87+
protected ArrayValuesSourceAggregationBuilder(ArrayValuesSourceAggregationBuilder<AB> clone,
10788
Builder factoriesBuilder, Map<String, Object> metaData) {
10889
super(clone, factoriesBuilder, metaData);
109-
this.valuesSourceType = clone.valuesSourceType;
110-
this.targetValueType = clone.targetValueType;
11190
this.fields = new ArrayList<>(clone.fields);
112-
this.valueType = clone.valueType;
91+
this.userValueTypeHint = clone.userValueTypeHint;
11392
this.format = clone.format;
11493
this.missingMap = new HashMap<>(clone.missingMap);
11594
this.missing = clone.missing;
11695
}
11796

118-
protected ArrayValuesSourceAggregationBuilder(StreamInput in, ValuesSourceType valuesSourceType, ValueType targetValueType)
97+
protected ArrayValuesSourceAggregationBuilder(StreamInput in)
11998
throws IOException {
12099
super(in);
121-
assert false == serializeTargetValueType() : "Wrong read constructor called for subclass that provides its targetValueType";
122-
this.valuesSourceType = valuesSourceType;
123-
this.targetValueType = targetValueType;
124-
read(in);
125-
}
126-
127-
protected ArrayValuesSourceAggregationBuilder(StreamInput in, ValuesSourceType valuesSourceType) throws IOException {
128-
super(in);
129-
assert serializeTargetValueType() : "Wrong read constructor called for subclass that serializes its targetValueType";
130-
this.valuesSourceType = valuesSourceType;
131-
this.targetValueType = in.readOptionalWriteable(ValueType::readFromStream);
132100
read(in);
133101
}
134102

@@ -138,18 +106,15 @@ protected ArrayValuesSourceAggregationBuilder(StreamInput in, ValuesSourceType v
138106
@SuppressWarnings("unchecked")
139107
private void read(StreamInput in) throws IOException {
140108
fields = (ArrayList<String>)in.readGenericValue();
141-
valueType = in.readOptionalWriteable(ValueType::readFromStream);
109+
userValueTypeHint = in.readOptionalWriteable(ValueType::readFromStream);
142110
format = in.readOptionalString();
143111
missingMap = in.readMap();
144112
}
145113

146114
@Override
147115
protected final void doWriteTo(StreamOutput out) throws IOException {
148-
if (serializeTargetValueType()) {
149-
out.writeOptionalWriteable(targetValueType);
150-
}
151116
out.writeGenericValue(fields);
152-
out.writeOptionalWriteable(valueType);
117+
out.writeOptionalWriteable(userValueTypeHint);
153118
out.writeOptionalString(format);
154119
out.writeMap(missingMap);
155120
innerWriteTo(out);
@@ -179,25 +144,6 @@ public List<String> fields() {
179144
return fields;
180145
}
181146

182-
/**
183-
* Sets the {@link ValueType} for the value produced by this aggregation
184-
*/
185-
@SuppressWarnings("unchecked")
186-
public AB valueType(ValueType valueType) {
187-
if (valueType == null) {
188-
throw new IllegalArgumentException("[valueType] must not be null: [" + name + "]");
189-
}
190-
this.valueType = valueType;
191-
return (AB) this;
192-
}
193-
194-
/**
195-
* Gets the {@link ValueType} for the value produced by this aggregation
196-
*/
197-
public ValueType valueType() {
198-
return valueType;
199-
}
200-
201147
/**
202148
* Sets the format to use for the output of the aggregation.
203149
*/
@@ -249,7 +195,8 @@ protected final ArrayValuesSourceAggregatorFactory doBuild(QueryShardContext que
249195
protected Map<String, ValuesSourceConfig> resolveConfig(QueryShardContext queryShardContext) {
250196
HashMap<String, ValuesSourceConfig> configs = new HashMap<>();
251197
for (String field : fields) {
252-
ValuesSourceConfig config = config(queryShardContext, field, null);
198+
ValuesSourceConfig config = ValuesSourceConfig.resolve(queryShardContext, userValueTypeHint, field, null, missingMap.get(field),
199+
null, format, CoreValuesSourceType.BYTES, "");
253200
configs.put(field, config);
254201
}
255202
return configs;
@@ -260,76 +207,6 @@ protected abstract ArrayValuesSourceAggregatorFactory innerBuild(QueryShardConte
260207
AggregatorFactory parent,
261208
AggregatorFactories.Builder subFactoriesBuilder) throws IOException;
262209

263-
public ValuesSourceConfig config(QueryShardContext queryShardContext, String field, Script script) {
264-
265-
ValueType valueType = this.valueType != null ? this.valueType : targetValueType;
266-
267-
if (field == null) {
268-
if (script == null) {
269-
ValuesSourceConfig config = new ValuesSourceConfig(CoreValuesSourceType.ANY);
270-
return config.format(resolveFormat(null, valueType));
271-
}
272-
ValuesSourceType valuesSourceType = valueType != null ? valueType.getValuesSourceType() : this.valuesSourceType;
273-
if (valuesSourceType == null || valuesSourceType == CoreValuesSourceType.ANY) {
274-
// the specific value source type is undefined, but for scripts,
275-
// we need to have a specific value source
276-
// type to know how to handle the script values, so we fallback
277-
// on Bytes
278-
valuesSourceType = CoreValuesSourceType.BYTES;
279-
}
280-
ValuesSourceConfig config = new ValuesSourceConfig(valuesSourceType);
281-
config.missing(missingMap.get(field));
282-
return config.format(resolveFormat(format, valueType));
283-
}
284-
285-
MappedFieldType fieldType = queryShardContext.getMapperService().fullName(field);
286-
if (fieldType == null) {
287-
ValuesSourceType valuesSourceType = valueType != null ? valueType.getValuesSourceType() : this.valuesSourceType;
288-
ValuesSourceConfig config = new ValuesSourceConfig(valuesSourceType);
289-
config.missing(missingMap.get(field));
290-
config.format(resolveFormat(format, valueType));
291-
return config.unmapped(true);
292-
}
293-
294-
IndexFieldData<?> indexFieldData = queryShardContext.getForField(fieldType);
295-
296-
ValuesSourceConfig config;
297-
if (valuesSourceType == CoreValuesSourceType.ANY) {
298-
if (indexFieldData instanceof IndexNumericFieldData) {
299-
config = new ValuesSourceConfig(CoreValuesSourceType.NUMERIC);
300-
} else if (indexFieldData instanceof IndexGeoPointFieldData) {
301-
config = new ValuesSourceConfig(CoreValuesSourceType.GEOPOINT);
302-
} else {
303-
config = new ValuesSourceConfig(CoreValuesSourceType.BYTES);
304-
}
305-
} else {
306-
config = new ValuesSourceConfig(valuesSourceType);
307-
}
308-
309-
config.fieldContext(new FieldContext(field, indexFieldData, fieldType));
310-
config.missing(missingMap.get(field));
311-
return config.format(fieldType.docValueFormat(format, null));
312-
}
313-
314-
private static DocValueFormat resolveFormat(@Nullable String format, @Nullable ValueType valueType) {
315-
if (valueType == null) {
316-
return DocValueFormat.RAW; // we can't figure it out
317-
}
318-
DocValueFormat valueFormat = valueType.defaultFormat();
319-
if (valueFormat instanceof DocValueFormat.Decimal && format != null) {
320-
valueFormat = new DocValueFormat.Decimal(format);
321-
}
322-
return valueFormat;
323-
}
324-
325-
/**
326-
* Should this builder serialize its targetValueType? Defaults to false. All subclasses that override this to true
327-
* should use the three argument read constructor rather than the four argument version.
328-
*/
329-
protected boolean serializeTargetValueType() {
330-
return false;
331-
}
332-
333210
@Override
334211
public final XContentBuilder internalXContent(XContentBuilder builder, Params params) throws IOException {
335212
builder.startObject();
@@ -343,8 +220,8 @@ public final XContentBuilder internalXContent(XContentBuilder builder, Params pa
343220
if (format != null) {
344221
builder.field(CommonFields.FORMAT.getPreferredName(), format);
345222
}
346-
if (valueType != null) {
347-
builder.field(CommonFields.VALUE_TYPE.getPreferredName(), valueType.getPreferredName());
223+
if (userValueTypeHint != null) {
224+
builder.field(CommonFields.VALUE_TYPE.getPreferredName(), userValueTypeHint.getPreferredName());
348225
}
349226
doXContentBody(builder, params);
350227
builder.endObject();
@@ -355,20 +232,18 @@ public final XContentBuilder internalXContent(XContentBuilder builder, Params pa
355232

356233
@Override
357234
public int hashCode() {
358-
return Objects.hash(super.hashCode(), fields, format, missing, targetValueType, valueType, valuesSourceType);
235+
return Objects.hash(super.hashCode(), fields, format, missing, userValueTypeHint);
359236
}
360237

361238
@Override
362239
public boolean equals(Object obj) {
363240
if (this == obj) return true;
364241
if (obj == null || getClass() != obj.getClass()) return false;
365242
if (super.equals(obj) == false) return false;
366-
ArrayValuesSourceAggregationBuilder<?, ?> other = (ArrayValuesSourceAggregationBuilder<?, ?>) obj;
243+
ArrayValuesSourceAggregationBuilder<?> other = (ArrayValuesSourceAggregationBuilder<?>) obj;
367244
return Objects.equals(fields, other.fields)
368245
&& Objects.equals(format, other.format)
369246
&& Objects.equals(missing, other.missing)
370-
&& Objects.equals(targetValueType, other.targetValueType)
371-
&& Objects.equals(valueType, other.valueType)
372-
&& Objects.equals(valuesSourceType, other.valuesSourceType);
247+
&& Objects.equals(userValueTypeHint, other.userValueTypeHint);
373248
}
374249
}

‎modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/support/ArrayValuesSourceParser.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ private ArrayValuesSourceParser(boolean formattable, ValuesSourceType valuesSour
7474
}
7575

7676
@Override
77-
public final ArrayValuesSourceAggregationBuilder<VS, ?> parse(String aggregationName, XContentParser parser)
77+
public final ArrayValuesSourceAggregationBuilder<?> parse(String aggregationName, XContentParser parser)
7878
throws IOException {
7979

8080
List<String> fields = null;
@@ -139,7 +139,7 @@ private ArrayValuesSourceParser(boolean formattable, ValuesSourceType valuesSour
139139
}
140140
}
141141

142-
ArrayValuesSourceAggregationBuilder<VS, ?> factory = createFactory(aggregationName, this.valuesSourceType, this.targetValueType,
142+
ArrayValuesSourceAggregationBuilder<?> factory = createFactory(aggregationName, this.valuesSourceType, this.targetValueType,
143143
otherOptions);
144144
if (fields != null) {
145145
factory.fields(fields);
@@ -193,10 +193,10 @@ private void parseMissingAndAdd(final String aggregationName, final String curre
193193
* method
194194
* @return the created factory
195195
*/
196-
protected abstract ArrayValuesSourceAggregationBuilder<VS, ?> createFactory(String aggregationName,
197-
ValuesSourceType valuesSourceType,
198-
ValueType targetValueType,
199-
Map<ParseField, Object> otherOptions);
196+
protected abstract ArrayValuesSourceAggregationBuilder<?> createFactory(String aggregationName,
197+
ValuesSourceType valuesSourceType,
198+
ValueType targetValueType,
199+
Map<ParseField, Object> otherOptions);
200200

201201
/**
202202
* Allows subclasses of {@link ArrayValuesSourceParser} to parse extra

0 commit comments

Comments
 (0)