Skip to content
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

[Docs] Clarify multi match query field limit #34801

Merged
merged 1 commit into from
Oct 25, 2018

Conversation

cbuescher
Copy link
Member

We limit the number of fields that a query can be expanded to to 1024 starting
with 7.0. However, I think that we don't enforce this in earlier versions
already. This change clarifies that while leaving a note that the limit is going
to be in effect in future versions and that a large number of fields can cause
problems.

Relates to #34778

@cbuescher cbuescher added >docs General docs changes :Search/Search Search-related issues that do not fall into other categories labels Oct 24, 2018
@cbuescher cbuescher requested a review from jimczi October 24, 2018 12:35
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

Copy link
Member Author

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

@jimczi I have a suspicion this got backported by accident, the limit is checked on master in QueryParserHelper, but I couldn't find any code enforcing this on the 6.x branches. I might be wrong about this, in which case we can simply close the issue.

We limit the number of fields that a query can be expanded to to 1024 starting
with 7.0. However, I think that we don't enforce this in earlier versions
already. This change clarifies that while leaving a note that the limit is going
to be in effect in future versions and that a large number of fields can cause
problems.
@cbuescher cbuescher force-pushed the docs-multiMatchLimits branch from f08e16a to 3ce6bf3 Compare October 25, 2018 09:02
Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

Good catch @cbuescher ! We didn't backport #26541 in 6x, I think that we should though. We need to log a deprecation warning in 6x if the number of fields reached the limit in order to warn users about the breaking change in 7. Can we handle the documentation update in the backport ?
I can do it if you don't have time, just say the word ;).

@cbuescher
Copy link
Member Author

@jimczi great, I can do the deprecation in the backport in a separate PR if that is okay. I think it makes sense to merge this docs PR separately and port it back all the way to 6.1 because this is were I think we start mentioning the limit now. I would add the deprecation warning only on 6.x and 6.4 branch since these are the only ones we still might do releases from.

@jimczi
Copy link
Contributor

jimczi commented Oct 25, 2018

I can do the deprecation in the backport in a separate PR if that is okay. I think it makes sense to merge this docs PR separately and port it back all the way to 6.1 because this is were I think we start mentioning the limit now.

Thanks and I agree that we can backport the docs change up to 6.1

I would add the deprecation warning only on 6.x and 6.4 branch since these are the only ones we still might do releases from.

I think 6.6 is enough and we can add it for 6.5.1 too but IMO it's too late for 6.4

@cbuescher
Copy link
Member Author

@jimczi thanks, would you approve this docs change then, I will merge it to the 6.x branches down to 6.1 and open a follow up for the deprecation warning

@cbuescher cbuescher merged commit 45435dd into elastic:6.x Oct 25, 2018
cbuescher pushed a commit that referenced this pull request Oct 25, 2018
We limit the number of fields that a query can be expanded to to 1024 starting
with 7.0. However, we don't enforce this in earlier versions already. This change 
clarifies that while leaving a note that the limit is going to be in effect in future 
versions and that a large number of fields can cause problems.
cbuescher pushed a commit that referenced this pull request Oct 25, 2018
We limit the number of fields that a query can be expanded to to 1024 starting
with 7.0. However, we don't enforce this in earlier versions already. This change 
clarifies that while leaving a note that the limit is going to be in effect in future 
versions and that a large number of fields can cause problems.
cbuescher pushed a commit that referenced this pull request Oct 25, 2018
We limit the number of fields that a query can be expanded to to 1024 starting
with 7.0. However, we don't enforce this in earlier versions already. This change
clarifies that while leaving a note that the limit is going to be in effect in future
versions and that a large number of fields can cause problems.
cbuescher pushed a commit that referenced this pull request Oct 25, 2018
We limit the number of fields that a query can be expanded to to 1024 starting
with 7.0. However, we don't enforce this in earlier versions already. This change
clarifies that while leaving a note that the limit is going to be in effect in future
versions and that a large number of fields can cause problems.
cbuescher pushed a commit that referenced this pull request Oct 25, 2018
We limit the number of fields that a query can be expanded to to 1024 starting
with 7.0. However, we don't enforce this in earlier versions already. This change
clarifies that while leaving a note that the limit is going to be in effect in future
versions and that a large number of fields can cause problems.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes :Search/Search Search-related issues that do not fall into other categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants