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 ability to set the value of meta fields inside _source #11074

Merged
merged 2 commits into from May 13, 2015

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented May 9, 2015

A few meta fields can currently be set within a document's source.
However, the recommended way to set meta fields like this is through
the api, and setting within the document can be a performance trap
(e.g. needing to find _id in order to route the document).

This change removes the ability to set meta fields within
a document source for 2.0+ indexes.

closes #11051

@rjernst rjernst added v2.0.0-beta1 :Search/Mapping Index mappings, including merging and defining field types labels May 9, 2015
if (!rootMapper.includeInObject() && rootMapper instanceof FieldMapper) {
newFieldMappers.add((FieldMapper<?>) rootMapper);
if (rootMapper instanceof FieldMapper) {
newFieldMappers.add((FieldMapper)rootMapper);
Copy link
Contributor

Choose a reason for hiding this comment

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

s/FieldMapper/FieldMapper<?>/ ?

@jpountz
Copy link
Contributor

jpountz commented May 9, 2015

This looks great! I think we should also add a note about this to the migration guide and have a simple test that checks that specifying these fields still works on old indices?

@rjernst rjernst force-pushed the pr/include-in-object-removal branch from 5c9fe7a to ffd59e1 Compare May 11, 2015 18:35
rjernst added a commit to rjernst/elasticsearch that referenced this pull request May 11, 2015
A few meta fields can currently be set within a document's source.
However, the recommended way to set meta fields like this is through
the api, and setting within the document can be a performance trap
(e.g. needing to find _id in order to route the document).

This change removes the ability to set meta fields within
a document source for 2.0+ indexes.

closes elastic#11051
closes elastic#11074
@rjernst
Copy link
Member Author

rjernst commented May 13, 2015

@jpountz I pushed a commit adding a migration note and tests. I actually found while testing that only _id and _parent actually worked when specified in a document. The others would result in ignoring the field, which I am testing for true "backcompat".

A few meta fields can currently be set within a document's source.
However, the recommended way to set meta fields like this is through
the api, and setting within the document can be a performance trap
(e.g. needing to find _id in order to route the document).

This change removes the ability to set meta fields within
a document source for 2.0+ indexes.

closes elastic#11051
closes elastic#11074
@rjernst rjernst force-pushed the pr/include-in-object-removal branch from ffd59e1 to 1df807d Compare May 13, 2015 06:09
@rjernst rjernst force-pushed the pr/include-in-object-removal branch from 1df807d to f766b26 Compare May 13, 2015 06:11
@jpountz
Copy link
Contributor

jpountz commented May 13, 2015

LGTM

rjernst added a commit that referenced this pull request May 13, 2015
Mappings: Remove ability to set meta fields inside documents
@rjernst rjernst merged commit 1b15333 into elastic:master May 13, 2015
@rjernst rjernst deleted the pr/include-in-object-removal branch May 13, 2015 06:25
@clintongormley clintongormley changed the title Mappings: Remove ability to set meta fields inside documents Remove ability to set the value of meta fields inside _source Jun 6, 2015
rjernst added a commit to rjernst/elasticsearch that referenced this pull request Oct 7, 2015
We previously removed the ability to specify metadata fields inside
documents in elastic#11074, but the backcompat left leniency that allowed this
to still occur. This change locks down parsing so any metadata field
found while parsing a document results in an exception. This only
affects 2.0+ indexes; backcompat is maintained.

closes elastic#13740
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>breaking :Search/Mapping Index mappings, including merging and defining field types v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enforce meta fields are set through the api, not inside a document
3 participants