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 blended terms take 2 #15894

Merged
merged 1 commit into from Jan 15, 2016

Conversation

Projects
None yet
2 participants
@nik9000
Copy link
Contributor

commented Jan 11, 2016

Closes #15860

@nik9000

This comment has been minimized.

Copy link
Contributor Author

commented Jan 11, 2016

@s1monw I was a bit overzealous with that last one. I ended up merging it without running all the tests. Something about a weekend in between. I reverted it because it was pretty destructive. Anyway, this is the code from the last one in first commit and code to fix the fallout of the first commit in the second. Mostly it works around funkiness with setting an analyzer and using numeric or date fields.

@nik9000

This comment has been minimized.

Copy link
Contributor Author

commented Jan 12, 2016

I just blasted all the tags saying backport this very far into the past after talking with some other Elastic employees. This has been like this for a long time and its a fiddly change so I'd like it to soak in integration testing. Thus I'm targeting just master and 2.x branches. So the 3.0 and 2.3 releases.

@jpountz

View changes

core/src/main/java/org/elasticsearch/index/search/MatchQuery.java Outdated
QueryParsers.setRewriteMethod((FuzzyQuery) query, fuzzyRewriteMethod);
}
return query;
} catch (Exception e) {

This comment has been minimized.

Copy link
@jpountz

jpountz Jan 13, 2016

Contributor

it looks a bit inconsistent to catch Exception here and RuntimeException above?

@jpountz

This comment has been minimized.

Copy link
Contributor

commented Jan 13, 2016

LGTM. I agree this fix is to subtle to be backported to bugfix releases.

Fix blended terms for non-strings take 2
It had some funky errors, like lenient:true not working and queries with
two integer fields blowing up if there was no analyzer defined on the
query. This throws a bunch more tests at it and rejiggers how non-strings
are handled so they don't wander off into scary QueryBuilder-land unless
they have a nice strong analyzer to protect them.

@nik9000 nik9000 force-pushed the nik9000:fix_blended_terms_2 branch to 50098bf Jan 15, 2016

nik9000 added a commit that referenced this pull request Jan 15, 2016

@nik9000 nik9000 merged commit 7745c64 into elastic:master Jan 15, 2016

1 check passed

CLA Commit author has signed the CLA
Details
@nik9000

This comment has been minimized.

Copy link
Contributor Author

commented Jan 15, 2016

And cherry picked to 2.x: 1c95b75

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.