-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
SavedObjects search_dsl: add match_phrase_prefix clauses when using prefix search #82693
SavedObjects search_dsl: add match_phrase_prefix clauses when using prefix search #82693
Conversation
Pinging @elastic/kibana-platform (Team:Platform) |
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.
Left a question, but otherwise lgtm
return mppFields.map(({ field, boost }) => { | ||
return { | ||
match_phrase_prefix: { | ||
[field]: { | ||
query, | ||
boost, | ||
}, | ||
}, | ||
}; | ||
}); |
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.
Don't really know how this API works, but does it matter that we're creating a query like:
"should": [
{
"simple_query_string": {
"query": "my-vis*",
"fields": [
"visualization.title",
"visualization.description"
],
"default_operator": "OR"
}
},
{
"match_phrase_prefix": {
"visualization.title": {
"query": "my-vis",
"boost": 1
}
}
},
{
"match_phrase_prefix": {
"visualization.description": {
"query": "my-vis",
"boost": 1
}
}
}
]
I was expecting it to be:
"should": [
{
"simple_query_string": {
"query": "my-vis*",
"fields": [
"visualization.title",
"visualization.description"
],
"default_operator": "OR"
}
},
{
"match_phrase_prefix": {
"visualization.title": {
"query": "my-vis",
"boost": 1
},
"visualization.description": {
"query": "my-vis",
"boost": 1
}
}
}
]
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 also was tbh. But match_phrase
prefix does not work that way unfortunately (and even if it was, I would expect your syntax to be a AND condition between the fields, where we want a OR). That can creates a lot of clauses in the bool query, but when I discussed it with the ES teams, they told me it should not impact performances in any significative way.
❤️ |
…refix search (elastic#82693) * add match_phrase_prefix clauses when using prefix search * add FTR tests
…refix search (elastic#82693) * add match_phrase_prefix clauses when using prefix search * add FTR tests # Conflicts: # src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts # src/core/server/saved_objects/service/lib/search_dsl/query_params.ts
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
…using prefix search (#82693) (#82935) * SavedObjects search_dsl: add match_phrase_prefix clauses when using prefix search (#82693) * add match_phrase_prefix clauses when using prefix search * add FTR tests # Conflicts: # src/core/server/saved_objects/service/lib/search_dsl/query_params.test.ts # src/core/server/saved_objects/service/lib/search_dsl/query_params.ts * fix test file due to merge * fix files again
* master: Added `defaultActionMessage` to index threshold alert UI type definition (elastic#80936) [ILM] Migrate Delete phase and name field to Form Lib (elastic#82834) skip flaky suite (elastic#57426) [Alerting] adds an Run When field in the alert flyout to assign the action to an Action Group (elastic#82472) [APM] Expose APM event client as part of plugin contract (elastic#82724) [Logs UI] Fix errors during navigation (elastic#78319) Enable send to background in TSVB (elastic#82835) SavedObjects search_dsl: add match_phrase_prefix clauses when using prefix search (elastic#82693) [Ingest Manager] Unify install* under installPackage (elastic#82916)
…e-details-overlay * 'master' of github.com:elastic/kibana: (201 commits) Added `defaultActionMessage` to index threshold alert UI type definition (elastic#80936) [ILM] Migrate Delete phase and name field to Form Lib (elastic#82834) skip flaky suite (elastic#57426) [Alerting] adds an Run When field in the alert flyout to assign the action to an Action Group (elastic#82472) [APM] Expose APM event client as part of plugin contract (elastic#82724) [Logs UI] Fix errors during navigation (elastic#78319) Enable send to background in TSVB (elastic#82835) SavedObjects search_dsl: add match_phrase_prefix clauses when using prefix search (elastic#82693) [Ingest Manager] Unify install* under installPackage (elastic#82916) [Fleet] Make stream id unique in agent policy (elastic#82447) skip flaky suite (elastic#82915) skip flaky suite (elastic#75794) Copy `dateAsStringRt` to observability plugin (elastic#82839) [Maps] rename connected_components/map folder to mb_map (elastic#82897) [Security Solution] Fix EventsViewer DnD cypress tests (elastic#82619) [Security Solution] Adds logging and performance fan out API for threat/Indicator matching (elastic#82546) Implemented Alerting health status pusher by using task manager and status pooler for Kibana status plugins 'kibanahost/api/status' (elastic#79056) [APM] Adds new configuration 'xpack.apm.maxServiceEnvironments' (elastic#82090) Move single use function in line (elastic#82885) [ML] Add unsigned_long support to data frame analytics and anomaly detection (elastic#82636) ...
Summary
Fix #5734 Fix #20242
See #5734 (comment) for more context about the implementation design.
getQueryParams
to add per-fieldmatch_phrase_prefix
clauses when the user is performing a prefix search (*
wildcard at the end of the query)getQueryParams
to no longer relies on mapping, and uses the SO type registry as single source of truth regarding declared typesChecklist
Release notes
Fix a bug causing searching for saved objects using special characters such as
*
or-
to not return any results.