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 MapperService#searchFilter(...) #15923

Merged
merged 1 commit into from Jan 13, 2016

Conversation

Projects
None yet
4 participants
@martijnvg
Copy link
Member

commented Jan 12, 2016

Search filter should wrap the types filters in a separate boolean as should clauses

So that a document must either match with one of the types and the non nested clause.

Closes #15757

@nik9000

This comment has been minimized.

Copy link
Contributor

commented Jan 12, 2016

LGTM. I'd have liked to see a test that verifies what can be found but with our desire to limit integration tests I think it'd be a pain. I think it'd be more obvious what its fixing.

@jpountz

This comment has been minimized.

Copy link
Contributor

commented Jan 12, 2016

LGTM2. I think that as a follow-up we should look into simplifying this method and:

  • remove the useTermsFilter optimization: given that TermsFilter rewrites to a BooleanQuery when there are less then 16 terms anyway it doesn't change anything
  • refactor the code so that we don't have 3 completely different branches for the no type, one type and several types cases
@martijnvg

This comment has been minimized.

Copy link
Member Author

commented Jan 12, 2016

thanks @nik9000 @jpountz! I opened #15924 to simplify this method.

mappings: Search filter should wrap the types filters in a separate b…
…oolean as should clauses

So that a document must either match with one of the types and the non nested clause.

Closes #15757

@martijnvg martijnvg force-pushed the martijnvg:mappings/fix_search_filter branch to a2796b5 Jan 13, 2016

@martijnvg martijnvg merged commit a2796b5 into elastic:master Jan 13, 2016

1 check passed

CLA Commit author is a member of Elasticsearch
Details

@martijnvg martijnvg added v2.2.1 v2.1.3 v2.0.4 and removed review labels Jan 13, 2016

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.