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

Replace FieldStatsProvider with a method on MappedFieldType. #17334

Merged

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Mar 24, 2016

FieldStatsProvider had to perform instanceof calls to properly handle dates or
ip addresses. By moving the logic to MappedFieldType, each field type can check
whether all values are within bounds its way.

Note that this commit only keeps rewriting support for dates, which are the only
field for which the rewriting mechanism is likely to help (because of time-based
indices).

randomBoolean(), randomBoolean(), null, null));
}

public void testIsFieldWithinQuery() throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a test to each of the other types to check that for random query bounds MappedFieldType.isFieldWithinQuery(IndexReader, Object, Object, boolean, boolean, TimeZone, DateMathParser) always returns Relation.INTERSECTS?

@colings86
Copy link
Contributor

@jpountz I left some comments on the tests but I like how much this change simplifies the logic

@jpountz
Copy link
Contributor Author

jpountz commented Mar 31, 2016

@colings86 I pushed more commits to improve the testing. Would you mind having another look?

@colings86
Copy link
Contributor

@jpountz Thanks for making the changes to the tests. LGTM

…17334

FieldStatsProvider had to perform instanceof calls to properly handle dates or
ip addresses. By moving the logic to MappedFieldType, each field type can check
whether all values are within bounds its way.

Note that this commit only keeps rewriting support for dates, which are the only
field for which the rewriting mechanism is likely to help (because of time-based
indices).
@jpountz jpountz force-pushed the enhancement/remove_fieldstats_provider branch from 08b6962 to 4c4bbb3 Compare April 1, 2016 08:29
@jpountz jpountz merged commit 4c4bbb3 into elastic:master Apr 1, 2016
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.

2 participants