Skip to content

Commit

Permalink
[Bug] Fix filter creation for numeric scripted fields in Discover (el…
Browse files Browse the repository at this point in the history
…astic#93224)

* fixes elastic#74301

* comment

* tests

* test title
  • Loading branch information
lizozom authored and Liza K committed Mar 3, 2021
1 parent 1106195 commit 9031393
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 5 deletions.
54 changes: 52 additions & 2 deletions src/plugins/data/common/es_query/filters/phrase_filter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,25 @@ describe('Phrase filter builder', () => {
expect(typeof buildPhraseFilter).toBe('function');
});

it('should return a match query filter when passed a standard field', () => {
it('should return a match query filter when passed a standard string field', () => {
const field = getField('extension');

expect(buildPhraseFilter(field, 'jpg', indexPattern)).toEqual({
meta: {
index: 'id',
},
query: {
match_phrase: {
extension: 'jpg',
},
},
});
});

it('should return a match query filter when passed a standard numeric field', () => {
const field = getField('bytes');

expect(buildPhraseFilter(field, 5, indexPattern)).toEqual({
expect(buildPhraseFilter(field, '5', indexPattern)).toEqual({
meta: {
index: 'id',
},
Expand All @@ -42,6 +57,21 @@ describe('Phrase filter builder', () => {
});
});

it('should return a match query filter when passed a standard bool field', () => {
const field = getField('ssl');

expect(buildPhraseFilter(field, 'true', indexPattern)).toEqual({
meta: {
index: 'id',
},
query: {
match_phrase: {
ssl: true,
},
},
});
});

it('should return a script filter when passed a scripted field', () => {
const field = getField('script number');

Expand All @@ -61,6 +91,26 @@ describe('Phrase filter builder', () => {
},
});
});

it('should return a script filter when passed a scripted field with numeric conversion', () => {
const field = getField('script number');

expect(buildPhraseFilter(field, '5', indexPattern)).toEqual({
meta: {
index: 'id',
field: 'script number',
},
script: {
script: {
lang: 'expression',
params: {
value: 5,
},
source: '(1234) == value',
},
},
});
});
});

describe('buildInlineScriptForPhraseFilter', () => {
Expand Down
15 changes: 12 additions & 3 deletions src/plugins/data/common/es_query/filters/phrase_filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,14 @@ export const getPhraseScript = (field: IFieldType, value: string) => {
};
};

// See https://github.com/elastic/elasticsearch/issues/20941 and https://github.com/elastic/kibana/issues/8677
// and https://github.com/elastic/elasticsearch/pull/22201
// for the reason behind this change. Aggs now return boolean buckets with a key of 1 or 0.
/**
* See issues bellow for the reason behind this change.
* Values need to be converted to correct types for boolean \ numeric fields.
* https://github.com/elastic/kibana/issues/74301
* https://github.com/elastic/kibana/issues/8677
* https://github.com/elastic/elasticsearch/issues/20941
* https://github.com/elastic/elasticsearch/pull/22201
**/
export const getConvertedValueForField = (field: IFieldType, value: any) => {
if (typeof value !== 'boolean' && field.type === 'boolean') {
if ([1, 'true'].includes(value)) {
Expand All @@ -109,6 +114,10 @@ export const getConvertedValueForField = (field: IFieldType, value: any) => {
throw new Error(`${value} is not a valid boolean value for boolean field ${field.name}`);
}
}

if (typeof value !== 'number' && field.type === 'number') {
return Number(value);
}
return value;
};

Expand Down

0 comments on commit 9031393

Please sign in to comment.