From a19df4ab3bad6d209e20434eada4dbd1ee5be651 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Wed, 23 May 2018 14:39:04 +0200 Subject: [PATCH] Add a `format` option to `docvalue_fields`. (#29639) This commit adds the ability to configure how a docvalue field should be formatted, so that it would be possible eg. to return a date field formatted as the number of milliseconds since Epoch. Closes #27740 --- .../search/request/docvalue-fields.asciidoc | 27 +++++- .../search/request/inner-hits.asciidoc | 7 +- .../test/search.inner_hits/10_basic.yml | 9 +- .../test/search/10_source_filtering.yml | 46 +++++++++ .../rest-api-spec/test/search/30_limits.yml | 11 ++- .../action/search/ExpandSearchPhase.java | 2 +- .../action/search/SearchRequestBuilder.java | 14 ++- .../index/query/InnerHitBuilder.java | 61 +++++++++--- .../rest/action/search/RestSearchAction.java | 2 +- .../elasticsearch/search/DocValueFormat.java | 22 ++--- .../DateHistogramAggregationBuilder.java | 4 +- .../histogram/InternalDateHistogram.java | 4 +- .../bucket/histogram/InternalHistogram.java | 4 +- .../bucket/range/InternalBinaryRange.java | 4 +- .../bucket/range/InternalRange.java | 4 +- .../significant/SignificantLongTerms.java | 2 +- .../significant/SignificantStringTerms.java | 2 +- .../SignificantTermsAggregatorFactory.java | 4 +- .../SignificantTextAggregatorFactory.java | 2 +- .../bucket/terms/DoubleTerms.java | 4 +- .../aggregations/bucket/terms/LongTerms.java | 4 +- .../bucket/terms/StringTerms.java | 2 +- .../InternalNumericMetricsAggregation.java | 4 +- .../aggregations/metrics/avg/InternalAvg.java | 2 +- .../aggregations/metrics/max/InternalMax.java | 2 +- .../aggregations/metrics/min/InternalMin.java | 2 +- .../hdr/AbstractInternalHDRPercentiles.java | 2 +- .../AbstractInternalTDigestPercentiles.java | 2 +- .../aggregations/metrics/sum/InternalSum.java | 2 +- .../tophits/TopHitsAggregationBuilder.java | 74 +++++++-------- .../tophits/TopHitsAggregatorFactory.java | 5 +- .../pipeline/InternalSimpleValue.java | 2 +- .../InternalBucketMetricValue.java | 2 +- .../percentile/InternalPercentilesBucket.java | 2 +- .../search/builder/SearchSourceBuilder.java | 64 ++++++++++--- .../fetch/subphase/DocValueFieldsContext.java | 94 ++++++++++++++++++- .../subphase/DocValueFieldsFetchSubPhase.java | 91 +++++++++++++++--- .../index/mapper/BooleanFieldTypeTests.java | 4 +- .../index/query/InnerHitBuilderTests.java | 12 ++- .../search/DocValueFormatTests.java | 16 ++-- .../metrics/InternalSumTests.java | 2 +- .../aggregations/metrics/TopHitsIT.java | 4 +- .../aggregations/metrics/TopHitsTests.java | 2 +- .../search/fields/SearchFieldsIT.java | 59 ++++++++++++ .../docs/en/sql/endpoints/translate.asciidoc | 10 +- .../ml/job/persistence/MockClientBuilder.java | 3 + .../sql/plugin/SqlTranslateResponseTests.java | 3 +- .../xpack/sql/execution/search/Querier.java | 4 +- .../execution/search/SqlSourceBuilder.java | 11 ++- .../search/extractor/FieldHitExtractor.java | 28 +++++- .../scalar/datetime/DateTimeProcessor.java | 22 +++-- .../querydsl/container/QueryContainer.java | 5 +- .../querydsl/container/SearchHitFieldRef.java | 16 +++- .../xpack/sql/action/SqlLicenseIT.java | 4 +- .../sql/action/SqlTranslateActionIT.java | 5 +- .../search/SqlSourceBuilderTests.java | 6 +- .../extractor/ComputingExtractorTests.java | 2 +- .../extractor/FieldHitExtractorTests.java | 39 +++++--- .../rest-api-spec/test/sql/translate.yml | 8 +- .../xpack/qa/sql/nosecurity/CliExplainIT.java | 5 +- 60 files changed, 661 insertions(+), 200 deletions(-) diff --git a/docs/reference/search/request/docvalue-fields.asciidoc b/docs/reference/search/request/docvalue-fields.asciidoc index b4d2493d853..9d917c27ab0 100644 --- a/docs/reference/search/request/docvalue-fields.asciidoc +++ b/docs/reference/search/request/docvalue-fields.asciidoc @@ -11,13 +11,38 @@ GET /_search "query" : { "match_all": {} }, - "docvalue_fields" : ["test1", "test2"] + "docvalue_fields" : [ + { + "field": "my_ip_field", <1> + "format": "use_field_mapping" <2> + }, + { + "field": "my_date_field", + "format": "epoch_millis" <3> + } + ] } -------------------------------------------------- // CONSOLE +<1> the name of the field +<2> the special `use_field_mapping` format tells Elasticsearch to use the format from the mapping +<3> date fields may use a custom format Doc value fields can work on fields that are not stored. Note that if the fields parameter specifies fields without docvalues it will try to load the value from the fielddata cache causing the terms for that field to be loaded to memory (cached), which will result in more memory consumption. +[float] +==== Custom formats + +While most fields do not support custom formats, some of them do: + - <> fields can take any <>. + - <> fields accept a https://docs.oracle.com/javase/8/docs/api/java/text/DecimalFormat.html[DecimalFormat pattern]. + +All fields support the special `use_field_mapping` format, which tells +Elasticsearch to use the mappings to figure out a default format. + +NOTE: The default is currently to return the same output as +<>. However it will change in 7.0 +to behave as if the `use_field_mapping` format was provided. diff --git a/docs/reference/search/request/inner-hits.asciidoc b/docs/reference/search/request/inner-hits.asciidoc index dce6bb2a2d8..887ae2bdf14 100644 --- a/docs/reference/search/request/inner-hits.asciidoc +++ b/docs/reference/search/request/inner-hits.asciidoc @@ -242,7 +242,12 @@ POST test/_search }, "inner_hits": { "_source" : false, - "docvalue_fields" : ["comments.text.keyword"] + "docvalue_fields" : [ + { + "field": "comments.text.keyword", + "format": "use_field_mapping" + } + ] } } } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search.inner_hits/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search.inner_hits/10_basic.yml index 40e9d705ea4..814d4399bfc 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search.inner_hits/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search.inner_hits/10_basic.yml @@ -45,9 +45,8 @@ setup: "Nested doc version and seqIDs": - skip: - # fixed in 6.0.1 - version: " - 6.0.0" - reason: "version and seq IDs where not accurate in previous versions" + version: " - 6.99.99" # TODO change to 6.3.99 on backport + reason: "object notation for docvalue_fields was introduced in 6.4" - do: index: @@ -61,7 +60,7 @@ setup: - do: search: - body: { "query" : { "nested" : { "path" : "nested_field", "query" : { "match_all" : {} }, "inner_hits" : { version: true, "docvalue_fields": ["_seq_no"]} }}, "version": true, "docvalue_fields" : ["_seq_no"] } + body: { "query" : { "nested" : { "path" : "nested_field", "query" : { "match_all" : {} }, "inner_hits" : { version: true, "docvalue_fields": [ { "field": "_seq_no", "format": "use_field_mapping" } ]} }}, "version": true, "docvalue_fields" : [ { "field": "_seq_no", "format": "use_field_mapping" } ] } - match: { hits.total: 1 } - match: { hits.hits.0._index: "test" } @@ -84,7 +83,7 @@ setup: - do: search: - body: { "query" : { "nested" : { "path" : "nested_field", "query" : { "match_all" : {} }, "inner_hits" : { version: true, "docvalue_fields": ["_seq_no"]} }}, "version": true, "docvalue_fields" : ["_seq_no"] } + body: { "query" : { "nested" : { "path" : "nested_field", "query" : { "match_all" : {} }, "inner_hits" : { version: true, "docvalue_fields": [ { "field": "_seq_no", "format": "use_field_mapping" } ]} }}, "version": true, "docvalue_fields" : [ { "field": "_seq_no", "format": "use_field_mapping" } ] } - match: { hits.total: 1 } - match: { hits.hits.0._index: "test" } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/10_source_filtering.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/10_source_filtering.yml index 3830a68b28f..48b2f5f2021 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/10_source_filtering.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/10_source_filtering.yml @@ -133,7 +133,53 @@ setup: --- "docvalue_fields": + - skip: + version: " - 6.99.99" # TODO: change version on backport + reason: format option was added in 6.4 + features: warnings - do: + warnings: + - 'Doc-value field [count] is not using a format. The output will change in 7.0 when doc value fields get formatted based on mappings by default. It is recommended to pass [format=use_field_mapping] with the doc value field in order to opt in for the future behaviour and ease the migration to 7.0.' + search: + body: + docvalue_fields: [ "count" ] + - match: { hits.hits.0.fields.count: [1] } + +--- +"docvalue_fields as url param": + - skip: + version: " - 6.99.99" # TODO: change version on backport + reason: format option was added in 6.4 + features: warnings + - do: + warnings: + - 'Doc-value field [count] is not using a format. The output will change in 7.0 when doc value fields get formatted based on mappings by default. It is recommended to pass [format=use_field_mapping] with the doc value field in order to opt in for the future behaviour and ease the migration to 7.0.' search: docvalue_fields: [ "count" ] - match: { hits.hits.0.fields.count: [1] } + +--- +"docvalue_fields with default format": + - skip: + version: " - 6.99.99" # TODO: change version on backport + reason: format option was added in 6.4 + - do: + search: + body: + docvalue_fields: + - field: "count" + format: "use_field_mapping" + - match: { hits.hits.0.fields.count: [1] } + +--- +"docvalue_fields with explicit format": + - skip: + version: " - 6.99.99" # TODO: change version on backport + reason: format option was added in 6.4 + - do: + search: + body: + docvalue_fields: + - field: "count" + format: "#.0" + - match: { hits.hits.0.fields.count: ["1.0"] } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml index da9c739ed66..11c0ccda2bd 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/30_limits.yml @@ -62,6 +62,9 @@ setup: --- "Docvalues_fields size limit": + - skip: + version: " - 6.99.99" # TODO: change to 6.3.99 on backport + reason: "The object notation for docvalue_fields is only supported on 6.4+" - do: catch: /Trying to retrieve too many docvalue_fields\. Must be less than or equal to[:] \[2\] but was \[3\]\. This limit can be set by changing the \[index.max_docvalue_fields_search\] index level setting\./ search: @@ -69,7 +72,13 @@ setup: body: query: match_all: {} - docvalue_fields: ["one", "two", "three"] + docvalue_fields: + - field: "one" + format: "use_field_mapping" + - field: "two" + format: "use_field_mapping" + - field: "three" + format: "use_field_mapping" --- "Script_fields size limit": diff --git a/server/src/main/java/org/elasticsearch/action/search/ExpandSearchPhase.java b/server/src/main/java/org/elasticsearch/action/search/ExpandSearchPhase.java index 1e11f126bb6..d9ed6e6792f 100644 --- a/server/src/main/java/org/elasticsearch/action/search/ExpandSearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/ExpandSearchPhase.java @@ -153,7 +153,7 @@ final class ExpandSearchPhase extends SearchPhase { } } if (options.getDocValueFields() != null) { - options.getDocValueFields().forEach(groupSource::docValueField); + options.getDocValueFields().forEach(ff -> groupSource.docValueField(ff.field, ff.format)); } if (options.getStoredFieldsContext() != null && options.getStoredFieldsContext().fieldNames() != null) { options.getStoredFieldsContext().fieldNames().forEach(groupSource::storedField); diff --git a/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java index 91ac46c1d62..424db04ce39 100644 --- a/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java @@ -290,11 +290,21 @@ public class SearchRequestBuilder extends ActionRequestBuilder FieldAndFormat.fromXContent(p), SearchSourceBuilder.DOCVALUE_FIELDS_FIELD); PARSER.declareField((p, i, c) -> { try { Set scriptFields = new HashSet<>(); @@ -102,7 +105,7 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject { private StoredFieldsContext storedFieldsContext; private QueryBuilder query = DEFAULT_INNER_HIT_QUERY; private List> sorts; - private List docValueFields; + private List docValueFields; private Set scriptFields; private HighlightBuilder highlightBuilder; private FetchSourceContext fetchSourceContext; @@ -134,7 +137,18 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject { version = in.readBoolean(); trackScores = in.readBoolean(); storedFieldsContext = in.readOptionalWriteable(StoredFieldsContext::new); - docValueFields = (List) in.readGenericValue(); + if (in.getVersion().before(Version.V_7_0_0_alpha1)) { // TODO: change to 6.4.0 after backport + List fieldList = (List) in.readGenericValue(); + if (fieldList == null) { + docValueFields = null; + } else { + docValueFields = fieldList.stream() + .map(field -> new FieldAndFormat(field, null)) + .collect(Collectors.toList()); + } + } else { + docValueFields = in.readBoolean() ? in.readList(FieldAndFormat::new) : null; + } if (in.readBoolean()) { int size = in.readVInt(); scriptFields = new HashSet<>(size); @@ -174,7 +188,16 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject { out.writeBoolean(version); out.writeBoolean(trackScores); out.writeOptionalWriteable(storedFieldsContext); - out.writeGenericValue(docValueFields); + if (out.getVersion().before(Version.V_7_0_0_alpha1)) { // TODO: change to 6.4.0 after backport + out.writeGenericValue(docValueFields == null + ? null + : docValueFields.stream().map(ff -> ff.field).collect(Collectors.toList())); + } else { + out.writeBoolean(docValueFields != null); + if (docValueFields != null) { + out.writeList(docValueFields); + } + } boolean hasScriptFields = scriptFields != null; out.writeBoolean(hasScriptFields); if (hasScriptFields) { @@ -248,7 +271,9 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject { out.writeBoolean(version); out.writeBoolean(trackScores); out.writeOptionalWriteable(storedFieldsContext); - out.writeGenericValue(docValueFields); + out.writeGenericValue(docValueFields == null + ? null + : docValueFields.stream().map(ff -> ff.field).collect(Collectors.toList())); boolean hasScriptFields = scriptFields != null; out.writeBoolean(hasScriptFields); if (hasScriptFields) { @@ -390,14 +415,14 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject { /** * Gets the docvalue fields. */ - public List getDocValueFields() { + public List getDocValueFields() { return docValueFields; } /** * Sets the stored fields to load from the docvalue and return. */ - public InnerHitBuilder setDocValueFields(List docValueFields) { + public InnerHitBuilder setDocValueFields(List docValueFields) { this.docValueFields = docValueFields; return this; } @@ -405,14 +430,21 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject { /** * Adds a field to load from the docvalue and return. */ - public InnerHitBuilder addDocValueField(String field) { + public InnerHitBuilder addDocValueField(String field, String format) { if (docValueFields == null) { docValueFields = new ArrayList<>(); } - docValueFields.add(field); + docValueFields.add(new FieldAndFormat(field, null)); return this; } + /** + * Adds a field to load from doc values and return. + */ + public InnerHitBuilder addDocValueField(String field) { + return addDocValueField(field, null); + } + public Set getScriptFields() { return scriptFields; } @@ -489,8 +521,15 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject { } if (docValueFields != null) { builder.startArray(SearchSourceBuilder.DOCVALUE_FIELDS_FIELD.getPreferredName()); - for (String docValueField : docValueFields) { - builder.value(docValueField); + for (FieldAndFormat docValueField : docValueFields) { + if (docValueField.format == null) { + builder.value(docValueField.field); + } else { + builder.startObject() + .field("field", docValueField.field) + .field("format", docValueField.format) + .endObject(); + } } builder.endArray(); } diff --git a/server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java b/server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java index 3098dc03a8c..4074d1a8fa1 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/search/RestSearchAction.java @@ -214,7 +214,7 @@ public class RestSearchAction extends BaseRestHandler { if (Strings.hasText(sDocValueFields)) { String[] sFields = Strings.splitStringByCommaToArray(sDocValueFields); for (String field : sFields) { - searchSourceBuilder.docValueField(field); + searchSourceBuilder.docValueField(field, null); } } } diff --git a/server/src/main/java/org/elasticsearch/search/DocValueFormat.java b/server/src/main/java/org/elasticsearch/search/DocValueFormat.java index 1824f17941b..8677370fc99 100644 --- a/server/src/main/java/org/elasticsearch/search/DocValueFormat.java +++ b/server/src/main/java/org/elasticsearch/search/DocValueFormat.java @@ -49,17 +49,17 @@ public interface DocValueFormat extends NamedWriteable { /** Format a long value. This is used by terms and histogram aggregations * to format keys for fields that use longs as a doc value representation * such as the {@code long} and {@code date} fields. */ - String format(long value); + Object format(long value); /** Format a double value. This is used by terms and stats aggregations * to format keys for fields that use numbers as a doc value representation * such as the {@code long}, {@code double} or {@code date} fields. */ - String format(double value); + Object format(double value); /** Format a binary value. This is used by terms aggregations to format * keys for fields that use binary doc value representations such as the * {@code keyword} and {@code ip} fields. */ - String format(BytesRef value); + Object format(BytesRef value); /** Parse a value that was formatted with {@link #format(long)} back to the * original long value. */ @@ -85,13 +85,13 @@ public interface DocValueFormat extends NamedWriteable { } @Override - public String format(long value) { - return Long.toString(value); + public Long format(long value) { + return value; } @Override - public String format(double value) { - return Double.toString(value); + public Double format(double value) { + return value; } @Override @@ -235,13 +235,13 @@ public interface DocValueFormat extends NamedWriteable { } @Override - public String format(long value) { - return java.lang.Boolean.valueOf(value != 0).toString(); + public Boolean format(long value) { + return java.lang.Boolean.valueOf(value != 0); } @Override - public String format(double value) { - return java.lang.Boolean.valueOf(value != 0).toString(); + public Boolean format(double value) { + return java.lang.Boolean.valueOf(value != 0); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java index c72e9d22dc0..bb391f21f1e 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregationBuilder.java @@ -407,8 +407,8 @@ public class DateHistogramAggregationBuilder extends ValuesSourceAggregationBuil final long high = nextTransition; final DocValueFormat format = ft.docValueFormat(null, null); - final String formattedLow = format.format(low); - final String formattedHigh = format.format(high); + final Object formattedLow = format.format(low); + final Object formattedHigh = format.format(high); if (ft.isFieldWithinQuery(reader, formattedLow, formattedHigh, true, false, tz, null, context) == Relation.WITHIN) { // All values in this reader have the same offset despite daylight saving times. diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalDateHistogram.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalDateHistogram.java index dfa12db0cd3..84dec2c983e 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalDateHistogram.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalDateHistogram.java @@ -107,7 +107,7 @@ public final class InternalDateHistogram extends InternalMultiBucketAggregation< @Override public String getKeyAsString() { - return format.format(key); + return format.format(key).toString(); } @Override @@ -138,7 +138,7 @@ public final class InternalDateHistogram extends InternalMultiBucketAggregation< @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - String keyAsString = format.format(key); + String keyAsString = format.format(key).toString(); if (keyed) { builder.startObject(keyAsString); } else { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalHistogram.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalHistogram.java index b3516b04dfc..1831e012a31 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalHistogram.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalHistogram.java @@ -103,7 +103,7 @@ public final class InternalHistogram extends InternalMultiBucketAggregation @Override public String getKeyAsString() { - return format.format(term); + return format.format(term).toString(); } @Override @@ -90,7 +90,7 @@ public class LongTerms extends InternalMappedTerms protected final XContentBuilder keyToXContent(XContentBuilder builder) throws IOException { builder.field(CommonFields.KEY.getPreferredName(), term); if (format != DocValueFormat.RAW) { - builder.field(CommonFields.KEY_AS_STRING.getPreferredName(), format.format(term)); + builder.field(CommonFields.KEY_AS_STRING.getPreferredName(), format.format(term).toString()); } return builder; } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTerms.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTerms.java index 11d0b40c7ce..4971f74f03d 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTerms.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTerms.java @@ -80,7 +80,7 @@ public class StringTerms extends InternalMappedTerms> sorts = null; private HighlightBuilder highlightBuilder; private StoredFieldsContext storedFieldsContext; - private List fieldDataFields; + private List docValueFields; private Set scriptFields; private FetchSourceContext fetchSourceContext; @@ -91,7 +92,7 @@ public class TopHitsAggregationBuilder extends AbstractAggregationBuilder(clone.fieldDataFields); + this.docValueFields = clone.docValueFields == null ? null : new ArrayList<>(clone.docValueFields); this.scriptFields = clone.scriptFields == null ? null : new HashSet<>(clone.scriptFields); this.fetchSourceContext = clone.fetchSourceContext == null ? null : new FetchSourceContext(clone.fetchSourceContext.fetchSource(), clone.fetchSourceContext.includes(), @@ -112,9 +113,9 @@ public class TopHitsAggregationBuilder extends AbstractAggregationBuilder(size); + docValueFields = new ArrayList<>(size); for (int i = 0; i < size; i++) { - fieldDataFields.add(in.readString()); + docValueFields.add(new FieldAndFormat(in)); } } storedFieldsContext = in.readOptionalWriteable(StoredFieldsContext::new); @@ -143,12 +144,12 @@ public class TopHitsAggregationBuilder extends AbstractAggregationBuilder(); + if (docValueFields == null) { + docValueFields = new ArrayList<>(); } - fieldDataFields.add(fieldDataField); + docValueFields.add(new FieldAndFormat(docValueField, format)); return this; } /** - * Adds fields to load from the field data cache and return as part of + * Adds a field to load from doc values and return as part of * the search request. */ - public TopHitsAggregationBuilder fieldDataFields(List fieldDataFields) { - if (fieldDataFields == null) { - throw new IllegalArgumentException("[fieldDataFields] must not be null: [" + name + "]"); - } - if (this.fieldDataFields == null) { - this.fieldDataFields = new ArrayList<>(); - } - this.fieldDataFields.addAll(fieldDataFields); - return this; + public TopHitsAggregationBuilder docValueField(String docValueField) { + return docValueField(docValueField, null); } /** * Gets the field-data fields. */ - public List fieldDataFields() { - return fieldDataFields; + public List fieldDataFields() { + return docValueFields; } /** @@ -587,7 +581,7 @@ public class TopHitsAggregationBuilder extends AbstractAggregationBuilder fieldDataFields = new ArrayList<>(); while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { - if (token == XContentParser.Token.VALUE_STRING) { - fieldDataFields.add(parser.text()); - } else { - throw new ParsingException(parser.getTokenLocation(), "Expected [" + XContentParser.Token.VALUE_STRING - + "] in [" + currentFieldName + "] but found [" + token + "]", parser.getTokenLocation()); - } + FieldAndFormat ff = FieldAndFormat.fromXContent(parser); + factory.docValueField(ff.field, ff.format); } - factory.fieldDataFields(fieldDataFields); } else if (SearchSourceBuilder.SORT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { List> sorts = SortBuilder.fromXContent(parser); factory.sorts(sorts); @@ -752,7 +746,7 @@ public class TopHitsAggregationBuilder extends AbstractAggregationBuilder sort; private final HighlightBuilder highlightBuilder; private final StoredFieldsContext storedFieldsContext; - private final List docValueFields; + private final List docValueFields; private final List scriptFields; private final FetchSourceContext fetchSourceContext; TopHitsAggregatorFactory(String name, int from, int size, boolean explain, boolean version, boolean trackScores, Optional sort, HighlightBuilder highlightBuilder, StoredFieldsContext storedFieldsContext, - List docValueFields, List scriptFields, FetchSourceContext fetchSourceContext, + List docValueFields, List scriptFields, FetchSourceContext fetchSourceContext, SearchContext context, AggregatorFactory parent, AggregatorFactories.Builder subFactories, Map metaData) throws IOException { super(name, context, parent, subFactories, metaData); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/InternalSimpleValue.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/InternalSimpleValue.java index 4fa4f1f6c86..2eac04a9581 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/InternalSimpleValue.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/InternalSimpleValue.java @@ -85,7 +85,7 @@ public class InternalSimpleValue extends InternalNumericMetricsAggregation.Singl boolean hasValue = !(Double.isInfinite(value) || Double.isNaN(value)); builder.field(CommonFields.VALUE.getPreferredName(), hasValue ? value : null); if (hasValue && format != DocValueFormat.RAW) { - builder.field(CommonFields.VALUE_AS_STRING.getPreferredName(), format.format(value)); + builder.field(CommonFields.VALUE_AS_STRING.getPreferredName(), format.format(value).toString()); } return builder; } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/InternalBucketMetricValue.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/InternalBucketMetricValue.java index 76a240d3178..a7ef024028f 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/InternalBucketMetricValue.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/InternalBucketMetricValue.java @@ -108,7 +108,7 @@ public class InternalBucketMetricValue extends InternalNumericMetricsAggregation boolean hasValue = !Double.isInfinite(value); builder.field(CommonFields.VALUE.getPreferredName(), hasValue ? value : null); if (hasValue && format != DocValueFormat.RAW) { - builder.field(CommonFields.VALUE_AS_STRING.getPreferredName(), format.format(value)); + builder.field(CommonFields.VALUE_AS_STRING.getPreferredName(), format.format(value).toString()); } builder.startArray(KEYS_FIELD.getPreferredName()); for (String key : keys) { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/percentile/InternalPercentilesBucket.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/percentile/InternalPercentilesBucket.java index 42a8b28a51a..5d13638f70a 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/percentile/InternalPercentilesBucket.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/percentile/InternalPercentilesBucket.java @@ -97,7 +97,7 @@ public class InternalPercentilesBucket extends InternalNumericMetricsAggregation @Override public String percentileAsString(double percent) { - return format.format(percentile(percent)); + return format.format(percentile(percent)).toString(); } DocValueFormat formatter() { diff --git a/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java index c4a6b3da6b1..12e6f7c3f95 100644 --- a/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java @@ -47,6 +47,7 @@ import org.elasticsearch.search.aggregations.AggregatorFactories; import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; import org.elasticsearch.search.collapse.CollapseBuilder; import org.elasticsearch.search.fetch.StoredFieldsContext; +import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext.FieldAndFormat; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilder; import org.elasticsearch.search.internal.SearchContext; @@ -64,6 +65,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.stream.Collectors; import static org.elasticsearch.index.query.AbstractQueryBuilder.parseInnerQueryBuilder; @@ -162,7 +164,7 @@ public final class SearchSourceBuilder implements Writeable, ToXContentObject, R private int terminateAfter = SearchContext.DEFAULT_TERMINATE_AFTER; private StoredFieldsContext storedFieldsContext; - private List docValueFields; + private List docValueFields; private List scriptFields; private FetchSourceContext fetchSourceContext; @@ -197,7 +199,22 @@ public final class SearchSourceBuilder implements Writeable, ToXContentObject, R aggregations = in.readOptionalWriteable(AggregatorFactories.Builder::new); explain = in.readOptionalBoolean(); fetchSourceContext = in.readOptionalWriteable(FetchSourceContext::new); - docValueFields = (List) in.readGenericValue(); + if (in.getVersion().before(Version.V_7_0_0_alpha1)) { // TODO: change to 6.4.0 after backport + List dvFields = (List) in.readGenericValue(); + if (dvFields == null) { + docValueFields = null; + } else { + docValueFields = dvFields.stream() + .map(field -> new FieldAndFormat(field, null)) + .collect(Collectors.toList()); + } + } else { + if (in.readBoolean()) { + docValueFields = in.readList(FieldAndFormat::new); + } else { + docValueFields = null; + } + } storedFieldsContext = in.readOptionalWriteable(StoredFieldsContext::new); from = in.readVInt(); highlightBuilder = in.readOptionalWriteable(HighlightBuilder::new); @@ -246,7 +263,16 @@ public final class SearchSourceBuilder implements Writeable, ToXContentObject, R out.writeOptionalWriteable(aggregations); out.writeOptionalBoolean(explain); out.writeOptionalWriteable(fetchSourceContext); - out.writeGenericValue(docValueFields); + if (out.getVersion().before(Version.V_7_0_0_alpha1)) { // TODO: change to 6.4.0 after backport + out.writeGenericValue(docValueFields == null + ? null + : docValueFields.stream().map(ff -> ff.field).collect(Collectors.toList())); + } else { + out.writeBoolean(docValueFields != null); + if (docValueFields != null) { + out.writeList(docValueFields); + } + } out.writeOptionalWriteable(storedFieldsContext); out.writeVInt(from); out.writeOptionalWriteable(highlightBuilder); @@ -764,22 +790,30 @@ public final class SearchSourceBuilder implements Writeable, ToXContentObject, R /** * Gets the docvalue fields. */ - public List docValueFields() { + public List docValueFields() { return docValueFields; } /** - * Adds a field to load from the docvalue and return as part of the + * Adds a field to load from the doc values and return as part of the * search request. */ - public SearchSourceBuilder docValueField(String name) { + public SearchSourceBuilder docValueField(String name, @Nullable String format) { if (docValueFields == null) { docValueFields = new ArrayList<>(); } - docValueFields.add(name); + docValueFields.add(new FieldAndFormat(name, format)); return this; } + /** + * Adds a field to load from the doc values and return as part of the + * search request. + */ + public SearchSourceBuilder docValueField(String name) { + return docValueField(name, null); + } + /** * Adds a script field under the given name with the provided script. * @@ -1076,12 +1110,7 @@ public final class SearchSourceBuilder implements Writeable, ToXContentObject, R } else if (DOCVALUE_FIELDS_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { docValueFields = new ArrayList<>(); while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { - if (token == XContentParser.Token.VALUE_STRING) { - docValueFields.add(parser.text()); - } else { - throw new ParsingException(parser.getTokenLocation(), "Expected [" + XContentParser.Token.VALUE_STRING + - "] in [" + currentFieldName + "] but found [" + token + "]", parser.getTokenLocation()); - } + docValueFields.add(FieldAndFormat.fromXContent(parser)); } } else if (INDICES_BOOST_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { @@ -1177,8 +1206,13 @@ public final class SearchSourceBuilder implements Writeable, ToXContentObject, R if (docValueFields != null) { builder.startArray(DOCVALUE_FIELDS_FIELD.getPreferredName()); - for (String docValueField : docValueFields) { - builder.value(docValueField); + for (FieldAndFormat docValueField : docValueFields) { + builder.startObject() + .field("field", docValueField.field); + if (docValueField.format != null) { + builder.field("format", docValueField.format); + } + builder.endObject(); } builder.endArray(); } diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsContext.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsContext.java index 325d28e4592..e620e25943c 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsContext.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsContext.java @@ -18,23 +18,111 @@ */ package org.elasticsearch.search.fetch.subphase; +import org.elasticsearch.Version; +import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.XContent; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.XContentParser.Token; + +import java.io.IOException; import java.util.List; +import java.util.Objects; /** * All the required context to pull a field from the doc values. */ public class DocValueFieldsContext { - private final List fields; + public static final String USE_DEFAULT_FORMAT = "use_field_mapping"; - public DocValueFieldsContext(List fields) { + /** + * Wrapper around a field name and the format that should be used to + * display values of this field. + */ + public static final class FieldAndFormat implements Writeable { + + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("script", + a -> new FieldAndFormat((String) a[0], (String) a[1])); + static { + PARSER.declareString(ConstructingObjectParser.constructorArg(), new ParseField("field")); + PARSER.declareStringOrNull(ConstructingObjectParser.optionalConstructorArg(), new ParseField("format")); + } + + /** + * Parse a {@link FieldAndFormat} from some {@link XContent}. + */ + public static FieldAndFormat fromXContent(XContentParser parser) throws IOException { + Token token = parser.currentToken(); + if (token.isValue()) { + return new FieldAndFormat(parser.text(), null); + } else { + return PARSER.apply(parser, null); + } + } + + /** The name of the field. */ + public final String field; + + /** The format of the field, or {@code null} if defaults should be used. */ + public final String format; + + /** Sole constructor. */ + public FieldAndFormat(String field, @Nullable String format) { + this.field = Objects.requireNonNull(field); + this.format = format; + } + + /** Serialization constructor. */ + public FieldAndFormat(StreamInput in) throws IOException { + this.field = in.readString(); + if (in.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { // TODO: change to 6.4.0 after backport + format = in.readOptionalString(); + } else { + format = null; + } + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(field); + if (out.getVersion().onOrAfter(Version.V_7_0_0_alpha1)) { // TODO: change to 6.4.0 after backport + out.writeOptionalString(format); + } + } + + @Override + public int hashCode() { + int h = field.hashCode(); + h = 31 * h + Objects.hashCode(format); + return h; + } + + @Override + public boolean equals(Object obj) { + if (obj == null || getClass() != obj.getClass()) { + return false; + } + FieldAndFormat other = (FieldAndFormat) obj; + return field.equals(other.field) && Objects.equals(format, other.format); + } + + } + + private final List fields; + + public DocValueFieldsContext(List fields) { this.fields = fields; } /** * Returns the required docvalue fields */ - public List fields() { + public List fields() { return this.fields; } } diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java index 60def08c891..a1562e118fb 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/DocValueFieldsFetchSubPhase.java @@ -20,19 +20,32 @@ package org.elasticsearch.search.fetch.subphase; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.ReaderUtil; +import org.apache.lucene.index.SortedNumericDocValues; import org.elasticsearch.common.document.DocumentField; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.index.fielddata.AtomicFieldData; +import org.elasticsearch.index.fielddata.AtomicNumericFieldData; +import org.elasticsearch.index.fielddata.IndexFieldData; +import org.elasticsearch.index.fielddata.IndexNumericFieldData; import org.elasticsearch.index.fielddata.ScriptDocValues; +import org.elasticsearch.index.fielddata.SortedBinaryDocValues; +import org.elasticsearch.index.fielddata.SortedNumericDoubleValues; import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.search.DocValueFormat; import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.fetch.FetchSubPhase; +import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext.FieldAndFormat; import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; +import java.util.List; +import java.util.Objects; /** * Query sub phase which pulls data from doc values @@ -41,6 +54,8 @@ import java.util.HashMap; */ public final class DocValueFieldsFetchSubPhase implements FetchSubPhase { + private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(DocValueFieldsFetchSubPhase.class)); + @Override public void hitsExecute(SearchContext context, SearchHit[] hits) throws IOException { @@ -48,9 +63,10 @@ public final class DocValueFieldsFetchSubPhase implements FetchSubPhase { // retrieve the `doc_value` associated with the collapse field String name = context.collapse().getFieldType().name(); if (context.docValueFieldsContext() == null) { - context.docValueFieldsContext(new DocValueFieldsContext(Collections.singletonList(name))); - } else if (context.docValueFieldsContext().fields().contains(name) == false) { - context.docValueFieldsContext().fields().add(name); + context.docValueFieldsContext(new DocValueFieldsContext( + Collections.singletonList(new FieldAndFormat(name, DocValueFieldsContext.USE_DEFAULT_FORMAT)))); + } else if (context.docValueFieldsContext().fields().stream().map(ff -> ff.field).anyMatch(name::equals) == false) { + context.docValueFieldsContext().fields().add(new FieldAndFormat(name, DocValueFieldsContext.USE_DEFAULT_FORMAT)); } } @@ -59,24 +75,51 @@ public final class DocValueFieldsFetchSubPhase implements FetchSubPhase { } hits = hits.clone(); // don't modify the incoming hits - Arrays.sort(hits, (a, b) -> Integer.compare(a.docId(), b.docId())); + Arrays.sort(hits, Comparator.comparingInt(SearchHit::docId)); - for (String field : context.docValueFieldsContext().fields()) { + for (FieldAndFormat fieldAndFormat : context.docValueFieldsContext().fields()) { + String field = fieldAndFormat.field; MappedFieldType fieldType = context.mapperService().fullName(field); if (fieldType != null) { + final IndexFieldData indexFieldData = context.getForField(fieldType); + final DocValueFormat format; + if (fieldAndFormat.format == null) { + DEPRECATION_LOGGER.deprecated("Doc-value field [" + fieldAndFormat.field + "] is not using a format. The output will " + + "change in 7.0 when doc value fields get formatted based on mappings by default. It is recommended to pass " + + "[format={}] with the doc value field in order to opt in for the future behaviour and ease the migration to " + + "7.0.", DocValueFieldsContext.USE_DEFAULT_FORMAT); + format = null; + } else { + String formatDesc = fieldAndFormat.format; + if (Objects.equals(formatDesc, DocValueFieldsContext.USE_DEFAULT_FORMAT)) { + formatDesc = null; + } + format = fieldType.docValueFormat(formatDesc, null); + } LeafReaderContext subReaderContext = null; AtomicFieldData data = null; - ScriptDocValues values = null; + ScriptDocValues scriptValues = null; // legacy + SortedBinaryDocValues binaryValues = null; // binary / string / ip fields + SortedNumericDocValues longValues = null; // int / date fields + SortedNumericDoubleValues doubleValues = null; // floating-point fields for (SearchHit hit : hits) { // if the reader index has changed we need to get a new doc values reader instance if (subReaderContext == null || hit.docId() >= subReaderContext.docBase + subReaderContext.reader().maxDoc()) { int readerIndex = ReaderUtil.subIndex(hit.docId(), context.searcher().getIndexReader().leaves()); subReaderContext = context.searcher().getIndexReader().leaves().get(readerIndex); - data = context.getForField(fieldType).load(subReaderContext); - values = data.getScriptValues(); + data = indexFieldData.load(subReaderContext); + if (format == null) { + scriptValues = data.getScriptValues(); + } else if (indexFieldData instanceof IndexNumericFieldData) { + if (((IndexNumericFieldData) indexFieldData).getNumericType().isFloatingPoint()) { + doubleValues = ((AtomicNumericFieldData) data).getDoubleValues(); + } else { + longValues = ((AtomicNumericFieldData) data).getLongValues(); + } + } else { + binaryValues = data.getBytesValues(); + } } - int subDocId = hit.docId() - subReaderContext.docBase; - values.setNextDocId(subDocId); if (hit.fieldsOrNull() == null) { hit.fields(new HashMap<>(2)); } @@ -85,7 +128,33 @@ public final class DocValueFieldsFetchSubPhase implements FetchSubPhase { hitField = new DocumentField(field, new ArrayList<>(2)); hit.getFields().put(field, hitField); } - hitField.getValues().addAll(values); + final List values = hitField.getValues(); + + int subDocId = hit.docId() - subReaderContext.docBase; + if (scriptValues != null) { + scriptValues.setNextDocId(subDocId); + values.addAll(scriptValues); + } else if (binaryValues != null) { + if (binaryValues.advanceExact(subDocId)) { + for (int i = 0, count = binaryValues.docValueCount(); i < count; ++i) { + values.add(format.format(binaryValues.nextValue())); + } + } + } else if (longValues != null) { + if (longValues.advanceExact(subDocId)) { + for (int i = 0, count = longValues.docValueCount(); i < count; ++i) { + values.add(format.format(longValues.nextValue())); + } + } + } else if (doubleValues != null) { + if (doubleValues.advanceExact(subDocId)) { + for (int i = 0, count = doubleValues.docValueCount(); i < count; ++i) { + values.add(format.format(doubleValues.nextValue())); + } + } + } else { + throw new AssertionError("Unreachable code"); + } } } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldTypeTests.java index d119a27f22e..8621e775838 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldTypeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldTypeTests.java @@ -36,8 +36,8 @@ public class BooleanFieldTypeTests extends FieldTypeTestCase { public void testValueFormat() { MappedFieldType ft = createDefaultFieldType(); - assertEquals("false", ft.docValueFormat(null, null).format(0)); - assertEquals("true", ft.docValueFormat(null, null).format(1)); + assertEquals(false, ft.docValueFormat(null, null).format(0)); + assertEquals(true, ft.docValueFormat(null, null).format(1)); } public void testValueForSearch() { diff --git a/server/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java index a4e68561662..a2068a666f4 100644 --- a/server/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java @@ -32,6 +32,8 @@ import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptType; import org.elasticsearch.search.SearchModule; import org.elasticsearch.search.builder.SearchSourceBuilder; +import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; +import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext.FieldAndFormat; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.fetch.subphase.highlight.HighlightBuilderTests; import org.elasticsearch.search.internal.ShardSearchLocalRequest; @@ -147,7 +149,9 @@ public class InnerHitBuilderTests extends ESTestCase { if (randomBoolean()) { innerHits.setStoredFieldNames(randomListStuff(16, () -> randomAlphaOfLengthBetween(1, 16))); } - innerHits.setDocValueFields(randomListStuff(16, () -> randomAlphaOfLengthBetween(1, 16))); + innerHits.setDocValueFields(randomListStuff(16, + () -> new FieldAndFormat(randomAlphaOfLengthBetween(1, 16), + randomBoolean() ? null : DocValueFieldsContext.USE_DEFAULT_FORMAT))); // Random script fields deduped on their field name. Map scriptFields = new HashMap<>(); for (SearchSourceBuilder.ScriptField field: randomListStuff(16, InnerHitBuilderTests::randomScript)) { @@ -187,9 +191,9 @@ public class InnerHitBuilderTests extends ESTestCase { modifiers.add(() -> copy.setName(randomValueOtherThan(copy.getName(), () -> randomAlphaOfLengthBetween(1, 16)))); modifiers.add(() -> { if (randomBoolean()) { - copy.setDocValueFields(randomValueOtherThan(copy.getDocValueFields(), () -> { - return randomListStuff(16, () -> randomAlphaOfLengthBetween(1, 16)); - })); + copy.setDocValueFields(randomValueOtherThan(copy.getDocValueFields(), + () -> randomListStuff(16, () -> new FieldAndFormat(randomAlphaOfLengthBetween(1, 16), + randomBoolean() ? null : DocValueFieldsContext.USE_DEFAULT_FORMAT)))); } else { copy.addDocValueField(randomAlphaOfLengthBetween(1, 16)); } diff --git a/server/src/test/java/org/elasticsearch/search/DocValueFormatTests.java b/server/src/test/java/org/elasticsearch/search/DocValueFormatTests.java index 7bf5308eb63..e5cfbf98b3d 100644 --- a/server/src/test/java/org/elasticsearch/search/DocValueFormatTests.java +++ b/server/src/test/java/org/elasticsearch/search/DocValueFormatTests.java @@ -85,20 +85,20 @@ public class DocValueFormatTests extends ESTestCase { } public void testRawFormat() { - assertEquals("0", DocValueFormat.RAW.format(0)); - assertEquals("-1", DocValueFormat.RAW.format(-1)); - assertEquals("1", DocValueFormat.RAW.format(1)); + assertEquals(0L, DocValueFormat.RAW.format(0)); + assertEquals(-1L, DocValueFormat.RAW.format(-1)); + assertEquals(1L, DocValueFormat.RAW.format(1)); - assertEquals("0.0", DocValueFormat.RAW.format(0d)); - assertEquals("0.5", DocValueFormat.RAW.format(.5d)); - assertEquals("-1.0", DocValueFormat.RAW.format(-1d)); + assertEquals(0d, DocValueFormat.RAW.format(0d)); + assertEquals(.5d, DocValueFormat.RAW.format(.5d)); + assertEquals(-1d, DocValueFormat.RAW.format(-1d)); assertEquals("abc", DocValueFormat.RAW.format(new BytesRef("abc"))); } public void testBooleanFormat() { - assertEquals("false", DocValueFormat.BOOLEAN.format(0)); - assertEquals("true", DocValueFormat.BOOLEAN.format(1)); + assertEquals(false, DocValueFormat.BOOLEAN.format(0)); + assertEquals(true, DocValueFormat.BOOLEAN.format(1)); } public void testIpFormat() { diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/InternalSumTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/InternalSumTests.java index 884f9bfbe0d..aa9d25af49e 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/InternalSumTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/InternalSumTests.java @@ -37,7 +37,7 @@ public class InternalSumTests extends InternalAggregationTestCase { @Override protected InternalSum createTestInstance(String name, List pipelineAggregators, Map metaData) { double value = frequently() ? randomDouble() : randomFrom(Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, Double.NaN); - DocValueFormat formatter = randomFrom(new DocValueFormat.Decimal("###.##"), DocValueFormat.BOOLEAN, DocValueFormat.RAW); + DocValueFormat formatter = randomFrom(new DocValueFormat.Decimal("###.##"), DocValueFormat.RAW); return new InternalSum(name, value, formatter, pipelineAggregators, metaData); } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java index 4f8493c0b00..952eb22848e 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java @@ -583,7 +583,7 @@ public class TopHitsIT extends ESIntegTestCase { .highlighter(new HighlightBuilder().field("text")) .explain(true) .storedField("text") - .fieldDataField("field1") + .docValueField("field1") .scriptField("script", new Script(ScriptType.INLINE, MockScriptEngine.NAME, "5", Collections.emptyMap())) .fetchSource("text", null) .version(true) @@ -865,7 +865,7 @@ public class TopHitsIT extends ESIntegTestCase { .addAggregation( nested("to-comments", "comments").subAggregation( topHits("top-comments").size(1).highlighter(new HighlightBuilder().field(hlField)).explain(true) - .fieldDataField("comments.user") + .docValueField("comments.user") .scriptField("script", new Script(ScriptType.INLINE, MockScriptEngine.NAME, "5", Collections.emptyMap())).fetchSource("comments.message", null) .version(true).sort("comments.date", SortOrder.ASC))).get(); assertHitCount(searchResponse, 2); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsTests.java index e2ef28480fa..4d2331b86f2 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/metrics/TopHitsTests.java @@ -81,7 +81,7 @@ public class TopHitsTests extends BaseAggregationTestCase(searchResponse.getHits().getAt(0).getFields().keySet()); + assertThat(fields, equalTo(newHashSet("byte_field", "short_field", "integer_field", "long_field", + "float_field", "double_field", "date_field", "boolean_field", "text_field", "keyword_field", + "ip_field"))); + + assertThat(searchResponse.getHits().getAt(0).getFields().get("byte_field").getValue().toString(), equalTo("1")); + assertThat(searchResponse.getHits().getAt(0).getFields().get("short_field").getValue().toString(), equalTo("2")); + assertThat(searchResponse.getHits().getAt(0).getFields().get("integer_field").getValue(), equalTo((Object) 3L)); + assertThat(searchResponse.getHits().getAt(0).getFields().get("long_field").getValue(), equalTo((Object) 4L)); + assertThat(searchResponse.getHits().getAt(0).getFields().get("float_field").getValue(), equalTo((Object) 5.0)); + assertThat(searchResponse.getHits().getAt(0).getFields().get("double_field").getValue(), equalTo((Object) 6.0d)); + assertThat(searchResponse.getHits().getAt(0).getFields().get("date_field").getValue(), + equalTo(Joda.forPattern("dateOptionalTime").printer().print(date))); + assertThat(searchResponse.getHits().getAt(0).getFields().get("boolean_field").getValue(), equalTo((Object) true)); + assertThat(searchResponse.getHits().getAt(0).getFields().get("text_field").getValue(), equalTo("foo")); + assertThat(searchResponse.getHits().getAt(0).getFields().get("keyword_field").getValue(), equalTo("foo")); + assertThat(searchResponse.getHits().getAt(0).getFields().get("ip_field").getValue(), equalTo("::1")); + + builder = client().prepareSearch().setQuery(matchAllQuery()) + .addDocValueField("byte_field", "#.0") + .addDocValueField("short_field", "#.0") + .addDocValueField("integer_field", "#.0") + .addDocValueField("long_field", "#.0") + .addDocValueField("float_field", "#.0") + .addDocValueField("double_field", "#.0") + .addDocValueField("date_field", "epoch_millis"); + searchResponse = builder.execute().actionGet(); + + assertThat(searchResponse.getHits().getTotalHits(), equalTo(1L)); + assertThat(searchResponse.getHits().getHits().length, equalTo(1)); + fields = new HashSet<>(searchResponse.getHits().getAt(0).getFields().keySet()); + assertThat(fields, equalTo(newHashSet("byte_field", "short_field", "integer_field", "long_field", + "float_field", "double_field", "date_field"))); + + assertThat(searchResponse.getHits().getAt(0).getFields().get("byte_field").getValue(), equalTo("1.0")); + assertThat(searchResponse.getHits().getAt(0).getFields().get("short_field").getValue(), equalTo("2.0")); + assertThat(searchResponse.getHits().getAt(0).getFields().get("integer_field").getValue(), equalTo("3.0")); + assertThat(searchResponse.getHits().getAt(0).getFields().get("long_field").getValue(), equalTo("4.0")); + assertThat(searchResponse.getHits().getAt(0).getFields().get("float_field").getValue(), equalTo("5.0")); + assertThat(searchResponse.getHits().getAt(0).getFields().get("double_field").getValue(), equalTo("6.0")); + assertThat(searchResponse.getHits().getAt(0).getFields().get("date_field").getValue(), + equalTo(Joda.forPattern("epoch_millis").printer().print(date))); } public void testScriptFields() throws Exception { diff --git a/x-pack/docs/en/sql/endpoints/translate.asciidoc b/x-pack/docs/en/sql/endpoints/translate.asciidoc index 27821141130..9c1d71af5d3 100644 --- a/x-pack/docs/en/sql/endpoints/translate.asciidoc +++ b/x-pack/docs/en/sql/endpoints/translate.asciidoc @@ -23,8 +23,14 @@ Which returns: { "size" : 10, "docvalue_fields" : [ - "page_count", - "release_date" + { + "field": "page_count", + "format": "use_field_mapping" + }, + { + "field": "release_date", + "format": "epoch_millis" + } ], "_source": { "includes": [ diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/MockClientBuilder.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/MockClientBuilder.java index 437a965dcf0..61fc73d1641 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/MockClientBuilder.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/persistence/MockClientBuilder.java @@ -197,6 +197,7 @@ public class MockClientBuilder { when(builder.setFetchSource(anyBoolean())).thenReturn(builder); when(builder.setScroll(anyString())).thenReturn(builder); when(builder.addDocValueField(any(String.class))).thenReturn(builder); + when(builder.addDocValueField(any(String.class), any(String.class))).thenReturn(builder); when(builder.addSort(any(String.class), any(SortOrder.class))).thenReturn(builder); when(builder.setQuery(any())).thenReturn(builder); when(builder.setSize(anyInt())).thenReturn(builder); @@ -246,6 +247,7 @@ public class MockClientBuilder { when(builder.setSize(eq(size))).thenReturn(builder); when(builder.setFetchSource(eq(true))).thenReturn(builder); when(builder.addDocValueField(any(String.class))).thenReturn(builder); + when(builder.addDocValueField(any(String.class), any(String.class))).thenReturn(builder); when(builder.addSort(any(String.class), any(SortOrder.class))).thenReturn(builder); when(builder.get()).thenReturn(response); when(client.prepareSearch(eq(index))).thenReturn(builder); @@ -262,6 +264,7 @@ public class MockClientBuilder { when(builder.setSize(any(Integer.class))).thenReturn(builder); when(builder.setFetchSource(eq(true))).thenReturn(builder); when(builder.addDocValueField(any(String.class))).thenReturn(builder); + when(builder.addDocValueField(any(String.class), any(String.class))).thenReturn(builder); when(builder.addSort(any(String.class), any(SortOrder.class))).thenReturn(builder); when(builder.get()).thenReturn(response); when(client.prepareSearch(eq(index))).thenReturn(builder); diff --git a/x-pack/plugin/sql/sql-proto/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlTranslateResponseTests.java b/x-pack/plugin/sql/sql-proto/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlTranslateResponseTests.java index 061b70a55d9..76f73fada06 100644 --- a/x-pack/plugin/sql/sql-proto/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlTranslateResponseTests.java +++ b/x-pack/plugin/sql/sql-proto/src/test/java/org/elasticsearch/xpack/sql/plugin/SqlTranslateResponseTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.sql.plugin; import org.elasticsearch.search.builder.SearchSourceBuilder; +import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; import org.elasticsearch.test.AbstractStreamableTestCase; import java.io.IOException; @@ -18,7 +19,7 @@ public class SqlTranslateResponseTests extends AbstractStreamableTestCase sourceFields = new LinkedHashSet<>(); - final Set docFields = new LinkedHashSet<>(); + final Set docFields = new LinkedHashSet<>(); final Map scriptFields = new LinkedHashMap<>(); boolean trackScores = false; @@ -47,8 +49,8 @@ public class SqlSourceBuilder { /** * Retrieve the requested field from doc values (or fielddata) of the document */ - public void addDocField(String field) { - docFields.add(field); + public void addDocField(String field, String format) { + docFields.add(new FieldAndFormat(field, format)); } /** @@ -67,7 +69,8 @@ public class SqlSourceBuilder { if (!sourceFields.isEmpty()) { sourceBuilder.fetchSource(sourceFields.toArray(Strings.EMPTY_ARRAY), null); } - docFields.forEach(sourceBuilder::docValueField); + docFields.forEach(field -> sourceBuilder.docValueField(field.field, + field.format == null ? DocValueFieldsContext.USE_DEFAULT_FORMAT : field.format)); scriptFields.forEach(sourceBuilder::scriptField); } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java index 159127fb24c..66e17753054 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java @@ -5,12 +5,16 @@ */ package org.elasticsearch.xpack.sql.execution.search.extractor; +import org.elasticsearch.Version; import org.elasticsearch.common.Strings; import org.elasticsearch.common.document.DocumentField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.search.SearchHit; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; +import org.elasticsearch.xpack.sql.type.DataType; +import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; import org.joda.time.ReadableDateTime; import java.io.IOException; @@ -41,15 +45,17 @@ public class FieldHitExtractor implements HitExtractor { } private final String fieldName, hitName; + private final DataType dataType; private final boolean useDocValue; private final String[] path; - public FieldHitExtractor(String name, boolean useDocValue) { - this(name, useDocValue, null); + public FieldHitExtractor(String name, DataType dataType, boolean useDocValue) { + this(name, dataType, useDocValue, null); } - public FieldHitExtractor(String name, boolean useDocValue, String hitName) { + public FieldHitExtractor(String name, DataType dataType, boolean useDocValue, String hitName) { this.fieldName = name; + this.dataType = dataType; this.useDocValue = useDocValue; this.hitName = hitName; @@ -64,6 +70,16 @@ public class FieldHitExtractor implements HitExtractor { FieldHitExtractor(StreamInput in) throws IOException { fieldName = in.readString(); + if (in.getVersion().onOrAfter(Version.V_6_4_0)) { + String esType = in.readOptionalString(); + if (esType != null) { + dataType = DataType.fromEsType(esType); + } else { + dataType = null; + } + } else { + dataType = null; + } useDocValue = in.readBoolean(); hitName = in.readOptionalString(); path = sourcePath(fieldName, useDocValue, hitName); @@ -77,6 +93,9 @@ public class FieldHitExtractor implements HitExtractor { @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(fieldName); + if (out.getVersion().onOrAfter(Version.V_6_4_0)) { + out.writeOptionalString(dataType == null ? null : dataType.esType); + } out.writeBoolean(useDocValue); out.writeOptionalString(hitName); } @@ -117,6 +136,9 @@ public class FieldHitExtractor implements HitExtractor { if (values instanceof Map) { throw new SqlIllegalArgumentException("Objects (returned by [{}]) are not supported", fieldName); } + if (values instanceof String && dataType == DataType.DATE) { + return new DateTime(Long.parseLong(values.toString()), DateTimeZone.UTC); + } if (values instanceof Long || values instanceof Double || values instanceof String || values instanceof Boolean || values instanceof ReadableDateTime) { return values; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeProcessor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeProcessor.java index 6aa6b6a50e9..d135b8a0865 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeProcessor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeProcessor.java @@ -9,9 +9,11 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.expression.function.scalar.processor.runtime.Processor; +import org.joda.time.DateTime; import org.joda.time.DateTimeFieldType; import org.joda.time.DateTimeZone; import org.joda.time.ReadableDateTime; +import org.joda.time.ReadableInstant; import java.io.IOException; import java.util.Objects; @@ -78,15 +80,21 @@ public class DateTimeProcessor implements Processor { return null; } - if (!(l instanceof ReadableDateTime)) { - throw new SqlIllegalArgumentException("A date/time is required; received {}", l); + ReadableDateTime dt; + if (l instanceof String) { + // 6.4+ + final long millis = Long.parseLong(l.toString()); + dt = new DateTime(millis, DateTimeZone.forTimeZone(timeZone)); + } else if (l instanceof ReadableInstant) { + // 6.3- + dt = (ReadableDateTime) l; + if (!TimeZone.getTimeZone("UTC").equals(timeZone)) { + dt = dt.toDateTime().withZone(DateTimeZone.forTimeZone(timeZone)); + } + } else { + throw new SqlIllegalArgumentException("A string or a date is required; received {}", l); } - ReadableDateTime dt = (ReadableDateTime) l; - - if (!TimeZone.getTimeZone("UTC").equals(timeZone)) { - dt = dt.toDateTime().withZone(DateTimeZone.forTimeZone(timeZone)); - } return extractor.extract(dt); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java index bca180315d9..9f9c1bb21bb 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java @@ -173,7 +173,7 @@ public class QueryContainer { // reference methods // private FieldExtraction topHitFieldRef(FieldAttribute fieldAttr) { - return new SearchHitFieldRef(aliasName(fieldAttr), fieldAttr.field().hasDocValues()); + return new SearchHitFieldRef(aliasName(fieldAttr), fieldAttr.field().getDataType(), fieldAttr.field().hasDocValues()); } private Tuple nestedHitFieldRef(FieldAttribute attr) { @@ -184,7 +184,8 @@ public class QueryContainer { Query q = rewriteToContainNestedField(query, attr.location(), attr.nestedParent().name(), name, attr.field().hasDocValues()); - SearchHitFieldRef nestedFieldRef = new SearchHitFieldRef(name, attr.field().hasDocValues(), attr.parent().name()); + SearchHitFieldRef nestedFieldRef = new SearchHitFieldRef(name, attr.field().getDataType(), + attr.field().hasDocValues(), attr.parent().name()); nestedRefs.add(nestedFieldRef); return new Tuple<>(new QueryContainer(q, aggs, columns, aliases, pseudoFunctions, scalarFunctions, sort, limit), nestedFieldRef); diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/SearchHitFieldRef.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/SearchHitFieldRef.java index 6a7f24b447e..7f799108d28 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/SearchHitFieldRef.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/SearchHitFieldRef.java @@ -6,18 +6,21 @@ package org.elasticsearch.xpack.sql.querydsl.container; import org.elasticsearch.xpack.sql.execution.search.SqlSourceBuilder; +import org.elasticsearch.xpack.sql.type.DataType; public class SearchHitFieldRef extends FieldReference { private final String name; + private final DataType dataType; private final boolean docValue; private final String hitName; - public SearchHitFieldRef(String name, boolean useDocValueInsteadOfSource) { - this(name, useDocValueInsteadOfSource, null); + public SearchHitFieldRef(String name, DataType dataType, boolean useDocValueInsteadOfSource) { + this(name, dataType, useDocValueInsteadOfSource, null); } - public SearchHitFieldRef(String name, boolean useDocValueInsteadOfSource, String hitName) { + public SearchHitFieldRef(String name, DataType dataType, boolean useDocValueInsteadOfSource, String hitName) { this.name = name; + this.dataType = dataType; this.docValue = useDocValueInsteadOfSource; this.hitName = hitName; } @@ -31,6 +34,10 @@ public class SearchHitFieldRef extends FieldReference { return name; } + public DataType getDataType() { + return dataType; + } + public boolean useDocValue() { return docValue; } @@ -42,7 +49,8 @@ public class SearchHitFieldRef extends FieldReference { return; } if (docValue) { - sourceBuilder.addDocField(name); + String format = dataType == DataType.DATE ? "epoch_millis" : null; + sourceBuilder.addDocField(name, format); } else { sourceBuilder.addSourceField(name); } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/SqlLicenseIT.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/SqlLicenseIT.java index a97f66763a9..8988f70672a 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/SqlLicenseIT.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/SqlLicenseIT.java @@ -16,6 +16,7 @@ import org.elasticsearch.license.License; import org.elasticsearch.license.License.OperationMode; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.search.builder.SearchSourceBuilder; +import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.test.hamcrest.ElasticsearchAssertions; import org.elasticsearch.transport.Netty4Plugin; @@ -150,7 +151,8 @@ public class SqlLicenseIT extends AbstractLicensesIntegrationTestCase { SqlTranslateResponse response = client().prepareExecute(SqlTranslateAction.INSTANCE).query("SELECT * FROM test").get(); SearchSourceBuilder source = response.source(); - assertThat(source.docValueFields(), Matchers.contains("count")); + assertThat(source.docValueFields(), Matchers.contains( + new DocValueFieldsContext.FieldAndFormat("count", DocValueFieldsContext.USE_DEFAULT_FORMAT))); FetchSourceContext fetchSource = source.fetchSource(); assertThat(fetchSource.includes(), Matchers.arrayContaining("data")); } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/SqlTranslateActionIT.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/SqlTranslateActionIT.java index 5de9cfca97a..a4c440eb9df 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/SqlTranslateActionIT.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/action/SqlTranslateActionIT.java @@ -8,6 +8,7 @@ package org.elasticsearch.xpack.sql.action; import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.search.builder.SearchSourceBuilder; +import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.sort.SortBuilders; import org.elasticsearch.xpack.sql.plugin.SqlTranslateAction; @@ -35,7 +36,9 @@ public class SqlTranslateActionIT extends AbstractSqlIntegTestCase { FetchSourceContext fetch = source.fetchSource(); assertEquals(true, fetch.fetchSource()); assertArrayEquals(new String[] { "data" }, fetch.includes()); - assertEquals(singletonList("count"), source.docValueFields()); + assertEquals( + singletonList(new DocValueFieldsContext.FieldAndFormat("count", DocValueFieldsContext.USE_DEFAULT_FORMAT)), + source.docValueFields()); assertEquals(singletonList(SortBuilders.fieldSort("count")), source.sorts()); } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/SqlSourceBuilderTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/SqlSourceBuilderTests.java index 0d57ad97c98..6ee843c2c63 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/SqlSourceBuilderTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/SqlSourceBuilderTests.java @@ -24,8 +24,8 @@ public class SqlSourceBuilderTests extends ESTestCase { ssb.trackScores(); ssb.addSourceField("foo"); ssb.addSourceField("foo2"); - ssb.addDocField("bar"); - ssb.addDocField("bar2"); + ssb.addDocField("bar", null); + ssb.addDocField("bar2", null); final Script s = new Script("eggplant"); ssb.addScriptField("baz", s); final Script s2 = new Script("potato"); @@ -35,7 +35,7 @@ public class SqlSourceBuilderTests extends ESTestCase { assertTrue(source.trackScores()); FetchSourceContext fsc = source.fetchSource(); assertThat(Arrays.asList(fsc.includes()), contains("foo", "foo2")); - assertThat(source.docValueFields(), contains("bar", "bar2")); + assertThat(source.docValueFields().stream().map(ff -> ff.field).collect(Collectors.toList()), contains("bar", "bar2")); Map scriptFields = source.scriptFields() .stream() .collect(Collectors.toMap(SearchSourceBuilder.ScriptField::fieldName, SearchSourceBuilder.ScriptField::script)); diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/ComputingExtractorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/ComputingExtractorTests.java index 74721eca22a..375de112fe8 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/ComputingExtractorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/ComputingExtractorTests.java @@ -70,7 +70,7 @@ public class ComputingExtractorTests extends AbstractWireSerializingTestCase documentFieldValues = Collections.singletonList(Long.toString(millis)); + SearchHit hit = new SearchHit(1); + DocumentField field = new DocumentField("my_date_field", documentFieldValues); + hit.fields(singletonMap("my_date_field", field)); + FieldHitExtractor extractor = new FieldHitExtractor("my_date_field", DataType.DATE, true); + assertEquals(new DateTime(millis, DateTimeZone.UTC), extractor.extract(hit)); + } + public void testGetSource() throws IOException { String fieldName = randomAlphaOfLength(5); - FieldHitExtractor extractor = new FieldHitExtractor(fieldName, false); + FieldHitExtractor extractor = new FieldHitExtractor(fieldName, null, false); int times = between(1, 1000); for (int i = 0; i < times; i++) { @@ -164,12 +177,12 @@ public class FieldHitExtractorTests extends AbstractWireSerializingTestCase map = singletonMap("a", singletonMap("b", singletonMap("c", value))); assertThat(fe.extractFromSource(map), is(value)); } public void testExtractSourceIncorrectPath() { - FieldHitExtractor fe = new FieldHitExtractor("a.b.c.d", false); + FieldHitExtractor fe = new FieldHitExtractor("a.b.c.d", null, false); Object value = randomNonNullValue(); Map map = singletonMap("a", singletonMap("b", singletonMap("c", value))); SqlException ex = expectThrows(SqlException.class, () -> fe.extractFromSource(map)); @@ -223,7 +236,7 @@ public class FieldHitExtractorTests extends AbstractWireSerializingTestCase map = singletonMap("a", asList(value, value)); SqlException ex = expectThrows(SqlException.class, () -> fe.extractFromSource(map)); diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/sql/translate.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/sql/translate.yml index b3d93e52988..32080ce2e1f 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/sql/translate.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/sql/translate.yml @@ -1,5 +1,10 @@ --- "Translate SQL": + - skip: + version: " - 6.99.99" # TODO: change version on backport + reason: format option was added in 6.4 + features: warnings + - do: bulk: refresh: true @@ -23,7 +28,8 @@ - str excludes: [] docvalue_fields: - - int + - field: int + format: use_field_mapping sort: - int: order: asc diff --git a/x-pack/qa/sql/no-security/src/test/java/org/elasticsearch/xpack/qa/sql/nosecurity/CliExplainIT.java b/x-pack/qa/sql/no-security/src/test/java/org/elasticsearch/xpack/qa/sql/nosecurity/CliExplainIT.java index f913395759c..63831c2d4de 100644 --- a/x-pack/qa/sql/no-security/src/test/java/org/elasticsearch/xpack/qa/sql/nosecurity/CliExplainIT.java +++ b/x-pack/qa/sql/no-security/src/test/java/org/elasticsearch/xpack/qa/sql/nosecurity/CliExplainIT.java @@ -103,7 +103,10 @@ public class CliExplainIT extends CliIntegrationTestCase { assertThat(readLine(), startsWith(" \"excludes\" : [ ]")); assertThat(readLine(), startsWith(" },")); assertThat(readLine(), startsWith(" \"docvalue_fields\" : [")); - assertThat(readLine(), startsWith(" \"i\"")); + assertThat(readLine(), startsWith(" {")); + assertThat(readLine(), startsWith(" \"field\" : \"i\"")); + assertThat(readLine(), startsWith(" \"format\" : \"use_field_mapping\"")); + assertThat(readLine(), startsWith(" }")); assertThat(readLine(), startsWith(" ],")); assertThat(readLine(), startsWith(" \"sort\" : [")); assertThat(readLine(), startsWith(" {"));