From f29c743a4789d3aa82cf0a677defeefd5bea6dfd Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 14 Sep 2020 10:08:58 -0700 Subject: [PATCH] Support the 'fields' option in inner_hits and top_hits. (#62259) This PR adds support for the 'fields' option in the following places: * Anytime `inner_hits` is used, for both fetching nested/ child docs and field collapsing * The `top_hits` aggregation Addresses #61949. --- .../metrics/tophits-aggregation.asciidoc | 3 +- .../retrieve-inner-hits.asciidoc | 1 + .../elasticsearch/join/query/InnerHitsIT.java | 14 +- .../test/search/110_field_collapsing.yml | 47 ++++++- .../aggregations/metrics/TopHitsIT.java | 19 ++- .../search/fetch/subphase/InnerHitsIT.java | 14 +- .../action/search/ExpandSearchPhase.java | 3 + .../index/query/InnerHitBuilder.java | 106 +++++++++++---- .../index/query/InnerHitContextBuilder.java | 7 + .../metrics/TopHitsAggregationBuilder.java | 121 +++++++++++++----- .../metrics/TopHitsAggregatorFactory.java | 9 ++ .../search/fetch/FetchPhase.java | 10 +- .../index/query/InnerHitBuilderTests.java | 19 +++ .../aggregations/metrics/TopHitsTests.java | 6 + .../runtime-fields/qa/rest/build.gradle | 3 +- 15 files changed, 299 insertions(+), 83 deletions(-) diff --git a/docs/reference/aggregations/metrics/tophits-aggregation.asciidoc b/docs/reference/aggregations/metrics/tophits-aggregation.asciidoc index 9e53b53c395a1..f02d1b700ed3e 100644 --- a/docs/reference/aggregations/metrics/tophits-aggregation.asciidoc +++ b/docs/reference/aggregations/metrics/tophits-aggregation.asciidoc @@ -24,6 +24,7 @@ The top_hits aggregation returns regular search hits, because of this many per h * <> * <> * <> +* <> * <> * <> * <> @@ -41,7 +42,7 @@ by aggregations, use a <> or ==== Example -In the following example we group the sales by type and per type we show the last sale. +In the following example we group the sales by type and per type we show the last sale. For each sale only the date and price fields are being included in the source. [source,console] diff --git a/docs/reference/search/search-your-data/retrieve-inner-hits.asciidoc b/docs/reference/search/search-your-data/retrieve-inner-hits.asciidoc index af984cd0c28de..29c06cea3087a 100644 --- a/docs/reference/search/search-your-data/retrieve-inner-hits.asciidoc +++ b/docs/reference/search/search-your-data/retrieve-inner-hits.asciidoc @@ -73,6 +73,7 @@ Inner hits also supports the following per document features: * <> * <> +* <> * <> * <> * <> diff --git a/modules/parent-join/src/internalClusterTest/java/org/elasticsearch/join/query/InnerHitsIT.java b/modules/parent-join/src/internalClusterTest/java/org/elasticsearch/join/query/InnerHitsIT.java index 53d41f09c39a3..982355a36d113 100644 --- a/modules/parent-join/src/internalClusterTest/java/org/elasticsearch/join/query/InnerHitsIT.java +++ b/modules/parent-join/src/internalClusterTest/java/org/elasticsearch/join/query/InnerHitsIT.java @@ -171,7 +171,7 @@ public void testSimpleParentChild() throws Exception { .setQuery( hasChildQuery("comment", matchQuery("message", "fox"), ScoreMode.None).innerHit( new InnerHitBuilder() - .addDocValueField("message") + .addFetchField("message") .setHighlightBuilder(new HighlightBuilder().field("message")) .setExplain(true).setSize(1) .addScriptField("script", new Script(ScriptType.INLINE, MockScriptEngine.NAME, "5", @@ -182,8 +182,18 @@ public void testSimpleParentChild() throws Exception { assertThat(innerHits.getHits().length, equalTo(1)); assertThat(innerHits.getAt(0).getHighlightFields().get("message").getFragments()[0].string(), equalTo("fox eat quick")); assertThat(innerHits.getAt(0).getExplanation().toString(), containsString("weight(message:fox")); - assertThat(innerHits.getAt(0).getFields().get("message").getValue().toString(), equalTo("eat")); + assertThat(innerHits.getAt(0).getFields().get("message").getValue().toString(), equalTo("fox eat quick")); assertThat(innerHits.getAt(0).getFields().get("script").getValue().toString(), equalTo("5")); + + response = client().prepareSearch("articles") + .setQuery( + hasChildQuery("comment", matchQuery("message", "fox"), ScoreMode.None).innerHit( + new InnerHitBuilder().addDocValueField("message").setSize(1) + )).get(); + assertNoFailures(response); + innerHits = response.getHits().getAt(0).getInnerHits().get("comment"); + assertThat(innerHits.getHits().length, equalTo(1)); + assertThat(innerHits.getAt(0).getFields().get("message").getValue().toString(), equalTo("eat")); } public void testRandomParentChild() throws Exception { diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/110_field_collapsing.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/110_field_collapsing.yml index eafb5e088085a..4e0b7b1c1fe04 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/110_field_collapsing.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/110_field_collapsing.yml @@ -6,6 +6,7 @@ setup: mappings: properties: numeric_group: { type: integer } + tag: { type: keyword } - do: index: @@ -13,42 +14,42 @@ setup: id: 1 version_type: external version: 11 - body: { numeric_group: 1, sort: 10 } + body: { numeric_group: 1, tag: A, sort: 10 } - do: index: index: test id: 2 version_type: external version: 22 - body: { numeric_group: 1, sort: 6 } + body: { numeric_group: 1, tag: B, sort: 6 } - do: index: index: test id: 3 version_type: external version: 33 - body: { numeric_group: 1, sort: 24 } + body: { numeric_group: 1, tag: A, sort: 24 } - do: index: index: test id: 4 version_type: external version: 44 - body: { numeric_group: 25, sort: 10 } + body: { numeric_group: 25, tag: B, sort: 10 } - do: index: index: test id: 5 version_type: external version: 55 - body: { numeric_group: 25, sort: 5 } + body: { numeric_group: 25, tag: A, sort: 5 } - do: index: index: test id: 6 version_type: external version: 66 - body: { numeric_group: 3, sort: 36 } + body: { numeric_group: 3, tag: B, sort: 36 } - do: indices.refresh: index: test @@ -139,6 +140,40 @@ setup: - match: { hits.hits.2.inner_hits.sub_hits.hits.hits.0._id: "5" } - match: { hits.hits.2.inner_hits.sub_hits.hits.hits.1._id: "4" } +--- +"field collapsing, inner_hits, and fields": + - skip: + version: " - 7.99.99" + reason: the enhancement is not yet backported to 7.x + - do: + search: + rest_total_hits_as_int: true + index: test + body: + collapse: + field: numeric_group + inner_hits: + name: sub_hits + size: 2 + sort: [{ sort: asc }] + fields: ["tag"] + sort: [{ sort: desc }] + + - length: { hits.hits: 3 } + - length: { hits.hits.0.inner_hits.sub_hits.hits.hits: 1 } + - match: { hits.hits.0.inner_hits.sub_hits.hits.hits.0._id: "6" } + - match: { hits.hits.0.inner_hits.sub_hits.hits.hits.0.fields.tag: ["B"]} + - length: { hits.hits.1.inner_hits.sub_hits.hits.hits: 2 } + - match: { hits.hits.1.inner_hits.sub_hits.hits.hits.0._id: "2" } + - match: { hits.hits.1.inner_hits.sub_hits.hits.hits.0.fields.tag: ["B"]} + - match: { hits.hits.1.inner_hits.sub_hits.hits.hits.1._id: "1" } + - match: { hits.hits.1.inner_hits.sub_hits.hits.hits.1.fields.tag: ["A"]} + - length: { hits.hits.2.inner_hits.sub_hits.hits.hits: 2 } + - match: { hits.hits.2.inner_hits.sub_hits.hits.hits.0._id: "5" } + - match: { hits.hits.2.inner_hits.sub_hits.hits.hits.0.fields.tag: ["A"]} + - match: { hits.hits.2.inner_hits.sub_hits.hits.hits.1._id: "4" } + - match: { hits.hits.2.inner_hits.sub_hits.hits.hits.1.fields.tag: ["B"]} + --- "field collapsing, inner_hits and maxConcurrentGroupRequests": diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java index 8cda3d796d51d..29f549c9f63a4 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/metrics/TopHitsIT.java @@ -166,6 +166,7 @@ public void setupSuiteScopeCluster() throws Exception { .field(SORT_FIELD, i + 1) .field("text", "some text to entertain") .field("field1", 5) + .field("field2", 2.71) .endObject())); } @@ -315,7 +316,7 @@ public void testBasics() throws Exception { assertThat((Long) hits.getAt(1).getSortValues()[0], equalTo(higestSortValue - 1)); assertThat((Long) hits.getAt(2).getSortValues()[0], equalTo(higestSortValue - 2)); - assertThat(hits.getAt(0).getSourceAsMap().size(), equalTo(4)); + assertThat(hits.getAt(0).getSourceAsMap().size(), equalTo(5)); } } @@ -402,7 +403,7 @@ public void testBreadthFirstWithScoreNeeded() throws Exception { assertThat(hits.getTotalHits().value, equalTo(10L)); assertThat(hits.getHits().length, equalTo(3)); - assertThat(hits.getAt(0).getSourceAsMap().size(), equalTo(4)); + assertThat(hits.getAt(0).getSourceAsMap().size(), equalTo(5)); } } @@ -433,7 +434,7 @@ public void testBreadthFirstWithAggOrderAndScoreNeeded() throws Exception { assertThat(hits.getTotalHits().value, equalTo(10L)); assertThat(hits.getHits().length, equalTo(3)); - assertThat(hits.getAt(0).getSourceAsMap().size(), equalTo(4)); + assertThat(hits.getAt(0).getSourceAsMap().size(), equalTo(5)); id--; } } @@ -597,6 +598,7 @@ public void testFetchFeatures() { .explain(true) .storedField("text") .docValueField("field1") + .fetchField("field2") .scriptField("script", new Script(ScriptType.INLINE, MockScriptEngine.NAME, "5", Collections.emptyMap())) .fetchSource("text", null) @@ -639,13 +641,16 @@ public void testFetchFeatures() { assertThat(hit.getMatchedQueries()[0], equalTo("test")); - DocumentField field = hit.field("field1"); - assertThat(field.getValue().toString(), equalTo("5")); + DocumentField field1 = hit.field("field1"); + assertThat(field1.getValue(), equalTo(5L)); + + DocumentField field2 = hit.field("field2"); + assertThat(field2.getValue(), equalTo(2.71f)); assertThat(hit.getSourceAsMap().get("text").toString(), equalTo("some text to entertain")); - field = hit.field("script"); - assertThat(field.getValue().toString(), equalTo("5")); + field2 = hit.field("script"); + assertThat(field2.getValue().toString(), equalTo("5")); assertThat(hit.getSourceAsMap().size(), equalTo(1)); assertThat(hit.getSourceAsMap().get("text").toString(), equalTo("some text to entertain")); diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/InnerHitsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/InnerHitsIT.java index da01f6bea627e..164dfbc2a499c 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/InnerHitsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/subphase/InnerHitsIT.java @@ -165,7 +165,7 @@ public void testSimpleNested() throws Exception { .setQuery(nestedQuery("comments", matchQuery("comments.message", "fox"), ScoreMode.Avg).innerHit( new InnerHitBuilder().setHighlightBuilder(new HighlightBuilder().field("comments.message")) .setExplain(true) - .addDocValueField("comments.mes*") + .addFetchField("comments.mes*") .addScriptField("script", new Script(ScriptType.INLINE, MockScriptEngine.NAME, "5", Collections.emptyMap())) .setSize(1))).get(); @@ -176,8 +176,18 @@ public void testSimpleNested() throws Exception { assertThat(innerHits.getAt(0).getHighlightFields().get("comments.message").getFragments()[0].string(), equalTo("fox eat quick")); assertThat(innerHits.getAt(0).getExplanation().toString(), containsString("weight(comments.message:fox in")); - assertThat(innerHits.getAt(0).getFields().get("comments.message").getValue().toString(), equalTo("eat")); + assertThat(innerHits.getAt(0).getFields().get("comments.message").getValue().toString(), equalTo("fox eat quick")); assertThat(innerHits.getAt(0).getFields().get("script").getValue().toString(), equalTo("5")); + + response = client().prepareSearch("articles") + .setQuery(nestedQuery("comments", matchQuery("comments.message", "fox"), ScoreMode.Avg).innerHit( + new InnerHitBuilder() + .addDocValueField("comments.mes*") + .setSize(1))).get(); + assertNoFailures(response); + innerHits = response.getHits().getAt(0).getInnerHits().get("comments"); + assertThat(innerHits.getHits().length, equalTo(1)); + assertThat(innerHits.getAt(0).getFields().get("comments.message").getValue().toString(), equalTo("eat")); } public void testRandomNested() throws Exception { 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 cffbf7ea0a072..57082ed450941 100644 --- a/server/src/main/java/org/elasticsearch/action/search/ExpandSearchPhase.java +++ b/server/src/main/java/org/elasticsearch/action/search/ExpandSearchPhase.java @@ -136,6 +136,9 @@ private SearchSourceBuilder buildExpandSearchSourceBuilder(InnerHitBuilder optio options.getFetchSourceContext().excludes()); } } + if (options.getFetchFields() != null) { + options.getFetchFields().forEach(ff -> groupSource.fetchField(ff.field, ff.format)); + } if (options.getDocValueFields() != null) { options.getDocValueFields().forEach(ff -> groupSource.docValueField(ff.field, ff.format)); } diff --git a/server/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java b/server/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java index 31c30aac86655..e0502d8caa21b 100644 --- a/server/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java @@ -18,6 +18,8 @@ */ package org.elasticsearch.index.query; +import org.elasticsearch.Version; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.Strings; @@ -71,6 +73,8 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject { PARSER.declareStringArray(InnerHitBuilder::setStoredFieldNames, SearchSourceBuilder.STORED_FIELDS_FIELD); PARSER.declareObjectArray(InnerHitBuilder::setDocValueFields, (p,c) -> FieldAndFormat.fromXContent(p), SearchSourceBuilder.DOCVALUE_FIELDS_FIELD); + PARSER.declareObjectArray(InnerHitBuilder::setFetchFields, + (p,c) -> FieldAndFormat.fromXContent(p), SearchSourceBuilder.FETCH_FIELDS_FIELD); PARSER.declareField((p, i, c) -> { try { Set scriptFields = new HashSet<>(); @@ -133,6 +137,7 @@ public final class InnerHitBuilder implements Writeable, ToXContentObject { private Set scriptFields; private HighlightBuilder highlightBuilder; private FetchSourceContext fetchSourceContext; + private List fetchFields; private CollapseBuilder innerCollapseBuilder = null; public InnerHitBuilder() { @@ -175,6 +180,12 @@ public InnerHitBuilder(StreamInput in) throws IOException { } highlightBuilder = in.readOptionalWriteable(HighlightBuilder::new); this.innerCollapseBuilder = in.readOptionalWriteable(CollapseBuilder::new); + + if (in.getVersion().onOrAfter(Version.CURRENT)) { + if (in.readBoolean()) { + fetchFields = in.readList(FieldAndFormat::new); + } + } } @Override @@ -213,6 +224,13 @@ public void writeTo(StreamOutput out) throws IOException { } out.writeOptionalWriteable(highlightBuilder); out.writeOptionalWriteable(innerCollapseBuilder); + + if (out.getVersion().onOrAfter(Version.CURRENT)) { + out.writeBoolean(fetchFields != null); + if (fetchFields != null) { + out.writeList(fetchFields); + } + } } public String getName() { @@ -349,6 +367,41 @@ public InnerHitBuilder addDocValueField(String field) { return addDocValueField(field, null); } + /** + * Gets the fields to load and return as part of the search request. + */ + public List getFetchFields() { + return fetchFields; + } + + /** + * Sets the stored fields to load and return as part of the search request. + */ + public InnerHitBuilder setFetchFields(List fetchFields) { + this.fetchFields = fetchFields; + return this; + } + + /** + * Adds a field to load and return as part of the search request. + */ + public InnerHitBuilder addFetchField(String name) { + return addFetchField(name, null); + } + + /** + * Adds a field to load and return as part of the search request. + * @param name the field name. + * @param format an optional format string used when formatting values, for example a date format. + */ + public InnerHitBuilder addFetchField(String name, @Nullable String format) { + if (fetchFields == null) { + fetchFields = new ArrayList<>(); + } + fetchFields.add(new FieldAndFormat(name, format)); + return this; + } + public Set getScriptFields() { return scriptFields; } @@ -436,14 +489,14 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (docValueFields != null) { builder.startArray(SearchSourceBuilder.DOCVALUE_FIELDS_FIELD.getPreferredName()); for (FieldAndFormat docValueField : docValueFields) { - if (docValueField.format == null) { - builder.value(docValueField.field); - } else { - builder.startObject() - .field("field", docValueField.field) - .field("format", docValueField.format) - .endObject(); - } + docValueField.toXContent(builder, params); + } + builder.endArray(); + } + if (fetchFields != null) { + builder.startArray(SearchSourceBuilder.FETCH_FIELDS_FIELD.getPreferredName()); + for (FieldAndFormat docValueField : fetchFields) { + docValueField.toXContent(builder, params); } builder.endArray(); } @@ -475,29 +528,30 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - InnerHitBuilder that = (InnerHitBuilder) o; - return Objects.equals(name, that.name) && - Objects.equals(ignoreUnmapped, that.ignoreUnmapped) && - Objects.equals(from, that.from) && - Objects.equals(size, that.size) && - Objects.equals(explain, that.explain) && - Objects.equals(version, that.version) && - Objects.equals(seqNoAndPrimaryTerm, that.seqNoAndPrimaryTerm) && - Objects.equals(trackScores, that.trackScores) && - Objects.equals(storedFieldsContext, that.storedFieldsContext) && - Objects.equals(docValueFields, that.docValueFields) && - Objects.equals(scriptFields, that.scriptFields) && - Objects.equals(fetchSourceContext, that.fetchSourceContext) && - Objects.equals(sorts, that.sorts) && - Objects.equals(highlightBuilder, that.highlightBuilder) && - Objects.equals(innerCollapseBuilder, that.innerCollapseBuilder); + return ignoreUnmapped == that.ignoreUnmapped && + from == that.from && + size == that.size && + explain == that.explain && + version == that.version && + seqNoAndPrimaryTerm == that.seqNoAndPrimaryTerm && + trackScores == that.trackScores && + Objects.equals(name, that.name) && + Objects.equals(storedFieldsContext, that.storedFieldsContext) && + Objects.equals(query, that.query) && + Objects.equals(sorts, that.sorts) && + Objects.equals(docValueFields, that.docValueFields) && + Objects.equals(scriptFields, that.scriptFields) && + Objects.equals(highlightBuilder, that.highlightBuilder) && + Objects.equals(fetchSourceContext, that.fetchSourceContext) && + Objects.equals(fetchFields, that.fetchFields) && + Objects.equals(innerCollapseBuilder, that.innerCollapseBuilder); } @Override public int hashCode() { - return Objects.hash(name, ignoreUnmapped, from, size, explain, version, seqNoAndPrimaryTerm, trackScores, - storedFieldsContext, docValueFields, scriptFields, fetchSourceContext, sorts, highlightBuilder, innerCollapseBuilder); + return Objects.hash(name, ignoreUnmapped, from, size, explain, version, seqNoAndPrimaryTerm, trackScores, storedFieldsContext, + query, sorts, docValueFields, scriptFields, highlightBuilder, fetchSourceContext, fetchFields, innerCollapseBuilder); } public static InnerHitBuilder fromXContent(XContentParser parser) throws IOException { diff --git a/server/src/main/java/org/elasticsearch/index/query/InnerHitContextBuilder.java b/server/src/main/java/org/elasticsearch/index/query/InnerHitContextBuilder.java index 135da0e621e85..ed2e4a2315957 100644 --- a/server/src/main/java/org/elasticsearch/index/query/InnerHitContextBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/InnerHitContextBuilder.java @@ -23,6 +23,7 @@ import org.elasticsearch.script.FieldScript; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.search.fetch.subphase.FetchDocValuesContext; +import org.elasticsearch.search.fetch.subphase.FetchFieldsContext; import org.elasticsearch.search.fetch.subphase.InnerHitsContext; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.search.sort.SortAndFormats; @@ -92,6 +93,12 @@ protected void setupInnerHitsContext(QueryShardContext queryShardContext, queryShardContext.getMapperService(), innerHitBuilder.getDocValueFields()); innerHitsContext.docValuesContext(docValuesContext); } + if (innerHitBuilder.getFetchFields() != null) { + String indexName = queryShardContext.index().getName(); + FetchFieldsContext fieldsContext = FetchFieldsContext.create( + indexName, queryShardContext.getMapperService(), innerHitBuilder.getFetchFields()); + innerHitsContext.fetchFieldsContext(fieldsContext); + } if (innerHitBuilder.getScriptFields() != null) { for (SearchSourceBuilder.ScriptField field : innerHitBuilder.getScriptFields()) { QueryShardContext innerContext = innerHitsContext.getQueryShardContext(); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TopHitsAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TopHitsAggregationBuilder.java index edbc234efe3bc..14ee73de6b8e2 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TopHitsAggregationBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TopHitsAggregationBuilder.java @@ -19,6 +19,7 @@ package org.elasticsearch.search.aggregations.metrics; +import org.elasticsearch.Version; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.Strings; @@ -71,6 +72,7 @@ public class TopHitsAggregationBuilder extends AbstractAggregationBuilder docValueFields; + private List fetchFields; private Set scriptFields; private FetchSourceContext fetchSourceContext; @@ -93,6 +95,7 @@ protected TopHitsAggregationBuilder(TopHitsAggregationBuilder clone, this.storedFieldsContext = clone.storedFieldsContext == null ? null : new StoredFieldsContext(clone.storedFieldsContext); this.docValueFields = clone.docValueFields == null ? null : new ArrayList<>(clone.docValueFields); + this.fetchFields = clone.fetchFields == null ? null : new ArrayList<>(clone.fetchFields); this.scriptFields = clone.scriptFields == null ? null : new HashSet<>(clone.scriptFields); this.fetchSourceContext = clone.fetchSourceContext == null ? null : new FetchSourceContext(clone.fetchSourceContext.fetchSource(), clone.fetchSourceContext.includes(), @@ -139,6 +142,11 @@ public TopHitsAggregationBuilder(StreamInput in) throws IOException { trackScores = in.readBoolean(); version = in.readBoolean(); seqNoAndPrimaryTerm = in.readBoolean(); + if (in.getVersion().onOrAfter(Version.CURRENT)) { + if (in.readBoolean()) { + fetchFields = in.readList(FieldAndFormat::new); + } + } } @Override @@ -167,6 +175,12 @@ protected void doWriteTo(StreamOutput out) throws IOException { out.writeBoolean(trackScores); out.writeBoolean(version); out.writeBoolean(seqNoAndPrimaryTerm); + if (out.getVersion().onOrAfter(Version.CURRENT)) { + out.writeBoolean(fetchFields != null); + if (fetchFields != null) { + out.writeList(fetchFields); + } + } } /** @@ -425,10 +439,38 @@ public TopHitsAggregationBuilder docValueField(String docValueField) { /** * Gets the field-data fields. */ - public List fieldDataFields() { + public List docValueFields() { return docValueFields; } + /** + * Adds a field to load and return as part of the search request. + */ + public TopHitsAggregationBuilder fetchField(String field, String format) { + if (field == null) { + throw new IllegalArgumentException("[fields] must not be null: [" + name + "]"); + } + if (fetchFields == null) { + fetchFields = new ArrayList<>(); + } + fetchFields.add(new FieldAndFormat(field, format)); + return this; + } + + /** + * Adds a field to load and return as part of the search request. + */ + public TopHitsAggregationBuilder fetchField(String field) { + return fetchField(field, null); + } + + /** + * Gets the fields to load and return as part of the search request. + */ + public List fetchFields() { + return fetchFields; + } + /** * Adds a script field under the given name with the provided script. * @@ -580,12 +622,12 @@ protected TopHitsAggregatorFactory doBuild(QueryShardContext queryShardContext, ); } - List fields = new ArrayList<>(); - if (scriptFields != null) { - for (ScriptField field : scriptFields) { + List scriptFields = new ArrayList<>(); + if (this.scriptFields != null) { + for (ScriptField field : this.scriptFields) { FieldScript.Factory factory = queryShardContext.compile(field.script(), FieldScript.CONTEXT); FieldScript.LeafFactory searchScript = factory.newFactory(field.script().getParams(), queryShardContext.lookup()); - fields.add(new org.elasticsearch.search.fetch.subphase.ScriptFieldsContext.ScriptField( + scriptFields.add(new org.elasticsearch.search.fetch.subphase.ScriptFieldsContext.ScriptField( field.fieldName(), searchScript, field.ignoreFailure())); } } @@ -597,7 +639,7 @@ protected TopHitsAggregatorFactory doBuild(QueryShardContext queryShardContext, optionalSort = SortBuilder.buildSort(sorts, queryShardContext); } return new TopHitsAggregatorFactory(name, from, size, explain, version, seqNoAndPrimaryTerm, trackScores, optionalSort, - highlightBuilder, storedFieldsContext, docValueFields, fields, fetchSourceContext, queryShardContext, parent, + highlightBuilder, storedFieldsContext, docValueFields, fetchFields, scriptFields, fetchSourceContext, queryShardContext, parent, subfactoriesBuilder, metadata); } @@ -615,18 +657,23 @@ protected XContentBuilder internalXContent(XContentBuilder builder, Params param if (storedFieldsContext != null) { storedFieldsContext.toXContent(SearchSourceBuilder.STORED_FIELDS_FIELD.getPreferredName(), builder); } + if (docValueFields != null) { builder.startArray(SearchSourceBuilder.DOCVALUE_FIELDS_FIELD.getPreferredName()); - for (FieldAndFormat dvField : docValueFields) { - builder.startObject() - .field("field", dvField.field); - if (dvField.format != null) { - builder.field("format", dvField.format); - } - builder.endObject(); + for (FieldAndFormat docValueField : docValueFields) { + docValueField.toXContent(builder, params); + } + builder.endArray(); + } + + if (fetchFields != null) { + builder.startArray(SearchSourceBuilder.FETCH_FIELDS_FIELD.getPreferredName()); + for (FieldAndFormat docValueField : fetchFields) { + docValueField.toXContent(builder, params); } builder.endArray(); } + if (scriptFields != null) { builder.startObject(SearchSourceBuilder.SCRIPT_FIELDS_FIELD.getPreferredName()); for (ScriptField scriptField : scriptFields) { @@ -746,6 +793,11 @@ public static TopHitsAggregationBuilder parse(String aggregationName, XContentPa FieldAndFormat ff = FieldAndFormat.fromXContent(parser); factory.docValueField(ff.field, ff.format); } + } else if (SearchSourceBuilder.FETCH_FIELDS_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { + while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { + FieldAndFormat ff = FieldAndFormat.fromXContent(parser); + factory.fetchField(ff.field, ff.format); + } } else if (SearchSourceBuilder.SORT_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { List> sorts = SortBuilder.fromXContent(parser); factory.sorts(sorts); @@ -764,31 +816,30 @@ public static TopHitsAggregationBuilder parse(String aggregationName, XContentPa } @Override - public int hashCode() { - return Objects.hash(super.hashCode(), explain, fetchSourceContext, docValueFields, - storedFieldsContext, from, highlightBuilder, - scriptFields, size, sorts, trackScores, version, - seqNoAndPrimaryTerm); + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + TopHitsAggregationBuilder that = (TopHitsAggregationBuilder) o; + return from == that.from && + size == that.size && + explain == that.explain && + version == that.version && + seqNoAndPrimaryTerm == that.seqNoAndPrimaryTerm && + trackScores == that.trackScores && + Objects.equals(sorts, that.sorts) && + Objects.equals(highlightBuilder, that.highlightBuilder) && + Objects.equals(storedFieldsContext, that.storedFieldsContext) && + Objects.equals(docValueFields, that.docValueFields) && + Objects.equals(fetchFields, that.fetchFields) && + Objects.equals(scriptFields, that.scriptFields) && + Objects.equals(fetchSourceContext, that.fetchSourceContext); } @Override - public boolean equals(Object obj) { - if (this == obj) return true; - if (obj == null || getClass() != obj.getClass()) return false; - if (super.equals(obj) == false) return false; - TopHitsAggregationBuilder other = (TopHitsAggregationBuilder) obj; - return Objects.equals(explain, other.explain) - && Objects.equals(fetchSourceContext, other.fetchSourceContext) - && Objects.equals(docValueFields, other.docValueFields) - && Objects.equals(storedFieldsContext, other.storedFieldsContext) - && Objects.equals(from, other.from) - && Objects.equals(highlightBuilder, other.highlightBuilder) - && Objects.equals(scriptFields, other.scriptFields) - && Objects.equals(size, other.size) - && Objects.equals(sorts, other.sorts) - && Objects.equals(trackScores, other.trackScores) - && Objects.equals(version, other.version) - && Objects.equals(seqNoAndPrimaryTerm, other.seqNoAndPrimaryTerm); + public int hashCode() { + return Objects.hash(super.hashCode(), from, size, explain, version, seqNoAndPrimaryTerm, trackScores, sorts, highlightBuilder, + storedFieldsContext, docValueFields, fetchFields, scriptFields, fetchSourceContext); } @Override diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TopHitsAggregatorFactory.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TopHitsAggregatorFactory.java index ffee18e81cc24..7b6c440d77a2b 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TopHitsAggregatorFactory.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/TopHitsAggregatorFactory.java @@ -26,6 +26,7 @@ import org.elasticsearch.search.aggregations.CardinalityUpperBound; import org.elasticsearch.search.fetch.StoredFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchDocValuesContext; +import org.elasticsearch.search.fetch.subphase.FetchFieldsContext; import org.elasticsearch.search.fetch.subphase.FetchSourceContext; import org.elasticsearch.search.fetch.subphase.FieldAndFormat; import org.elasticsearch.search.fetch.subphase.ScriptFieldsContext; @@ -51,6 +52,7 @@ class TopHitsAggregatorFactory extends AggregatorFactory { private final HighlightBuilder highlightBuilder; private final StoredFieldsContext storedFieldsContext; private final List docValueFields; + private final List fetchFields; private final List scriptFields; private final FetchSourceContext fetchSourceContext; @@ -65,6 +67,7 @@ class TopHitsAggregatorFactory extends AggregatorFactory { HighlightBuilder highlightBuilder, StoredFieldsContext storedFieldsContext, List docValueFields, + List fetchFields, List scriptFields, FetchSourceContext fetchSourceContext, QueryShardContext queryShardContext, @@ -82,6 +85,7 @@ class TopHitsAggregatorFactory extends AggregatorFactory { this.highlightBuilder = highlightBuilder; this.storedFieldsContext = storedFieldsContext; this.docValueFields = docValueFields; + this.fetchFields = fetchFields; this.scriptFields = scriptFields; this.fetchSourceContext = fetchSourceContext; } @@ -109,6 +113,11 @@ public Aggregator createInternal(SearchContext searchContext, FetchDocValuesContext docValuesContext = FetchDocValuesContext.create(searchContext.mapperService(), docValueFields); subSearchContext.docValuesContext(docValuesContext); } + if (fetchFields != null) { + String indexName = searchContext.indexShard().shardId().getIndexName(); + FetchFieldsContext fieldsContext = FetchFieldsContext.create(indexName, searchContext.mapperService(), fetchFields); + subSearchContext.fetchFieldsContext(fieldsContext); + } for (ScriptFieldsContext.ScriptField field : scriptFields) { subSearchContext.scriptFields().add(field); } diff --git a/server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java index d87e012aed157..c06527ee56ed5 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java @@ -168,7 +168,7 @@ private FieldsVisitor createStoredFieldsVisitor(SearchContext context, Map rootSourceAsMap = null; 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 1d0cc23cbb32b..8cb4b56cd34c7 100644 --- a/server/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/InnerHitBuilderTests.java @@ -158,6 +158,8 @@ public static InnerHitBuilder randomInnerHits() { } innerHits.setDocValueFields(randomListStuff(16, () -> new FieldAndFormat(randomAlphaOfLengthBetween(1, 16), null))); + innerHits.setFetchFields(randomListStuff(16, + () -> new FieldAndFormat(randomAlphaOfLengthBetween(1, 16), null))); // Random script fields deduped on their field name. Map scriptFields = new HashMap<>(); for (SearchSourceBuilder.ScriptField field: randomListStuff(16, InnerHitBuilderTests::randomScript)) { @@ -204,6 +206,14 @@ static InnerHitBuilder mutate(InnerHitBuilder original) throws IOException { copy.addDocValueField(randomAlphaOfLengthBetween(1, 16)); } }); + modifiers.add(() -> { + if (randomBoolean()) { + copy.setFetchFields(randomValueOtherThan(copy.getFetchFields(), + () -> randomListStuff(16, () -> new FieldAndFormat(randomAlphaOfLengthBetween(1, 16), null)))); + } else { + copy.addFetchField(randomAlphaOfLengthBetween(1, 16)); + } + }); modifiers.add(() -> { if (randomBoolean()) { copy.setScriptFields(randomValueOtherThan(copy.getScriptFields(), () -> { @@ -292,4 +302,13 @@ public void testSetDocValueFormat() { Arrays.asList(new FieldAndFormat("foo", null), new FieldAndFormat("@timestamp", "epoch_millis")), innerHit.getDocValueFields()); } + + public void testSetFetchFieldFormat() { + InnerHitBuilder innerHit = new InnerHitBuilder(); + innerHit.addFetchField("foo"); + innerHit.addFetchField("@timestamp", "epoch_millis"); + assertEquals( + Arrays.asList(new FieldAndFormat("foo", null), new FieldAndFormat("@timestamp", "epoch_millis")), + innerHit.getFetchFields()); + } } 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 6ec1ea1cad301..5d352e2187d95 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 @@ -86,6 +86,12 @@ protected final TopHitsAggregationBuilder createTestAggregatorBuilder() { factory.docValueField(randomAlphaOfLengthBetween(5, 50)); } } + if (randomBoolean()) { + int fetchFieldsSize = randomInt(25); + for (int i = 0; i < fetchFieldsSize; i++) { + factory.fetchField(randomAlphaOfLengthBetween(5, 50)); + } + } if (randomBoolean()) { int scriptFieldsSize = randomInt(25); for (int i = 0; i < scriptFieldsSize; i++) { diff --git a/x-pack/plugin/runtime-fields/qa/rest/build.gradle b/x-pack/plugin/runtime-fields/qa/rest/build.gradle index 8c9111aa8b4cf..1c536bb9c8f8b 100644 --- a/x-pack/plugin/runtime-fields/qa/rest/build.gradle +++ b/x-pack/plugin/runtime-fields/qa/rest/build.gradle @@ -32,7 +32,8 @@ yamlRestTest { systemProperty 'tests.rest.blacklist', [ /////// TO FIX /////// - 'search/330_fetch_fields/*', // The whole API is not yet supported + 'search/330_fetch_fields/*', // The 'fields' option is not yet supported + 'search/110_field_collapsing/field collapsing, inner_hits, and fields', // Also fails because of the 'fields' option 'search.highlight/40_keyword_ignore/Plain Highligher should skip highlighting ignored keyword values', // The plain highlighter is incompatible with runtime fields. Worth fixing? 'search/115_multiple_field_collapsing/two levels fields collapsing', // Broken. Gotta fix. 'field_caps/30_filter/Field caps with index filter', // We don't support filtering field caps on runtime fields. What should we do?