From 4a19bdb2ea52c93c8e94286f3200c194fb843ce3 Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Mon, 14 Sep 2020 11:51:45 -0700 Subject: [PATCH] Support the 'fields' option in inner_hits and top_hits. (#62337) 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 | 105 +++++++++++---- .../index/query/InnerHitContextBuilder.java | 7 + .../metrics/TopHitsAggregationBuilder.java | 122 +++++++++++++----- .../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 6e4f7ba2c2c78..0840732a91800 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 bd2235de8d6a0..ca7e7a8172951 100644 --- a/docs/reference/search/search-your-data/retrieve-inner-hits.asciidoc +++ b/docs/reference/search/search-your-data/retrieve-inner-hits.asciidoc @@ -74,6 +74,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 730b05e7b0bf4..f00477acbf07b 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 @@ -176,7 +176,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", @@ -187,8 +187,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 8a08c3f8df203..1a1a1a079de2d 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 @@ -161,6 +162,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.9.99" + reason: the fields option was added in 7.10 + - 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 af897ba1be31b..6931c31334092 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 bc254a7d3910f..30e6c6d3fdaa0 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 10b9ede95e7f8..81dbc4c2e0a69 100644 --- a/server/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java @@ -19,6 +19,7 @@ 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; @@ -73,6 +74,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<>(); @@ -135,6 +138,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() { @@ -195,6 +199,12 @@ public InnerHitBuilder(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(Version.V_6_4_0)) { this.innerCollapseBuilder = in.readOptionalWriteable(CollapseBuilder::new); } + + if (in.getVersion().onOrAfter(Version.V_7_10_0)) { + if (in.readBoolean()) { + fetchFields = in.readList(FieldAndFormat::new); + } + } } @Override @@ -243,6 +253,13 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(Version.V_6_4_0)) { out.writeOptionalWriteable(innerCollapseBuilder); } + + if (out.getVersion().onOrAfter(Version.V_7_10_0)) { + out.writeBoolean(fetchFields != null); + if (fetchFields != null) { + out.writeList(fetchFields); + } + } } public String getName() { @@ -379,6 +396,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; } @@ -466,14 +518,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(); } @@ -505,29 +557,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 14e1fc5a36183..30f1cd36f59ba 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 cf759e05f30c2..1303acebd4846 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 @@ -72,6 +72,7 @@ public class TopHitsAggregationBuilder extends AbstractAggregationBuilder docValueFields; + private List fetchFields; private Set scriptFields; private FetchSourceContext fetchSourceContext; @@ -94,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(), @@ -142,6 +144,12 @@ public TopHitsAggregationBuilder(StreamInput in) throws IOException { if (in.getVersion().onOrAfter(Version.V_6_7_0)) { seqNoAndPrimaryTerm = in.readBoolean(); } + + if (in.getVersion().onOrAfter(Version.V_7_10_0)) { + if (in.readBoolean()) { + fetchFields = in.readList(FieldAndFormat::new); + } + } } @Override @@ -172,6 +180,13 @@ protected void doWriteTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(Version.V_6_7_0)) { out.writeBoolean(seqNoAndPrimaryTerm); } + + if (out.getVersion().onOrAfter(Version.V_7_10_0)) { + out.writeBoolean(fetchFields != null); + if (fetchFields != null) { + out.writeList(fetchFields); + } + } } /** @@ -430,10 +445,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. * @@ -585,12 +628,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())); } } @@ -602,7 +645,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); } @@ -620,18 +663,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) { @@ -751,6 +799,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); @@ -769,31 +822,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 5e1e7087041aa..1a5abd155c77f 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java @@ -172,7 +172,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 86b499d85533b..d3b5d5c8ecd5a 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 8edf63f1b0ac7..59fad2f2f2d64 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?