Skip to content

Don't use _type field for nested paths #50312

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

Closed

Conversation

romseygeek
Copy link
Contributor

The _type field is not only used to store types (obsolete in 8.0) but also
the nested paths of child documents. This overload prevents us from removing
the _type field and TypeFieldType.

This commit changes nested field mappers to use a _nested_path field instead,
with some backwards-compatibility shims to ensure that pre-8.0 indexes still use
_type.

Closes #24362

@romseygeek romseygeek added :Search Foundations/Mapping Index mappings, including merging and defining field types >refactoring v8.0.0 labels Dec 18, 2019
@romseygeek romseygeek self-assigned this Dec 18, 2019
@elasticmachine
Copy link
Collaborator

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

@romseygeek
Copy link
Contributor Author

@elasticmachine update branch

@romseygeek romseygeek requested a review from jpountz January 2, 2020 12:51
Copy link
Contributor

@jtibshirani jtibshirani left a comment

Choose a reason for hiding this comment

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

@romseygeek I think it would be helpful to separate this PR into two:

  • Switch to _nested_path instead of _type to store nested paths.
  • Then follow-up with a change to remove TypeFieldMapper.

This helps in isolating these two big changes, which are related but still distinct. It's also not clear to me whether we can just remove TypeFieldMapper in 8.0 -- some of the conversations we've had around API versioning seem to suggest that we need logic to accept (but ignore) references to _type in the search request. Separating the PR into two would let us make progress on reviewing the _nested_path change while we get clarity on the API versioning questions.

@romseygeek
Copy link
Contributor Author

I've opened #51100 to deal with just adding the nested field mapper.

@romseygeek romseygeek closed this Jan 16, 2020
@jtibshirani
Copy link
Contributor

Thanks for splitting this up! I'll let someone else (maybe @jpountz?) review #51100, since I'm not as familiar with the history/ context there.

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 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rethink how to index nested documents when types are gone
4 participants