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

Serialize doc values settings for _timestamp #8967

Merged
merged 1 commit into from Jan 12, 2015

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Dec 15, 2014

This change fixes _timestamp's serialization method to write out
doc_values and doc_values_format, which could already be set,
but would not be written out.

closes #8893

@@ -277,6 +279,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (includeDefaults || fieldType.stored() != Defaults.FIELD_TYPE.stored()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

There is also a line above that checks if all options have the default value and returns early. I think we should add the doc-values-related settings to this line as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

All metadata mappers have this kind of behaviour which is a pita to maintain... I'm wondering if we could come up with a better solution.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jpountz I added the default value check in the block you were referring to. I also created #9093 to address the repetitive and error prone pattern here.

@jpountz
Copy link
Contributor

jpountz commented Dec 17, 2014

Thanks for fixing this, I just left a comment about the detection that all parameters have the default value.

@rjernst
Copy link
Member Author

rjernst commented Jan 12, 2015

@jpountz I created a follow up issue, #9093, for simplifying how we handle defaults here.

@jpountz
Copy link
Contributor

jpountz commented Jan 12, 2015

LGTM

Thanks for opening this new issue!

This change fixes _timestamp's serialization method to write out
`doc_values` and `doc_values_format`, which could already be set,
but would not be written out.

closes elastic#8893
closes elastic#8967
@rjernst rjernst removed the review label Jan 12, 2015
@rjernst rjernst merged commit 7d5a15e into elastic:master Jan 12, 2015
rjernst added a commit that referenced this pull request Jan 12, 2015
rjernst added a commit that referenced this pull request Jan 12, 2015
This change fixes _timestamp's serialization method to write out
`doc_values` and `doc_values_format`, which could already be set,
but would not be written out.

closes #8893
closes #8967
rjernst added a commit that referenced this pull request Jan 12, 2015
This change fixes _timestamp's serialization method to write out
`doc_values` and `doc_values_format`, which could already be set,
but would not be written out.

closes #8893
closes #8967
rjernst added a commit that referenced this pull request Jan 12, 2015
rjernst added a commit that referenced this pull request Jan 12, 2015
rjernst added a commit that referenced this pull request Jan 12, 2015
@clintongormley clintongormley added the :Search/Mapping Index mappings, including merging and defining field types label Feb 10, 2015
@rjernst rjernst deleted the fix/8893 branch March 24, 2015 02:57
@clintongormley clintongormley changed the title Mapping: serialize doc values settings for _timestamp Serialize doc values settings for _timestamp Jun 7, 2015
mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
This change fixes _timestamp's serialization method to write out
`doc_values` and `doc_values_format`, which could already be set,
but would not be written out.

closes elastic#8893
closes elastic#8967
mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Mapping Index mappings, including merging and defining field types v1.4.3 v1.5.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mappings: _timestamp does not serialize its doc_values settings
3 participants