Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixup highlighting with synthetic source #87667

Merged
merged 5 commits into from
Jun 15, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,314 @@
setup:
- skip:
version: " - 8.3.99"
reason: introduced in 8.4.0

- do:
indices.create:
index: test
body:
mappings:
_source:
synthetic: true
properties:
foo:
type: keyword
fields:
analyze:
type: text
index_options: positions
vectors:
type: text
term_vector: with_positions_offsets
positions:
type: text
index_options: offsets

- do:
index:
index: test
id: 1
refresh: true
body:
foo: the quick brown fox jumped over the lazy dog

- do:
index:
index: test
id: 2
refresh: true
body:
foo:
- "To be, or not to be, that is the question:"
- "Whether 'tis nobler in the mind to suffer"
- "The slings and arrows of outrageous fortune,"
- "Or to take arms against a sea of troubles"
- "And by opposing end them. To die—to sleep,"
- "No more; and by a sleep to say we end"
- "The heart-ache and the thousand natural shocks"
- "That flesh is heir to: 'tis a consummation"
- "Devoutly to be wish'd. To die, to sleep;"
- "To sleep, perchance to dream—ay, there's the rub:"
- "For in that sleep of death what dreams may come,"
- "When we have shuffled off this mortal coil,"
- "Must give us pause—there's the respect"
- "That makes calamity of so long life."
- "For who would bear the whips and scorns of time,"
- "Th'oppressor's wrong, the proud man's contumely,"
- "The pangs of dispriz'd love, the law's delay,"
- "The insolence of office, and the spurns"
- "That patient merit of th'unworthy takes,"
- "When he himself might his quietus make"
- "With a bare bodkin? Who would fardels bear,"
- "To grunt and sweat under a weary life,"
- "But that the dread of something after death,"
- "The undiscovere'd country, from whose bourn"
- "No traveller returns, puzzles the will,"
- "And makes us rather bear those ills we have"
- "Than fly to others that we know not of?"
- "Thus conscience doth make cowards of us all,"
- "And thus the native hue of resolution"
- "Is sicklied o'er with the pale cast of thought,"
- "And enterprises of great pith and moment"
- "With this regard their currents turn awry"
- "And lose the name of action."

---
keyword single plain:
- do:
search:
index: test
body:
query:
match_phrase:
foo: "the quick brown fox jumped over the lazy dog"
highlight:
fields:
foo:
type: plain
- match: { hits.hits.0.highlight.foo.0: <em>the quick brown fox jumped over the lazy dog</em> }

---
keyword multi plain:
- do:
search:
index: test
body:
query:
match_phrase:
foo: "That makes calamity of so long life."
highlight:
fields:
foo:
type: plain
- match: { hits.hits.0.highlight.foo.0: <em>That makes calamity of so long life.</em> }

---
keyword single unified:
- do:
search:
index: test
body:
query:
match_phrase:
foo: "the quick brown fox jumped over the lazy dog"
highlight:
fields:
foo:
type: unified
- match: { hits.hits.0.highlight.foo.0: <em>the quick brown fox jumped over the lazy dog</em> }

---
keyword multi unified:
- do:
search:
index: test
body:
query:
match_phrase:
foo: "That makes calamity of so long life."
highlight:
fields:
foo:
type: unified
- match: { hits.hits.0.highlight.foo.0: <em>That makes calamity of so long life.</em> }

---
keyword single fvh:
- do:
catch: /the field \[foo\] should be indexed with term vector with position offsets to be used with fast vector highlighter/
search:
index: test
body:
query:
match_phrase:
foo: "the quick brown fox jumped over the lazy dog"
highlight:
fields:
foo:
type: fvh

---
keyword multi fvh:
- do:
catch: /the field \[foo\] should be indexed with term vector with position offsets to be used with fast vector highlighter/
search:
index: test
body:
query:
match_phrase:
foo: "That makes calamity of so long life."
highlight:
fields:
foo:
type: fvh

---
text single plain:
- do:
search:
index: test
body:
query:
match_phrase:
foo.analyze: "the quick brown fox jumped over the lazy dog"
highlight:
fields:
foo.analyze:
type: plain
- match: { hits.hits.0.highlight.foo\.analyze.0: <em>the</em> <em>quick</em> <em>brown</em> <em>fox</em> <em>jumped</em> <em>over</em> <em>the</em> <em>lazy</em> <em>dog</em> }

---
text multi plain:
- do:
search:
index: test
body:
query:
match_phrase:
foo.analyze: "That makes calamity of so long life."
highlight:
fields:
foo.analyze:
type: plain
- match: { hits.hits.0.highlight.foo\.analyze.0: <em>That</em> <em>makes</em> <em>calamity</em> <em>of</em> <em>so</em> <em>long</em> <em>life</em>. }

---
text single unified from reanalysis:
- do:
search:
index: test
body:
query:
match_phrase:
foo.analyze: "the quick brown fox jumped over the lazy dog"
highlight:
fields:
foo.analyze:
type: unified
- match: { hits.hits.0.highlight.foo\.analyze.0: <em>the</em> <em>quick</em> <em>brown</em> <em>fox</em> <em>jumped</em> <em>over</em> <em>the</em> <em>lazy</em> <em>dog</em> }

---
text multi unified from reanalysis:
- do:
search:
index: test
body:
query:
match_phrase:
foo.analyze: "That makes calamity of so long life."
highlight:
fields:
foo.analyze:
type: unified
- match: { hits.hits.0.highlight.foo\.analyze.0: <em>That</em> <em>makes</em> <em>calamity</em> <em>of</em> <em>so</em> <em>long</em> <em>life</em>. }

---
text single unified from positions:
- do:
search:
index: test
body:
query:
match_phrase:
foo.positions: "the quick brown fox jumped over the lazy dog"
highlight:
fields:
foo.positions:
type: unified
- match: { hits.hits.0.highlight.foo\.positions.0: <em>the</em> <em>quick</em> <em>brown</em> <em>fox</em> <em>jumped</em> <em>over</em> <em>the</em> <em>lazy</em> <em>dog</em> }

---
text multi unified from positions:
- do:
search:
index: test
body:
query:
match_phrase:
foo.positions: "That makes calamity of so long life."
highlight:
fields:
foo.positions:
type: unified
- match: { hits.hits.0.highlight.foo\.positions.0: <em>That</em> <em>makes</em> <em>calamity</em> <em>of</em> <em>so</em> <em>long</em> <em>life</em>. }

---
text single unified from vectors:
- do:
search:
index: test
body:
query:
match_phrase:
foo.vectors: "the quick brown fox jumped over the lazy dog"
highlight:
fields:
foo.vectors:
type: unified
- match: { hits.hits.0.highlight.foo\.vectors.0: <em>the</em> <em>quick</em> <em>brown</em> <em>fox</em> <em>jumped</em> <em>over</em> <em>the</em> <em>lazy</em> <em>dog</em> }

---
text multi unified from vectors:
- do:
search:
index: test
body:
query:
match_phrase:
foo.vectors: "That makes calamity of so long life."
highlight:
fields:
foo.vectors:
type: unified
- match: { hits.hits.0.highlight.foo\.vectors.0: <em>That</em> <em>makes</em> <em>calamity</em> <em>of</em> <em>so</em> <em>long</em> <em>life</em>. }

---
text single fvh:
- do:
search:
index: test
body:
query:
match_phrase:
foo.vectors: "the quick brown fox jumped over the lazy dog"
highlight:
fields:
foo.vectors:
type: fvh
- match: { hits.hits.0.highlight.foo\.vectors.0: <em>the quick brown fox jumped over the lazy dog</em> }

---
text multi fvh:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test that this also occurs when you ask for fragments in order, so that we exercise both FragmentsBuilder implementations?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test that this also occurs when you ask for fragments in order, so that we exercise both FragmentsBuilder implementations?

👍

- do:
catch: /The fast vector highlighter doesn't support loading multi-valued fields from _source in index \[test\] because _source can reorder field values/
search:
index: test
body:
query:
match_phrase:
foo.vectors: "That makes calamity of so long life."
highlight:
fields:
foo.vectors:
type: fvh
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
* Loads source {@code _source} during a GET or {@code _search}.
*/
public interface SourceLoader {
/**
* Does this {@link SourceLoader} reorder field values?
*/
boolean reordersFieldValues();

/**
* Build the loader for some segment.
*/
Expand All @@ -43,6 +48,11 @@ interface Leaf {
* Load {@code _source} from a stored field.
*/
SourceLoader FROM_STORED_SOURCE = new SourceLoader() {
@Override
public boolean reordersFieldValues() {
return false;
}

@Override
public Leaf leaf(LeafReader reader) {
return new Leaf() {
Expand All @@ -64,6 +74,11 @@ public Synthetic(Mapping mapping) {
loader = mapping.getRoot().syntheticFieldLoader();
}

@Override
public boolean reordersFieldValues() {
return true;
}

@Override
public Leaf leaf(LeafReader reader) throws IOException {
SyntheticFieldLoader.Leaf leaf = loader.leaf(reader);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
package org.elasticsearch.search.fetch;

import org.apache.lucene.search.Query;
import org.elasticsearch.index.mapper.SourceLoader;
import org.elasticsearch.index.query.ParsedQuery;
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.search.SearchExtBuilder;
Expand Down Expand Up @@ -36,13 +37,15 @@ public class FetchContext {

private final SearchContext searchContext;
private final SearchLookup searchLookup;
private final SourceLoader sourceLoader;

/**
* Create a FetchContext based on a SearchContext
*/
public FetchContext(SearchContext searchContext) {
this.searchContext = searchContext;
this.searchLookup = searchContext.getSearchExecutionContext().lookup();
this.sourceLoader = searchContext.newSourceLoader();
}

/**
Expand Down Expand Up @@ -203,6 +206,13 @@ public SearchExecutionContext getSearchExecutionContext() {
return searchContext.getSearchExecutionContext();
}

/**
* Loads source {@code _source} during a GET or {@code _search}.
*/
public SourceLoader sourceLoader() {
return sourceLoader;
}

/**
* For a hit document that's being processed, return the source lookup representing the
* root document. This method is used to pass down the root source when processing this
Expand Down
Loading