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

Completion Suggester: Fix CompletionFieldMapper to correctly parse weight #8090

Closed
uschindler opened this Issue Oct 15, 2014 · 1 comment

Comments

Projects
None yet
4 participants
@uschindler
Copy link
Contributor

uschindler commented Oct 15, 2014

Followup for #3977:

if #3977 a check was added, that indexing completion suggester fields are rejected, if the weight is not an integer. My problem is that this check introduced there only works if the weight is a number. Unfortunately the parser has a bit strange logic: The new code is not executed if the client (creating the JSON) is passing the weight as "string", e.g. { "weight" : "10.5" }

In fact the weight is then ignored completely and not even an error is given (this is an additional bug in the parser logic). This caused me headaches yesterday, because the weight was given as JSON string in the indexing document. For other fields this makes no difference while indexing.

The parser for completion fields should be improved to have the outer check on the JSON key first and later check the types, not vice versa. This would also be consistent with indexing other fields, where the type of JSON value does not matter.

@clintongormley clintongormley added >bug and removed >bug labels Oct 15, 2014

@uschindler uschindler changed the title Completion Suggester: Fix indexing field parser to correctly parse weight Completion Suggester: Fix CompletionFieldMapper to correctly parse weight Oct 15, 2014

areek added a commit to areek/elasticsearch that referenced this issue Oct 28, 2014

Completion Suggester: Fix CompletionFieldMapper to correctly parse we…
…ight

 - Allows weight to be defined as a string representation of a positive integer

closes elastic#8090

@areek areek closed this in #8197 Oct 28, 2014

areek added a commit that referenced this issue Oct 28, 2014

Completion Suggester: Fix CompletionFieldMapper to correctly parse we…
…ight

 - Allows weight to be defined as a string representation of a positive integer

closes #8090

areek added a commit that referenced this issue Oct 28, 2014

Completion Suggester: Fix CompletionFieldMapper to correctly parse we…
…ight

 - Allows weight to be defined as a string representation of a positive integer

closes #8090

areek added a commit that referenced this issue Oct 29, 2014

Completion Suggester: Fix CompletionFieldMapper to correctly parse we…
…ight

 - Allows weight to be defined as a string representation of a positive integer

closes #8090
@s1monw

This comment has been minimized.

Copy link
Contributor

s1monw commented Oct 29, 2014

@areek FYI I cherry-picked this commit into 1.4 as well I thing you missed it :)

mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015

Completion Suggester: Fix CompletionFieldMapper to correctly parse we…
…ight

 - Allows weight to be defined as a string representation of a positive integer

closes elastic#8090

mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015

Completion Suggester: Fix CompletionFieldMapper to correctly parse we…
…ight

 - Allows weight to be defined as a string representation of a positive integer

closes elastic#8090
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.