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

Elasticsearch should reject dynamic templates with unknown match_mapping_type. #17285

Merged

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Mar 23, 2016

When looking at the logstash template, I noticed that it has definitions for
dynamic temilates with match_mapping_type equal to byte for instance.
However elasticsearch never tries to find templates that match the byte type
(only long or double as far as numbers are concerned). This commit changes
template parsing in order to ignore bad values of match_mapping_type (given
how the logstash template is popular, this would break many upgrades
otherwise). Then I hope to fail the parsing on bad values in 6.0.

Relates to #16945

@jpountz jpountz added >enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types v6.0.0-alpha1 labels Mar 23, 2016
@jpountz jpountz force-pushed the fix/template_validate_match_mapping_type branch 2 times, most recently from 24a4c8d to 54be98e Compare March 23, 2016 15:14
@jpountz jpountz added the review label Mar 24, 2016
@rjernst
Copy link
Member

rjernst commented Apr 11, 2016

LGTM. For the future around this, it would be nice if this weren't a public enum. It seems like this enum is really used in place of a map.

@jpountz
Copy link
Contributor Author

jpountz commented Apr 11, 2016

For the future around this, it would be nice if this weren't a public enum.

Agreed. we will want to collapse the mapper sub packages at some point (waiting to be done with points integration before doing so).

@dakrone
Copy link
Member

dakrone commented Jul 11, 2016

@jpountz this has a +1, should it be merged?

…ping_type`. elastic#17285

When looking at the logstash template, I noticed that it has definitions for
dynamic temilates with `match_mapping_type` equal to `byte` for instance.
However elasticsearch never tries to find templates that match the byte type
(only long or double as far as numbers are concerned). This commit changes
template parsing in order to ignore bad values of `match_mapping_type` (given
how the logstash template is popular, this would break many upgrades
otherwise). Then I hope to fail the parsing on bad values in 6.0.
@jpountz jpountz force-pushed the fix/template_validate_match_mapping_type branch from 54be98e to 0854b03 Compare July 19, 2016 15:54
@jpountz jpountz merged commit 0854b03 into elastic:master Jul 19, 2016
@jpountz jpountz deleted the fix/template_validate_match_mapping_type branch July 19, 2016 15:55
dakrone added a commit to dakrone/elasticsearch that referenced this pull request Dec 12, 2016
When using dynamic templates, ES will now throw an exception if a
`match_mapping_type` is used that doesn't correspond to an actual type.

Relates to elastic#17285
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types v5.0.0-alpha5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants