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

Fix FuzzyQuery to properly handle Object, number, dates or String. #12020

Closed

Conversation

alexksikes
Copy link
Contributor

This makes FuzzyQueryBuilder and Parser take an Object as a value using the
same logic as termQuery, so that numbers, dates or Strings would be properly
handled.

Relates #11865

Makes FuzzyQueryBuilder take a value as a number, date or String using exactly
the same logic as TermQueryBuilder.

Relates elastic#11865
@jpountz
Copy link
Contributor

jpountz commented Jul 3, 2015

I think we also wanted to change the parser to parse the value as an object instead of a string?

@alexksikes
Copy link
Contributor Author

Yep I wasn't sure of that, I'll fix this.

public Query fuzzyQuery(String value, Fuzziness fuzziness, int prefixLength, int maxExpansions, boolean transpositions) {
long iValue = dateMathParser().parse(value, now());
public Query fuzzyQuery(Object value, Fuzziness fuzziness, int prefixLength, int maxExpansions, boolean transpositions) {
long iValue = dateMathParser().parse(value.toString(), now());
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use parseValue(value) instead? It should behave as similarly to termQuery() as possible.

@jpountz
Copy link
Contributor

jpountz commented Jul 7, 2015

I left some comments.

@jpountz
Copy link
Contributor

jpountz commented Jul 7, 2015

LGTM

Query query = null;
MappedFieldType fieldType = parseContext.fieldMapper(fieldName);
if (fieldType != null) {
query = fieldType.fuzzyQuery(value, fuzziness, prefixLength, maxExpansions, transpositions);
}
if (query == null) {
query = new FuzzyQuery(new Term(fieldName, value), fuzziness.asDistance(value), prefixLength, maxExpansions, transpositions);
query = new FuzzyQuery(new Term(fieldName, value.toString()), fuzziness.asDistance(value.toString()), prefixLength, maxExpansions, transpositions);
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't we use BytesRefs.toBytesRef(value) here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep I was just updating the PR as you wrote this.

@alexksikes
Copy link
Contributor Author

@jpountz @javanna Thanks for the review. I updated the PR accordingly.

@javanna
Copy link
Member

javanna commented Jul 8, 2015

LGTM

@javanna javanna added the :Core/Infra/Transport API Transport client API label Jul 8, 2015
@alexksikes alexksikes changed the title FuzzyQueryBuilder: value as an Object, number, date or String. Fix FuzzyQuery to properly handle Object, number, dates or String. Jul 8, 2015
@alexksikes alexksikes closed this in a6c0007 Jul 8, 2015
@alexksikes alexksikes deleted the feature/fuzzy-query-value-object branch July 8, 2015 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants