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

Unmapped fields are taken into account when checking the limit of full text query #49002

Closed
jimczi opened this issue Nov 12, 2019 · 4 comments
Closed
Labels
help wanted adoptme :Search/Search Search-related issues that do not fall into other categories

Comments

@jimczi
Copy link
Contributor

jimczi commented Nov 12, 2019

Today we have a limit in full-text query to limit the number of field expansion that we allow. The limit is controllable with the indices.query.bool.max_clause_count setting and we count all fields including unmapped ones. However for queries that target multiple indices it is possible that the list of field to expand the query to become bigger than the max clause setting even if the list of applicable field per index/shard is small. So for instance a query with 2000 fields where 1000 fields belong to one index and the other 1000 belong to the other index would fail because the total is greater than the max clause count. It is also important to note that we currently translate any unmapped field into a MatchNoDocsQuery to ease debugging but we could also completely ignore these unmapped fields if we wanted. I am opening this issue mainly to discuss this behavior and to assess whether it would be beneficial to only count the fields that will make a difference (the mapped ones).

@jimczi jimczi added :Search/Search Search-related issues that do not fall into other categories team-discuss labels Nov 12, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/Search)

@jimczi
Copy link
Contributor Author

jimczi commented Nov 18, 2019

We discussed this issue in our Search meeting and we've decided to not count unmapped fields when computing the field limit. Today field pattern that don't match anything in the mapping are not taken into account when computing the limit while explicit field are always counted. In order to be consistent concrete fields that don't match in the index mapping should be ignored too.

@jimczi jimczi added help wanted adoptme and removed team-discuss labels Nov 18, 2019
@zacharymorn
Copy link
Contributor

Hi @jimczi , I’m interested in taking this one, but may need more information to understand the scope of change better.

From your description of the issue & discussion result, we would like to ignore unmapped fields when comparing against the limit. I did some digging and looks like it can be achieved by local changes in the multiple QueryParserHelper.resolveMappingFields where checkForTooManyFields was called.

Also I’m not sure if I follow this

In order to be consistent concrete fields that don’t match in the index mapping should be ignored too.

Is this just rephrasing that unmapped fields should be ignored when comparing against limit?

@zacharymorn
Copy link
Contributor

@jimczi I've created a PR based on my understanding. Please let me know if that looks good to you.

@jimczi jimczi closed this as completed in 3fba171 Jan 20, 2020
jimczi pushed a commit that referenced this issue Jan 20, 2020
Take into account of number of unmapped fields when calculating against limit.
Closes #49002
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this issue Jan 23, 2020
Take into account of number of unmapped fields when calculating against limit. 
Closes elastic#49002
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted adoptme :Search/Search Search-related issues that do not fall into other categories
Projects
None yet
Development

No branches or pull requests

3 participants