Skip to content

Commit

Permalink
Ensure fetch fields aren't dropped when rewriting search. (#61383)
Browse files Browse the repository at this point in the history
Previously we didn't retain the requested fields when performing a shallow copy
of the search source. This meant that when a search was rewritten, we could drop
the requested fields and fail to return them in the response.
  • Loading branch information
jtibshirani authored Aug 20, 2020
1 parent fc9644d commit 468e58b
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ setup:
fields:
- field: keyword
format: "yyyy/MM/dd"

---
"Test disable source":
- do:
Expand Down Expand Up @@ -214,3 +215,47 @@ setup:

- match: { hits.hits.0.fields.keyword.0: "a" }
- match: { hits.hits.0.fields.integer.0: 42 }

---
"Test search rewrite":
- skip:
version: " - 7.99.99"
reason: "the corresponding bug fix isn't yet backported"

- do:
indices.create:
index: test
body:
settings:
index.number_of_shards: 1
mappings:
properties:
date:
type: date

- do:
index:
index: test
id: 1
body:
date: "1990-12-29T22:30:00.000Z"

- do:
indices.refresh:
index: [ test ]

- do:
search:
index: test
body:
query:
range:
date:
from: "1990-12-29T22:30:00.000Z"
fields:
- field: date
format: "yyyy/MM/dd"

- is_true: hits.hits.0._id
- is_true: hits.hits.0._source
- match: { hits.hits.0.fields.date.0: "1990/12/29" }
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,7 @@ private SearchSourceBuilder shallowCopy(QueryBuilder queryBuilder, QueryBuilder
rewrittenBuilder.explain = explain;
rewrittenBuilder.extBuilders = extBuilders;
rewrittenBuilder.fetchSourceContext = fetchSourceContext;
rewrittenBuilder.fetchFields = fetchFields;
rewrittenBuilder.docValueFields = docValueFields;
rewrittenBuilder.storedFieldsContext = storedFieldsContext;
rewrittenBuilder.from = from;
Expand Down Expand Up @@ -1560,10 +1561,10 @@ public boolean equals(Object obj) {

@Override
public int hashCode() {
return Objects.hash(aggregations, explain, fetchSourceContext, docValueFields, storedFieldsContext, from, highlightBuilder,
indexBoosts, minScore, postQueryBuilder, queryBuilder, rescoreBuilders, scriptFields, size,
sorts, searchAfterBuilder, sliceBuilder, stats, suggestBuilder, terminateAfter, timeout, trackScores, version,
seqNoAndPrimaryTerm, profile, extBuilders, collapse, trackTotalHitsUpTo);
return Objects.hash(aggregations, explain, fetchSourceContext, fetchFields, docValueFields, storedFieldsContext, from,
highlightBuilder, indexBoosts, minScore, postQueryBuilder, queryBuilder, rescoreBuilders, scriptFields, size,
sorts, searchAfterBuilder, sliceBuilder, stats, suggestBuilder, terminateAfter, timeout, trackScores, version,
seqNoAndPrimaryTerm, profile, extBuilders, collapse, trackTotalHitsUpTo);
}

@Override
Expand All @@ -1578,6 +1579,7 @@ public boolean equals(Object obj) {
return Objects.equals(aggregations, other.aggregations)
&& Objects.equals(explain, other.explain)
&& Objects.equals(fetchSourceContext, other.fetchSourceContext)
&& Objects.equals(fetchFields, other.fetchFields)
&& Objects.equals(docValueFields, other.docValueFields)
&& Objects.equals(storedFieldsContext, other.storedFieldsContext)
&& Objects.equals(from, other.from)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
package org.elasticsearch.search.builder;

import com.fasterxml.jackson.core.JsonParseException;

import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
Expand Down Expand Up @@ -106,6 +105,14 @@ public void testSerialization() throws IOException {
}
}

public void testShallowCopy() {
for (int i = 0; i < 10; i++) {
SearchSourceBuilder original = createSearchSourceBuilder();
SearchSourceBuilder copy = original.shallowCopy();
assertEquals(original, copy);
}
}

public void testEqualsAndHashcode() throws IOException {
// TODO add test checking that changing any member of this class produces an object that is not equal to the original
EqualsHashCodeTestUtils.checkEqualsAndHashCode(createSearchSourceBuilder(), this::copyBuilder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,13 @@ public static SearchSourceBuilder randomSearchSourceBuilder(
throw new IllegalStateException();
}

if (randomBoolean()) {
int numFields = randomInt(5);
for (int i = 0; i < numFields; i++) {
builder.fetchField(randomAlphaOfLengthBetween(5, 10));
}
}

if (randomBoolean()) {
int scriptFieldsSize = randomInt(25);
for (int i = 0; i < scriptFieldsSize; i++) {
Expand Down

0 comments on commit 468e58b

Please sign in to comment.