-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Add enable_fields_emulation flag to search requests #123267
Conversation
Pinging @elastic/kibana-app-services (Team:AppServicesSv) |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
…arch-fields-emulation
@@ -32,6 +32,7 @@ export async function getIgnoreThrottled( | |||
export async function getDefaultAsyncSubmitParams( | |||
uiSettingsClient: Pick<IUiSettingsClient, 'get'>, | |||
searchSessionsConfig: SearchSessionsConfigSchema | null, | |||
params: any, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no any
please :)
@@ -44,7 +45,9 @@ export async function getDefaultAsyncSubmitParams( | |||
| 'ignore_unavailable' | |||
| 'track_total_hits' | |||
| 'keep_on_completion' | |||
> | |||
> & { | |||
enable_fields_emulation: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we do the same for es
search strategy ?
what about rollups ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the PR to support our default search strategy which is used by other strategies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maps changes LGTM
code review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Page load bundle
History
To update your PR or re-run it, just comment with: cc @lukasolson |
**Fixes:** #125059 ## Summary Fixes a bug in the EQL search strategy presumably introduced in #123267. ## Screenshots **Before** (request to EQL search strategy is failing, and the rule creation form can't perform validation) ![](https://puu.sh/II2fT/0ffde0f0d6.png) **After** (request to EQL search strategy is returning results, and the rule creation form can properly validate the query) ![](https://puu.sh/II2gJ/060ff92e77.png) ![](https://puu.sh/II2ex/d7fbb31517.png) ![](https://puu.sh/II2f2/09cc4483a3.png) ## Notes on testing Create a test index and write a few documents to it: ``` PUT /test-eql { "mappings": { "properties": { "@timestamp": { "type": "date" }, "process": { "type": "object", "properties": { "name": { "type": "keyword" } } } } } } POST /test-eql/_doc { "@timestamp": "2022-02-05T10:00:08Z", "process": { "name": "mdworker" } } ```
Summary
Related: elastic/elasticsearch#82539, elastic/elasticsearch#82485
Replaces #123010.
Adds the
enable_fields_emulation
flag to search requests so that fields show up in scenarios where CCS is in use with a cluster that doesn't support the newfields
option in ES.Checklist
Delete any items that are not applicable to this PR.
Risk Matrix
Delete this section if it is not applicable to this PR.
Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.
When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:
For maintainers
Release note
Fixes cross-cluster-search when using the
fields
search request parameter for versions of Elasticsearch prior to 7.0.