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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Feb 13, 2017
Merged

Allow sorting and filtering on _type field #10254

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e1d3dae
Add searchable and aggregatable to the filterable sortable calculation
Bargs Feb 8, 2017
4e39c31
Account for fields that field stats knows are searchable even if thei…
Bargs Feb 8, 2017
6362953
Needed a new aggregatable but non-filterable example for this test
Bargs Feb 9, 2017
0f70647
Needed a new non-sortable example field as well
Bargs Feb 9, 2017
91dbc3e
Simplify sortable computation
Bargs Feb 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/core_plugins/kibana/public/discover/components/field_chooser/discover_field.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ app.directive('discoverField', function ($compile) {
' Values such as foo-bar will be broken into foo and bar.');
}

if (!field.indexed) {
if (!field.indexed && !field.searchable) {
warnings.push('This field is not indexed and might not be usable in visualizations.');
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/fixtures/logstash_fields.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ function stubbedLogstashFields() {
['_id', 'string', false, false, true, true ],
['_type', 'string', false, false, true, true ],
['_source', 'string', false, false, true, true ],
['non-filterable', 'string', false, false, true, false],
['non-sortable', 'string', false, false, false, false],
['custom_user_field', 'conflict', false, false, true, true ],
['script string', 'string', false, false, true, false, { script: '\'i am a string\'' } ],
['script number', 'number', false, false, true, false, { script: '1234' } ],
Expand Down
4 changes: 2 additions & 2 deletions src/ui/public/doc_table/__tests__/lib/get_sort.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ describe('docTable', function () {
});

it('should sort by the default when passed an unsortable field', function () {
expect(getSort(['_id', 'asc'], indexPattern)).to.eql(defaultSort);
expect(getSort(['non-sortable', 'asc'], indexPattern)).to.eql(defaultSort);
expect(getSort(['lol_nope', 'asc'], indexPattern)).to.eql(defaultSort);

delete indexPattern.timeFieldName;
expect(getSort(['_id', 'asc'], indexPattern)).to.eql({ _score: 'desc' });
expect(getSort(['non-sortable', 'asc'], indexPattern)).to.eql({ _score: 'desc' });
});

it('should sort in reverse chrono order otherwise on time based patterns', function () {
Expand Down
2 changes: 1 addition & 1 deletion src/ui/public/filter_bar/__tests__/filter_bar_click_handler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('filterBarClickHandler', function () {
{
type: 'terms',
schema: 'segment',
params: { field: '_type' }
params: { field: 'non-filterable' }
}
]
});
Expand Down
4 changes: 2 additions & 2 deletions src/ui/public/index_patterns/_field.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ export default function FieldObjectProvider(Private, shortDotsFilter, $rootScope

const indexed = !!spec.indexed;
const scripted = !!spec.scripted;
const sortable = spec.name === '_score' || ((indexed || scripted) && type.sortable);
const filterable = spec.name === '_id' || scripted || (indexed && type.filterable);
const searchable = !!spec.searchable || scripted;
const aggregatable = !!spec.aggregatable || scripted;
const sortable = spec.name === '_score' || ((indexed || scripted || aggregatable) && type.sortable);
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

const filterable = spec.name === '_id' || scripted || ((indexed || searchable) && type.filterable);
Copy link
Member

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?

Copy link
Contributor Author

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.

const visualizable = aggregatable;

obj.fact('name');
Expand Down