Remove highlight query

[lukasolson]

Fixes #13096.
Replaces #13185.

This PR changes the behavior of highlighting. Instead of using a separate highlight_query with "all_fields": true or "default_field": "*", this adds another parameter to the default query string parameters: "default_field": "*".

This should keep the highlighting behavior we desire without all the extra logic of adding the highlight_query to the request.

As a result of this change, anyone who has modified the existing advanced setting, query:queryString:options, will have to update it to include this parameter if they want highlighting to work across all fields. Also, anyone who has a default_field set in their index will have to update the advanced setting and remove the default_field parameter.

See #9671 for the original PR that added the highlight_query behavior.

[lukasolson]

@Bargs and @weltenwort, could you give me your thoughts on this PR vs. #13185? Thanks!

[Bargs]

I'm all for this change. The more I think about it the more I like it. It removes a lot of complexity from the code. It makes Kibana more consistent (the default_field will always be * instead of relying on whatever is in the index) while still giving users flexibility if they need it. I think we should set sane defaults like this more often because it makes Kibana's behavior less dependent on things defined outside of Kibana.

The only scenario I can think of where #13185 has an advantage is if the user has a mix of indices with and without _all and they want to be able to search on _all when it's available while still highlighting with all_fields. I think that's a pretty unlikely scenario, and _all is going away for good soon so we shouldn't optimize for that anyway.

[weltenwort]

This definitely looks better. But I don't feel I understand the reason why this is suddenly a viable option. I guess default_field has different semantics than all_fields when it comes to highlighting?

[lukasolson]

@weltenwort It's only a viable option for 6.x because it's technically a breaking change. We didn't want to break anything for 5.x which is why we went with a highlight_query with all_fields, but we said we'd probably have a different solution for 6.x.

[weltenwort]

That makes sense, thank you. IMHO we should take any chance we can find to get rid of such workarounds. 👍

[Bargs]

Functionality looks good.

I think we can remove all the code related to the highlightAll param on the SearchSource as well can't we?

kibana/src/ui/public/courier/data_source/_abstract.js

Line 335 in d379e9a

if (flatState.highlightAll != null) {

and

kibana/src/ui/public/courier/data_source/search_source.js

Line 96 in d379e9a

'highlightAll',

for example

[weltenwort]

Once nice thing about the highlightAll setting on the SearchSource was that the rather complex highlight configuration was not scattered throughout the code (see 909b8c7#diff-b1e7be471202c5be24e13d990666b5e2L486). Maybe we can retain that by providing shorthands for that, e.g. searchSource.highlight(createHighlightAllSetting()) and searchSource.highlight(createHighlightFieldsSetting(['field1', 'field2']))?

[Bargs]

Ah I'm remembering now, we needed highlightAll because not every request needs highlighting and apps needed a way to tell the search source if it should be added or not. I like that the actual ES highlighting syntax doesn't need to be stored in the saved object. The functionality could probably stay as is, highlightAll is just a bit of a misnomer. It would be nice to give it a different name... but if that causes migration headaches I wouldn't be entirely opposed to just leaving it.

[weltenwort]

jenkins, test this

[lukasolson]

6.x (6.1.0): 9bdba7c
6.0 (6.0.0): 3a755d9