[KQL] Skip slow wildcard checks when query is only *#96902
Merged
wylieconlon merged 4 commits intoelastic:masterfrom Apr 20, 2021
Merged
[KQL] Skip slow wildcard checks when query is only *#96902wylieconlon merged 4 commits intoelastic:masterfrom
wylieconlon merged 4 commits intoelastic:masterfrom
Conversation
wylieconlon
added
release_note:fix
Feature:KQL
Contributor
|
Pinging @elastic/kibana-app-services (Team:AppServices) |
ppisljar
reviewed
Apr 15, 2021
|
|
||
| const isExistsQuery = valueArg.type === 'wildcard' && valueArg.value === wildcard.wildcardSymbol; | ||
| const isAllFieldsQuery = | ||
| fieldNameArg.type === 'wildcard' && fieldNameArg.value === wildcard.wildcardSymbol; |
Contributor
There was a problem hiding this comment.
we were using fullFieldNameArg before, is that relevant ?
Contributor
Author
There was a problem hiding this comment.
@ppisljar The fullFieldNameArg is only used when we are dealing with specific field wildcards, so for example nested.* would be matched by that logic. Since this case checks that the fieldName is exactly *, we can guarantee that there every field matches.
| const isMatchAllQuery = isExistsQuery && isAllFieldsQuery; | ||
|
|
||
| if (isMatchAllQuery) { | ||
| if (isExistsQuery && fields && fields.length === indexPattern?.fields.length) { |
Contributor
There was a problem hiding this comment.
before we used OR betwen these conditions
wildcardStuff || fields.length === indexpatternfields.length
Contributor
Author
There was a problem hiding this comment.
You're right, I will use an OR here. The special case that I missed, but was in the previous logic, is:
- There is no index pattern
- The query is a wildcard query
- It should shortcut and be a match_all
Contributor
Author
|
@elasticmachine merge upstream |
Contributor
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / Chrome UI Functional Tests.test/functional/apps/discover/_runtime_fields_editor·ts.discover app discover integration with runtime fields editor allows adding custom label to existing fieldsStandard OutStack TraceMetrics [docs]Page load bundle
History
To update your PR or re-run it, just comment with: |
wylieconlon
pushed a commit
to wylieconlon/kibana
that referenced
this pull request
Apr 20, 2021
* [KQL] Skip slow wildcard checks when query is only * * Fix case without index pattern Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
wylieconlon
pushed a commit
that referenced
this pull request
Apr 21, 2021
* [KQL] Skip slow wildcard checks when query is only * * Fix case without index pattern Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
wylieconlon
pushed a commit
to wylieconlon/kibana
that referenced
this pull request
Apr 21, 2021
* [KQL] Skip slow wildcard checks when query is only * * Fix case without index pattern Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
wylieconlon
pushed a commit
that referenced
this pull request
Apr 21, 2021
* [KQL] Skip slow wildcard checks when query is only * * Fix case without index pattern Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
1 task
lukasolson
added a commit
that referenced
this pull request
May 29, 2024
## Summary The following code was was introduced in #96902: https://github.com/elastic/kibana/blob/69b28f317b07af12fa346571b04b411a6130b25f/packages/kbn-es-query/src/kuery/functions/is.ts#L109-L112 As far as I can tell it was really just added to ensure that a contrived behavior stayed exactly the same after the optimization that PR made. I thought it was related to the case where we have a query like `*` or `*:*` (in other words, a query that matches all documents) but we are already handling that in this code: https://github.com/elastic/kibana/blob/69b28f317b07af12fa346571b04b411a6130b25f/packages/kbn-es-query/src/kuery/functions/is.ts#L62-L68 As we are moving to a scenario where we expect the field list passed to this code to be a subset of the entire list of fields (see #183694), this condition and the corresponding contrived test are removed in this PR. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
KQL uses a very slow regular expression to validate fields for wildcard queries, but for some reason this slow test was being applied when the query was empty or
*, which are both special cases. This looks like it has been happening in every version of Kibana since we introduced support for Nested type fields in KQL.Checklist
Delete any items that are not applicable to this PR.