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: Reject non-integer weights on indexing #3977

Closed
spinscale opened this issue Oct 25, 2013 · 3 comments
Closed

Completion Suggester: Reject non-integer weights on indexing #3977

spinscale opened this issue Oct 25, 2013 · 3 comments

Comments

@spinscale
Copy link
Contributor

Right now the completion field mapper casts a float/double value to a long and stores it internally.

This may lead to confusion, because weight calculations then could possibly use the same weight, even though someone indexed a weight 4.2 and 4.5.

Solution is to simply reject such an index requests with an appropriate error message to use an integer - which might result in rejected index requests (thus the breaking tag).

spinscale added a commit that referenced this issue Oct 25, 2013
In order to make sure that people do not get confused, if they
index a float as weight, it makes more sense to reject it instead of
silently parsing it to an integer and using it.

The CompletionFieldMapper now checks for the type of the number which
is being read and throws and exception if the number is something else
than int or long.

Closes #3977
@uschindler
Copy link
Contributor

Hi @spinscale,

I cannot reopen issues, but in fact the problem here is not completely solved. Should I open a new issue? My problem is that this check introduced here only works if the weight is a number. Unfortunately the parser has a bit strange logic: Your new code is not executed if the client (creating the JSON) is passing the weight as "string", e.g. { "weight" : "10" }

In fact the weight is then ignored completely and not even an error is given (this is an additional bug). 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.

@clintongormley
Copy link

Hi @uschindler

yes please open another issue for this

@uschindler
Copy link
Contributor

OK. done.

mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015
In order to make sure that people do not get confused, if they
index a float as weight, it makes more sense to reject it instead of
silently parsing it to an integer and using it.

The CompletionFieldMapper now checks for the type of the number which
is being read and throws and exception if the number is something else
than int or long.

Closes elastic#3977
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants