-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Allow sorting and filtering on _type field #10254
Conversation
…r mappings don't include index:true
Tests are finally passing, should be good to review. |
const searchable = !!spec.searchable || scripted; | ||
const aggregatable = !!spec.aggregatable || scripted; | ||
const sortable = spec.name === '_score' || ((indexed || scripted || aggregatable) && type.sortable); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like aggregatable
implies scripted || aggregatable
, so scripted
could be left out here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
const searchable = !!spec.searchable || scripted; | ||
const aggregatable = !!spec.aggregatable || scripted; | ||
const sortable = spec.name === '_score' || ((indexed || scripted || aggregatable) && type.sortable); | ||
const filterable = spec.name === '_id' || scripted || ((indexed || searchable) && type.filterable); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably me not knowing enough about the various sortable
and filterable
attributes, but the asymmetry to the previous line has caught my eye. So a scripted
field's type has to be sortable
in order for the field to be sortable
, but its type does not have to be filterable
for the field to be filterable
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, you're reading that correctly. It's been that way for quite awhile though. I'm not sure if it's intentional or just a side effect of the code evolving over time. Given that filtering and sorting are two very different operations in ES, I wouldn't be totally surprised if there's some subtlety at play here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Backports PR #10254 **Commit 1:** Add searchable and aggregatable to the filterable sortable calculation * Original sha: e1d3dae * Authored by Matthew Bargar <[email protected]> on 2017-02-08T21:59:56Z **Commit 2:** Account for fields that field stats knows are searchable even if their mappings don't include index:true * Original sha: 4e39c31 * Authored by Matthew Bargar <[email protected]> on 2017-02-08T22:05:48Z **Commit 3:** Needed a new aggregatable but non-filterable example for this test * Original sha: 6362953 * Authored by Matthew Bargar <[email protected]> on 2017-02-09T21:58:11Z **Commit 4:** Needed a new non-sortable example field as well * Original sha: 0f70647 * Authored by Matthew Bargar <[email protected]> on 2017-02-09T23:21:02Z **Commit 5:** Simplify sortable computation * Original sha: 91dbc3e * Authored by Matthew Bargar <[email protected]> on 2017-02-10T15:44:50Z
Backports PR #10254 It was already possible to sort, aggregate, and filter on _type in ES but we didn't allow it in Kibana because Elasticsearch's APIs pre-5.0 didn't report mapping information for meta fields. Now that field stats returns searchable and aggregatable (including meta fields) we can safely determine if any field is filterable, sortable, or aggregatable without looking at the mappings. This PR simply includes the searchable and aggregatable information in the calculations that determine filterable and sortable status of a field.
Fixes #5684
It was already possible to sort, aggregate, and filter on
_type
in ES but we didn't allow it in Kibana because Elasticsearch's APIs pre-5.0 didn't report mapping information for meta fields. Now that field stats returnssearchable
andaggregatable
(including meta fields) we can safely determine if any field is filterable, sortable, or aggregatable without looking at the mappings. This PR simply includes thesearchable
andaggregatable
information in the calculations that determine filterable and sortable status of a field.