Skip to content

Commit

Permalink
Remove highlight query (#13231)
Browse files Browse the repository at this point in the history
* Change use of all_fields in highlight_query to default_field

* Remove highlight query and option

* Fix tests

* Remove unused setting

* Remove lingering references to all_fields
  • Loading branch information
lukasolson committed Aug 7, 2017
1 parent 84a34e3 commit 9bdba7c
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 96 deletions.
2 changes: 0 additions & 2 deletions docs/management/advanced-options.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ document.
`discover:aggs:terms:size`:: Determines how many terms will be visualized when clicking the "visualize" button, in the field drop downs, in the discover sidebar. The default value is `20`.
`doc_table:highlight`:: Highlight results in Discover and Saved Searches Dashboard. Highlighting makes request slow when
working on big documents. Set this property to `false` to disable highlighting.
`doc_table:highlight:all_fields`:: Improves highlighting by using a separate `highlight_query` that uses `all_fields` mode on
`query_string` queries. Set to `false` if you are using a `default_field` in your index.
`courier:maxSegmentCount`:: Kibana splits requests in the Discover app into segments to limit the size of requests sent to
the Elasticsearch cluster. This setting constrains the length of the segment list. Long segment lists can significantly
increase request processing time.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,53 +5,19 @@ describe('getHighlightRequest', () => {
let configMock;
const getConfig = (key) => configMock[key];
const queryStringQuery = { query_string: { query: 'foo' } };
const queryStringWithDefaultFieldQuery = { query_string: { query: 'foo', default_field: 'bar' } };
const queryStringWithFieldQuery = { query_string: { query: 'foo', fields: ['bar'] } };
const rangeQuery = { range: { '@timestamp': { gte: 0, lte: 0 } } };
const boolQueryWithSingleCondition = {
bool: {
must: queryStringQuery,
}
};
const boolQueryWithMultipleConditions = {
bool: {
must: [
queryStringQuery,
rangeQuery
]
}
};

beforeEach(function () {
configMock = {};
configMock['doc_table:highlight'] = true;
configMock['doc_table:highlight:all_fields'] = true;
});

it('should be a function', () => {
expect(getHighlightRequest).to.be.a(Function);
});

it('should add the all_fields param with query_string query without modifying original query', () => {
const request = getHighlightRequest(queryStringQuery, getConfig);
expect(request.fields['*']).to.have.property('highlight_query');
expect(request.fields['*'].highlight_query.query_string).to.have.property('all_fields');
expect(queryStringQuery.query_string).to.not.have.property('all_fields');
});

it('should add the all_fields param with bool query with single condition without modifying original query', () => {
const request = getHighlightRequest(boolQueryWithSingleCondition, getConfig);
expect(request.fields['*']).to.have.property('highlight_query');
expect(request.fields['*'].highlight_query.bool.must.query_string).to.have.property('all_fields');
expect(queryStringQuery.query_string).to.not.have.property('all_fields');
});

it('should add the all_fields param with bool query with multiple conditions without modifying original query', () => {
const request = getHighlightRequest(boolQueryWithMultipleConditions, getConfig);
expect(request.fields['*']).to.have.property('highlight_query');
expect(request.fields['*'].highlight_query.bool.must).to.have.length(boolQueryWithMultipleConditions.bool.must.length);
expect(request.fields['*'].highlight_query.bool.must[0].query_string).to.have.property('all_fields');
expect(queryStringQuery.query_string).to.not.have.property('all_fields');
it('should not modify the original query', () => {
getHighlightRequest(queryStringQuery, getConfig);
expect(queryStringQuery.query_string).to.not.have.property('highlight');
});

it('should return undefined if highlighting is turned off', () => {
Expand All @@ -60,12 +26,6 @@ describe('getHighlightRequest', () => {
expect(request).to.be(undefined);
});

it('should not add the highlight_query param if all_fields is turned off', () => {
configMock['doc_table:highlight:all_fields'] = false;
const request = getHighlightRequest(queryStringQuery, getConfig);
expect(request.fields['*']).to.not.have.property('highlight_query');
});

it('should enable/disable highlighting if config is changed', () => {
let request = getHighlightRequest(queryStringQuery, getConfig);
expect(request).to.not.be(undefined);
Expand All @@ -74,20 +34,4 @@ describe('getHighlightRequest', () => {
request = getHighlightRequest(queryStringQuery, getConfig);
expect(request).to.be(undefined);
});

it('should not add the all_fields param with query_string query when default_field is specified', () => {
const request = getHighlightRequest(queryStringWithDefaultFieldQuery, getConfig);
expect(request.fields['*']).to.have.property('highlight_query');
expect(request.fields['*'].highlight_query.query_string).to.have.property('default_field');
expect(request.fields['*'].highlight_query.query_string).to.not.have.property('all_fields');
expect(queryStringQuery.query_string).to.not.have.property('all_fields');
});

it('should not add the all_fields param with query_string query when fields are specified', () => {
const request = getHighlightRequest(queryStringWithFieldQuery, getConfig);
expect(request.fields['*']).to.have.property('highlight_query');
expect(request.fields['*'].highlight_query.query_string).to.have.property('fields');
expect(request.fields['*'].highlight_query.query_string).to.not.have.property('all_fields');
expect(queryStringQuery.query_string).to.not.have.property('all_fields');
});
});
30 changes: 1 addition & 29 deletions src/core_plugins/kibana/common/highlight/highlight_request.js
Original file line number Diff line number Diff line change
@@ -1,43 +1,15 @@
import _ from 'lodash';
import { highlightTags } from './highlight_tags';

const FRAGMENT_SIZE = Math.pow(2, 31) - 1; // Max allowed value for fragment_size (limit of a java int)

/**
* Returns a clone of the query with `"all_fields": true` set on any `query_string` queries
*/
function getHighlightQuery(query) {
const clone = _.cloneDeep(query);

if (
_.has(clone, 'query_string')
&& !_.has(clone, ['query_string', 'default_field'])
&& !_.has(clone, ['query_string', 'fields'])
) {
clone.query_string.all_fields = true;
} else if (_.has(clone, 'bool.must')) {
if (Array.isArray(clone.bool.must)) {
clone.bool.must = clone.bool.must.map(getHighlightQuery);
} else {
clone.bool.must = getHighlightQuery(clone.bool.must);
}
}

return clone;
}

export function getHighlightRequest(query, getConfig) {
if (!getConfig('doc_table:highlight')) return;

const fieldsParams = getConfig('doc_table:highlight:all_fields')
? { highlight_query: getHighlightQuery(query) }
: {};

return {
pre_tags: [highlightTags.pre],
post_tags: [highlightTags.post],
fields: {
'*': fieldsParams
'*': {}
},
fragment_size: FRAGMENT_SIZE
};
Expand Down
7 changes: 1 addition & 6 deletions src/core_plugins/kibana/ui_setting_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export function getUiSettingDefaults() {
readonly: true
},
'query:queryString:options': {
value: '{ "analyze_wildcard": true }',
value: '{ "analyze_wildcard": true, "default_field": "*" }',
description: '<a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-query-string-query.html" target="_blank">Options</a> for the lucene query string parser',
type: 'json'
},
Expand Down Expand Up @@ -91,11 +91,6 @@ export function getUiSettingDefaults() {
description: 'Highlight results in Discover and Saved Searches Dashboard.' +
'Highlighting makes requests slow when working on big documents.',
},
'doc_table:highlight:all_fields': {
value: true,
description: 'Improves highlighting by using a separate "highlight_query" that uses "all_fields" mode on "query_string" queries. ' +
'Set to false if you are using a "default_field" in your index.',
},
'courier:maxSegmentCount': {
value: 30,
description: 'Requests in discover are split into segments to prevent massive requests from being sent to ' +
Expand Down

0 comments on commit 9bdba7c

Please sign in to comment.