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

Allow to disable norms on an existing field. #5511

Closed
wants to merge 2 commits into from

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Mar 24, 2014

I followed @nik9000 's idea to carefully document the caveat of disabling
norms as opposed to trying to wrap readers at search time to hide norms.

Close #4813

@s1monw
Copy link
Contributor

s1monw commented Mar 24, 2014

LGTM this should go 1.2 and 2.0 no?

@jpountz
Copy link
Contributor Author

jpountz commented Mar 24, 2014

that was my plan

@@ -620,6 +620,7 @@ public void merge(Mapper mergeWith, MergeContext mergeContext) throws MergeMappi

if (!mergeContext.mergeFlags().simulate()) {
// apply changeable values
this.fieldType = fieldMergeWith.fieldType;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm worried that by copying over the field type completely we get more stuff then what we tested for collisions. For example, indexOptions is not tested. Maybe there are other stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I'll make a copy.

@jpountz
Copy link
Contributor Author

jpountz commented Mar 25, 2014

@bleskes Can you check if the last commit addresses your concerns? Thanks!

@bleskes
Copy link
Contributor

bleskes commented Mar 25, 2014

@jpountz it does. Thx.

@nik9000
Copy link
Member

nik9000 commented Mar 25, 2014

Thanks!

@jpountz jpountz closed this Mar 26, 2014
@jpountz jpountz deleted the feature/disabling_norms branch March 26, 2014 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Norms disabling on existing fields
4 participants