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

Remove DocumentMapperForType #72158

Closed
wants to merge 12 commits into from

Conversation

javanna
Copy link
Member

@javanna javanna commented Apr 23, 2021

No description provided.

@javanna javanna changed the title First attempt at removing DocumentMapperForType Remove DocumentMapperForType Apr 28, 2021
@javanna javanna added :Search Foundations/Mapping Index mappings, including merging and defining field types >refactoring labels Apr 30, 2021
@javanna
Copy link
Member Author

javanna commented Apr 30, 2021

Closing this. It was a good, unfortunately failed, experiment. The conclusion is that I was trying to take a shortcut which almost works but not entirely, which would cause us to have edge case situations where an index with only empty docs indexed has no mappings, which we don't want and it can lead to unexpected situations.

I have incorporated part of the changes that I had made in #72400, but without the removal that I was targeting initially. The approach we should take is to rather eagerly create a DocumentMapper whenever a new index is created. That way we don't have any situation where an index has no mappings, hence there is no need for DocumentMapperForType to exist. This approach has a couple of implications: we need to check all the consumers of MapperService#documentMapper and remove/replace their null checks; we need to have a different way to figure out whether an index is empty or not (may be useful to rewrite queries to match_none when there are no docs that can possibly match). Lastly, we cannot adopt this approach in 7.x, because a type that is not _doc is still accepted hence we can't eagerly create mappings for a given index, as its type may have to change later depending on what the user requests.

@javanna javanna closed this Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>refactoring :Search Foundations/Mapping Index mappings, including merging and defining field types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant