Skip to content

Commit ec399ef

Browse files
dimitris-athanasioukcm
authored andcommitted
[ML] Refactor doc value format into ExtractedField (#35053)
This commit moves the knowledge of which doc value format to be used down to the `ExtractedField` instead of being in the data extractor.
1 parent adb02b7 commit ec399ef

File tree

5 files changed

+44
-27
lines changed

5 files changed

+44
-27
lines changed

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ExtractedField.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import org.elasticsearch.common.document.DocumentField;
99
import org.elasticsearch.search.SearchHit;
10+
import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext;
1011
import org.joda.time.base.BaseDateTime;
1112

1213
import java.util.List;
@@ -51,6 +52,10 @@ public ExtractionMethod getExtractionMethod() {
5152

5253
public abstract Object[] value(SearchHit hit);
5354

55+
public String getDocValueFormat() {
56+
return DocValueFieldsContext.USE_DEFAULT_FORMAT;
57+
}
58+
5459
public static ExtractedField newTimeField(String name, ExtractionMethod extractionMethod) {
5560
if (extractionMethod == ExtractionMethod.SOURCE) {
5661
throw new IllegalArgumentException("time field cannot be extracted from source");
@@ -93,6 +98,8 @@ public Object[] value(SearchHit hit) {
9398

9499
private static class TimeField extends FromFields {
95100

101+
private static final String EPOCH_MILLIS_FORMAT = "epoch_millis";
102+
96103
TimeField(String name, ExtractionMethod extractionMethod) {
97104
super(name, name, extractionMethod);
98105
}
@@ -112,6 +119,11 @@ public Object[] value(SearchHit hit) {
112119
}
113120
return value;
114121
}
122+
123+
@Override
124+
public String getDocValueFormat() {
125+
return EPOCH_MILLIS_FORMAT;
126+
}
115127
}
116128

117129
private static class FromSource extends ExtractedField {

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ExtractedFields.java

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class ExtractedFields {
3131

3232
private final ExtractedField timeField;
3333
private final List<ExtractedField> allFields;
34-
private final String[] docValueFields;
34+
private final List<ExtractedField> docValueFields;
3535
private final String[] sourceFields;
3636

3737
ExtractedFields(ExtractedField timeField, List<ExtractedField> allFields) {
@@ -41,7 +41,8 @@ class ExtractedFields {
4141
this.timeField = Objects.requireNonNull(timeField);
4242
this.allFields = Collections.unmodifiableList(allFields);
4343
this.docValueFields = filterFields(ExtractedField.ExtractionMethod.DOC_VALUE, allFields);
44-
this.sourceFields = filterFields(ExtractedField.ExtractionMethod.SOURCE, allFields);
44+
this.sourceFields = filterFields(ExtractedField.ExtractionMethod.SOURCE, allFields).stream().map(ExtractedField::getName)
45+
.toArray(String[]::new);
4546
}
4647

4748
public List<ExtractedField> getAllFields() {
@@ -52,18 +53,12 @@ public String[] getSourceFields() {
5253
return sourceFields;
5354
}
5455

55-
public String[] getDocValueFields() {
56+
public List<ExtractedField> getDocValueFields() {
5657
return docValueFields;
5758
}
5859

59-
private static String[] filterFields(ExtractedField.ExtractionMethod method, List<ExtractedField> fields) {
60-
List<String> result = new ArrayList<>();
61-
for (ExtractedField field : fields) {
62-
if (field.getExtractionMethod() == method) {
63-
result.add(field.getName());
64-
}
65-
}
66-
return result.toArray(new String[result.size()]);
60+
private static List<ExtractedField> filterFields(ExtractedField.ExtractionMethod method, List<ExtractedField> fields) {
61+
return fields.stream().filter(field -> field.getExtractionMethod() == method).collect(Collectors.toList());
6762
}
6863

6964
public String timeField() {

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractor.java

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import org.elasticsearch.common.unit.TimeValue;
2020
import org.elasticsearch.search.SearchHit;
2121
import org.elasticsearch.search.fetch.StoredFieldsContext;
22-
import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext;
2322
import org.elasticsearch.search.sort.SortOrder;
2423
import org.elasticsearch.xpack.core.ClientHelper;
2524
import org.elasticsearch.xpack.core.ml.datafeed.extractor.DataExtractor;
@@ -44,7 +43,6 @@ class ScrollDataExtractor implements DataExtractor {
4443

4544
private static final Logger LOGGER = LogManager.getLogger(ScrollDataExtractor.class);
4645
private static final TimeValue SCROLL_TIMEOUT = new TimeValue(30, TimeUnit.MINUTES);
47-
private static final String EPOCH_MILLIS_FORMAT = "epoch_millis";
4846

4947
private final Client client;
5048
private final ScrollDataExtractorContext context;
@@ -112,12 +110,8 @@ private SearchRequestBuilder buildSearchRequest(long start) {
112110
.setQuery(ExtractorUtils.wrapInTimeRangeQuery(
113111
context.query, context.extractedFields.timeField(), start, context.end));
114112

115-
for (String docValueField : context.extractedFields.getDocValueFields()) {
116-
if (docValueField.equals(context.extractedFields.timeField())) {
117-
searchRequestBuilder.addDocValueField(docValueField, EPOCH_MILLIS_FORMAT);
118-
} else {
119-
searchRequestBuilder.addDocValueField(docValueField, DocValueFieldsContext.USE_DEFAULT_FORMAT);
120-
}
113+
for (ExtractedField docValueField : context.extractedFields.getDocValueFields()) {
114+
searchRequestBuilder.addDocValueField(docValueField.getName(), docValueField.getDocValueFormat());
121115
}
122116
String[] sourceFields = context.extractedFields.getSourceFields();
123117
if (sourceFields.length == 0) {

x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ExtractedFieldTests.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.elasticsearch.xpack.ml.datafeed.extractor.scroll;
77

88
import org.elasticsearch.search.SearchHit;
9+
import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext;
910
import org.elasticsearch.test.ESTestCase;
1011
import org.elasticsearch.xpack.ml.test.SearchHitBuilder;
1112
import org.joda.time.DateTime;
@@ -140,4 +141,14 @@ public void testAliasVersusName() {
140141
assertThat(field.getName(), equalTo("b"));
141142
assertThat(field.value(hit), equalTo(new Integer[] { 2 }));
142143
}
144+
145+
public void testGetDocValueFormat() {
146+
for (ExtractedField.ExtractionMethod method : ExtractedField.ExtractionMethod.values()) {
147+
assertThat(ExtractedField.newField("f", method).getDocValueFormat(), equalTo(DocValueFieldsContext.USE_DEFAULT_FORMAT));
148+
}
149+
assertThat(ExtractedField.newTimeField("doc_value_time", ExtractedField.ExtractionMethod.DOC_VALUE).getDocValueFormat(),
150+
equalTo("epoch_millis"));
151+
assertThat(ExtractedField.newTimeField("source_time", ExtractedField.ExtractionMethod.SCRIPT_FIELD).getDocValueFormat(),
152+
equalTo("epoch_millis"));
153+
}
143154
}

x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ExtractedFieldsTests.java

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.elasticsearch.rest.RestStatus;
1212
import org.elasticsearch.search.SearchHit;
1313
import org.elasticsearch.search.builder.SearchSourceBuilder;
14+
import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext;
1415
import org.elasticsearch.test.ESTestCase;
1516
import org.elasticsearch.xpack.core.ml.datafeed.DatafeedConfig;
1617
import org.elasticsearch.xpack.core.ml.job.config.AnalysisConfig;
@@ -43,7 +44,8 @@ public void testTimeFieldOnly() {
4344

4445
assertThat(extractedFields.getAllFields(), equalTo(Arrays.asList(timeField)));
4546
assertThat(extractedFields.timeField(), equalTo("time"));
46-
assertThat(extractedFields.getDocValueFields(), equalTo(new String[] { timeField.getName() }));
47+
assertThat(extractedFields.getDocValueFields().stream().map(ExtractedField::getName).toArray(String[]::new),
48+
equalTo(new String[] { timeField.getName() }));
4749
assertThat(extractedFields.getSourceFields().length, equalTo(0));
4850
}
4951

@@ -59,7 +61,8 @@ public void testAllTypesOfFields() {
5961

6062
assertThat(extractedFields.getAllFields().size(), equalTo(7));
6163
assertThat(extractedFields.timeField(), equalTo("time"));
62-
assertThat(extractedFields.getDocValueFields(), equalTo(new String[] {"time", "doc1", "doc2"}));
64+
assertThat(extractedFields.getDocValueFields().stream().map(ExtractedField::getName).toArray(String[]::new),
65+
equalTo(new String[] {"time", "doc1", "doc2"}));
6366
assertThat(extractedFields.getSourceFields(), equalTo(new String[] {"src1", "src2"}));
6467
}
6568

@@ -138,9 +141,11 @@ public void testBuildGivenMixtureOfTypes() {
138141
fieldCapabilitiesResponse);
139142

140143
assertThat(extractedFields.timeField(), equalTo("time"));
141-
assertThat(extractedFields.getDocValueFields().length, equalTo(2));
142-
assertThat(extractedFields.getDocValueFields()[0], equalTo("time"));
143-
assertThat(extractedFields.getDocValueFields()[1], equalTo("value"));
144+
assertThat(extractedFields.getDocValueFields().size(), equalTo(2));
145+
assertThat(extractedFields.getDocValueFields().get(0).getName(), equalTo("time"));
146+
assertThat(extractedFields.getDocValueFields().get(0).getDocValueFormat(), equalTo("epoch_millis"));
147+
assertThat(extractedFields.getDocValueFields().get(1).getName(), equalTo("value"));
148+
assertThat(extractedFields.getDocValueFields().get(1).getDocValueFormat(), equalTo(DocValueFieldsContext.USE_DEFAULT_FORMAT));
144149
assertThat(extractedFields.getSourceFields().length, equalTo(1));
145150
assertThat(extractedFields.getSourceFields()[0], equalTo("airline"));
146151
assertThat(extractedFields.getAllFields().size(), equalTo(4));
@@ -174,9 +179,9 @@ public void testBuildGivenMultiFields() {
174179
fieldCapabilitiesResponse);
175180

176181
assertThat(extractedFields.timeField(), equalTo("time"));
177-
assertThat(extractedFields.getDocValueFields().length, equalTo(2));
178-
assertThat(extractedFields.getDocValueFields()[0], equalTo("time"));
179-
assertThat(extractedFields.getDocValueFields()[1], equalTo("airport.keyword"));
182+
assertThat(extractedFields.getDocValueFields().size(), equalTo(2));
183+
assertThat(extractedFields.getDocValueFields().get(0).getName(), equalTo("time"));
184+
assertThat(extractedFields.getDocValueFields().get(1).getName(), equalTo("airport.keyword"));
180185
assertThat(extractedFields.getSourceFields().length, equalTo(1));
181186
assertThat(extractedFields.getSourceFields()[0], equalTo("airline"));
182187
assertThat(extractedFields.getAllFields().size(), equalTo(3));

0 commit comments

Comments
 (0)