From b2a86bb02e5f59d699bc8ef82d8d9993f2d34499 Mon Sep 17 00:00:00 2001 From: Matthew Bargar Date: Mon, 21 Nov 2016 18:18:20 -0500 Subject: [PATCH] Fix filters for complex painless scripted fields Painless scripted fields that consisted of more than a single expression would break when the user attempted to filter on them in Discover or Visualize because of the naive way we were building them. We now wrap the user's script in a lambda so that it can be as complex at they need it to be. The only special exception is that the user cannot define named functions since those cannot go inside a Painless lambda. Fixes https://github.com/elastic/kibana/issues/9024 Related https://github.com/elastic/elasticsearch/pull/21635 --- src/ui/public/field_editor/field_editor.html | 11 +--------- .../__tests__/filter_manager.js | 3 ++- .../public/filter_manager/filter_manager.js | 10 +++++++--- src/ui/public/filter_manager/lib/phrase.js | 12 +++++++---- src/ui/public/filter_manager/lib/range.js | 20 +++++++++++++++---- 5 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/ui/public/field_editor/field_editor.html b/src/ui/public/field_editor/field_editor.html index 7de9ded73ada5..11fb9144aa3c5 100644 --- a/src/ui/public/field_editor/field_editor.html +++ b/src/ui/public/field_editor/field_editor.html @@ -147,16 +147,7 @@

- Kibana currently imposes one special limitation on the scripts you write. Your scripts must consist of a single expression. Multi-statement scripts with an explicit return statement will not work. - In other words, the above example is a valid script, but something like the following is not. -

- -

- Invalid
- - def myField = doc['some_field'].value;
- return myField; - + Kibana currently imposes one special limitation on the painless scripts you write. They cannot contain named functions.

diff --git a/src/ui/public/filter_manager/__tests__/filter_manager.js b/src/ui/public/filter_manager/__tests__/filter_manager.js index a27b56ff6926c..9b13b1575350e 100644 --- a/src/ui/public/filter_manager/__tests__/filter_manager.js +++ b/src/ui/public/filter_manager/__tests__/filter_manager.js @@ -120,7 +120,8 @@ describe('Filter Manager', function () { meta: {index: 'myIndex', negate: false, field: 'scriptedField'}, script: { script: { - inline: '(' + scriptedField.script + ') == params.value', + inline: `boolean compare(Supplier s, def v) {return s.get() == v;} + compare(() -> { ${scriptedField.script} }, params.value);`, lang: scriptedField.lang, params: {value: 1} } diff --git a/src/ui/public/filter_manager/filter_manager.js b/src/ui/public/filter_manager/filter_manager.js index 6d8486f39dbcd..51b0ed0231e50 100644 --- a/src/ui/public/filter_manager/filter_manager.js +++ b/src/ui/public/filter_manager/filter_manager.js @@ -54,13 +54,17 @@ export default function (Private) { break; default: if (field.scripted) { - // painless expects params.value while groovy and expression languages expect value. - const valueClause = field.lang === 'painless' ? 'params.value' : 'value'; + // We must wrap painless scripts in a lambda in case they're more than a simple expression + let script = `(${field.script}) == value`; + if (field.lang === 'painless') { + script = `boolean compare(Supplier s, def v) {return s.get() == v;} + compare(() -> { ${field.script} }, params.value);`; + } filter = { meta: { negate: negate, index: index, field: fieldName }, script: { script: { - inline: '(' + field.script + ') == ' + valueClause, + inline: script, lang: field.lang, params: { value: value diff --git a/src/ui/public/filter_manager/lib/phrase.js b/src/ui/public/filter_manager/lib/phrase.js index b9e9cc92fa573..2a98a9bec0fba 100644 --- a/src/ui/public/filter_manager/lib/phrase.js +++ b/src/ui/public/filter_manager/lib/phrase.js @@ -3,9 +3,6 @@ export default function buildPhraseFilter(field, value, indexPattern) { let filter = { meta: { index: indexPattern.id} }; if (field.scripted) { - // painless expects params.value while groovy and expression languages expect value. - const valueClause = field.lang === 'painless' ? 'params.value' : 'value'; - // See https://github.com/elastic/elasticsearch/issues/20941 and https://github.com/elastic/kibana/issues/8677 // for the reason behind this change. ES doesn't handle boolean types very well, so they come // back as strings. @@ -17,8 +14,15 @@ export default function buildPhraseFilter(field, value, indexPattern) { convertedValue = value === 'true' ? true : false; } + // We must wrap painless scripts in a lambda in case they're more than a simple expression + let script = `(${field.script}) == value`; + if (field.lang === 'painless') { + script = `boolean compare(Supplier s, def v) {return s.get() == v;} + compare(() -> { ${field.script} }, params.value);`; + } + _.set(filter, 'script.script', { - inline: '(' + field.script + ') == ' + valueClause, + inline: script, lang: field.lang, params: { value: convertedValue diff --git a/src/ui/public/filter_manager/lib/range.js b/src/ui/public/filter_manager/lib/range.js index deec58b703199..c3c69145f7ac4 100644 --- a/src/ui/public/filter_manager/lib/range.js +++ b/src/ui/public/filter_manager/lib/range.js @@ -34,12 +34,24 @@ export default function buildRangeFilter(field, params, indexPattern, formattedV }; const knownParams = _.pick(params, (val, key) => { return key in operators; }); - const script = _.map(knownParams, function (val, key) { - // painless expects params.[key] while groovy and expression languages expect [key] only. - const valuePrefix = field.lang === 'painless' ? 'params.' : ''; - return '(' + field.script + ')' + operators[key] + valuePrefix + key; + let script = _.map(knownParams, function (val, key) { + return '(' + field.script + ')' + operators[key] + key; }).join(' && '); + // We must wrap painless scripts in a lambda in case they're more than a simple expression + if (field.lang === 'painless') { + const comparators = `boolean gt(Supplier s, def v) {return s.get() > v;} + boolean gte(Supplier s, def v) {return s.get() >= v;} + boolean lte(Supplier s, def v) {return s.get() <= v;} + boolean lt(Supplier s, def v) {return s.get() < v;}`; + + const comparisons = _.map(knownParams, function (val, key) { + return `${key}(() -> { ${field.script} }, params.${key})`; + }).join(' && '); + + script = `${comparators}${comparisons}`; + } + const value = _.map(knownParams, function (val, key) { return operators[key] + field.format.convert(val); }).join(' ');