Skip to content

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Mar 29, 2017

This is important for some queries like terms, which are parsed differently
depending on whether we want to get a query or a filter.

@javanna
Copy link
Member

javanna commented Mar 29, 2017

Can we have a test? :)

@jpountz
Copy link
Contributor Author

jpountz commented Mar 29, 2017

Yes sorry, it definitely needs one but I had not decided yet what would be best between stopping to parse queries and filters differently and keeping that functionnality. I was leaning towards the first option given that the terms filter now always parses like a filter but actually we still need that behaviour for bool queries.

@rahulanishetty
Copy link

@jpountz same thing needs to be done here as well SignificantTermsAggregatorFactory#filter?

@jpountz
Copy link
Contributor Author

jpountz commented Apr 4, 2017

@rahul1193 you are correct!

@jpountz jpountz force-pushed the fix/filter_agg_filter branch from c0b6e86 to fb44a67 Compare April 4, 2017 14:18
@jpountz jpountz changed the title The filter aggregation should parse the filter as a filter, not a query. The filter and significant_terms aggregations should parse the filter as a filter, not a query. Apr 4, 2017
@jpountz
Copy link
Contributor Author

jpountz commented Apr 4, 2017

OK, I fixed significant_terms too and added tests.

@jpountz jpountz requested a review from javanna April 4, 2017 14:20
Copy link
Member

@javanna javanna left a comment

Choose a reason for hiding this comment

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

LGTM

…ilter` as a filter, not as a query.

This is important for some queries like `bool`, which are parsed differently
depending on whether we want to get a query or a filter.
@jpountz jpountz force-pushed the fix/filter_agg_filter branch from fb44a67 to 37dab2f Compare April 5, 2017 14:45
@jpountz jpountz merged commit d5d0f14 into elastic:master Apr 5, 2017
@jpountz jpountz deleted the fix/filter_agg_filter branch April 5, 2017 14:46
rahulanishetty pushed a commit to rahulanishetty/elasticsearch that referenced this pull request Apr 13, 2017
…ilter` as a filter, not as a query. (elastic#23797)

This is important for some queries like `bool`, which are parsed differently
depending on whether we want to get a query or a filter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
close