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

Fixes breaking change introduced in #95558. #95711

Merged
merged 3 commits into from
May 2, 2023

Conversation

quux00
Copy link
Contributor

@quux00 quux00 commented May 1, 2023

Match and unmatch fields in dynamic_templates (match, unmatch, path_match, path_unmatch) can either be single-valued or a list of values. If a list of values is provided, the values must all be strings. Numbers, boolean, other lists, etc. are not allowed and an error will be returned to the user in that case.

However, previously (before #95558) we allowed any JSON type for the match/unmatch fields (not just string), so changing that would be a breaking change. Thus, this commit no longer enforces string-only types for single-valued match/unmatch fields in dynamic_templates.

@quux00 quux00 added >bug :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Meta label for search team v8.9.0 labels May 1, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 1, 2023

Documentation preview:

@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the external-contributor Pull request authored by a developer outside the Elasticsearch team label May 1, 2023
@quux00 quux00 requested review from romseygeek and javanna May 1, 2023 20:50
@quux00 quux00 removed the external-contributor Pull request authored by a developer outside the Elasticsearch team label May 1, 2023
Copy link
Member

@javanna javanna left a comment

Choose a reason for hiding this comment

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

Looks great, thanks. Left one small comment around docs.

*Note:* when specifying patterns (for `match`, `unmatch`, etc.) in an array,
{es} enforces that all values must be strings. However, for backwards
compatibility, {es} allows non-string values (such as numbers) for
single-valued match (and unmatch) fields.
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should document this. My worry is that users may end up relying on this behaviour, while it is there only to prevent breaking templates. We could consider as a follow-up deprecating non string single values. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I agree. Users should probably never use anything except strings, so probably good not to encourage the other usage. I'll remove it.

@quux00 quux00 removed the v8.9.0 label May 2, 2023
quux00 added 3 commits May 2, 2023 08:43
Match and unmatch fields in dynamic_templates (match, unmatch, path_match, path_unmatch)
can either be single-valued or a list of values. If a list of values is provided, the
values must all be strings. Numbers, boolean, other lists, etc. are not allowed and an
error will be returned to the user in that case.

However, previously (before elastic#95558) we allowed any JSON type for the match/unmatch
fields (not just string), so changing that would be a breaking change. Thus, this commit
no longer enforces string-only types for single-valued match/unmatch fields in dynamic_templates.
@quux00 quux00 force-pushed the mapping/dyntemplates-with-arrays-2 branch from 71fe7ef to 8e6a0d4 Compare May 2, 2023 12:44
@quux00 quux00 merged commit 7d243a4 into elastic:main May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>non-issue :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Meta label for search team v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants