Skip to content

Set new providers before building FetchSubPhaseProcessors #97460

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 10, 2023
Merged

Set new providers before building FetchSubPhaseProcessors #97460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3740bb2
Set new providers before building FetchSubPhaseProcessors
romseygeek Jul 7, 2023
416215c
Update docs/changelog/97460.yaml
romseygeek Jul 7, 2023
59fcf96
Update 97460.yaml
romseygeek Jul 7, 2023
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog/97460.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 97460
summary: Set new providers before building `FetchSubPhaseProcessors`
area: "Search"
type: bug
issues:
- 96284
30 changes: 30 additions & 0 deletions modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/top_hits.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -720,3 +720,33 @@ synthetic _source:
buckets_path:
ts: top_salary_hits[salary]
script: "params.ts < 8000"

---
runtime fields:
- do:
search:
index: test
body:
runtime_mappings:
runtime_field:
type: keyword
script:
lang: painless
source: "emit(params['_source'].toString())"
fields: [runtime_field]
size: 0
aggs:
page:
terms:
field: page
aggs:
top_hits:
top_hits: {}

- match: { hits.total.value: 3 }
- length: { aggregations.page.buckets: 2 }
- match: { aggregations.page.buckets.0.key: 1 }
- match: { aggregations.page.buckets.0.top_hits.hits.hits.0.fields.runtime_field: ["{page=1, text=the quick brown fox}"] }
- match: { aggregations.page.buckets.0.top_hits.hits.hits.1.fields.runtime_field: ["{page=1, text=jumped over the lazy dog}"] }
- match: { aggregations.page.buckets.1.key: 2 }
- match: { aggregations.page.buckets.1.top_hits.hits.hits.0.fields.runtime_field: ["{page=2, text=The vorpal blade went snicker-snack!}"] }
8 changes: 4 additions & 4 deletions server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ private SearchHits buildSearchHits(SearchContext context, Profiler profiler) {
FetchContext fetchContext = new FetchContext(context);
SourceLoader sourceLoader = context.newSourceLoader();

PreloadedSourceProvider sourceProvider = new PreloadedSourceProvider();
PreloadedFieldLookupProvider fieldLookupProvider = new PreloadedFieldLookupProvider();
context.getSearchExecutionContext().setLookupProviders(sourceProvider, ctx -> fieldLookupProvider);
Copy link
Member

@javanna javanna Jul 7, 2023

Choose a reason for hiding this comment

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

Would it be possible to add a unit test as well? I guess it really has to do with how top hits interacts with the fetch phase?
Copy link
Contributor Author

@romseygeek romseygeek Jul 10, 2023

Choose a reason for hiding this comment

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

Yes, unit tests for this will be tricky. We don't really have unit tests for any of the FetchPhase at the moment as the context structures that it uses are so heavy.

List<FetchSubPhaseProcessor> processors = getProcessors(context.shardTarget(), fetchContext, profiler);

StoredFieldsSpec storedFieldsSpec = StoredFieldsSpec.build(processors, FetchSubPhaseProcessor::storedFieldsSpec);
Expand All @@ -110,10 +114,6 @@ private SearchHits buildSearchHits(SearchContext context, Profiler profiler) {

NestedDocuments nestedDocuments = context.getSearchExecutionContext().getNestedDocuments();

PreloadedSourceProvider sourceProvider = new PreloadedSourceProvider();
PreloadedFieldLookupProvider fieldLookupProvider = new PreloadedFieldLookupProvider();
context.getSearchExecutionContext().setLookupProviders(sourceProvider, ctx -> fieldLookupProvider);

FetchPhaseDocsIterator docsIterator = new FetchPhaseDocsIterator() {

LeafReaderContext ctx;
Expand Down