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

Comments

Projects
None yet
3 participants
@spinscale
Copy link
Member

commented Oct 25, 2013

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 spinscale closed this in f9154de Oct 25, 2013

spinscale added a commit that referenced this issue Oct 25, 2013

CompletionFieldMapper: Return error if weight is no integer
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

This comment has been minimized.

Copy link
Contributor

commented Oct 15, 2014

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

This comment has been minimized.

Copy link
Member

commented Oct 15, 2014

Hi @uschindler

yes please open another issue for this

@uschindler

This comment has been minimized.

Copy link
Contributor

commented Oct 15, 2014

OK. done.

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

CompletionFieldMapper: Return error if weight is no integer
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
You can’t perform that action at this time.