Skip to content

Commit

Permalink
Fix filters for complex painless scripted fields
Browse files Browse the repository at this point in the history
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 #9024
Related elastic/elasticsearch#21635
  • Loading branch information
Bargs committed Nov 21, 2016
1 parent 5ef7a52 commit b2a86bb
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 22 deletions.
11 changes: 1 addition & 10 deletions src/ui/public/field_editor/field_editor.html
Original file line number Diff line number Diff line change
Expand Up @@ -147,16 +147,7 @@ <h4>
</p>

<p>
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.
</p>

<p>
<strong>Invalid</strong><br/>
<code>
def myField = doc['some_field'].value;<br/>
return myField;
</code>
Kibana currently imposes one special limitation on the painless scripts you write. They cannot contain named functions.
</p>

<p>
Expand Down
3 changes: 2 additions & 1 deletion src/ui/public/filter_manager/__tests__/filter_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}
}
Expand Down
10 changes: 7 additions & 3 deletions src/ui/public/filter_manager/filter_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 8 additions & 4 deletions src/ui/public/filter_manager/lib/phrase.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
20 changes: 16 additions & 4 deletions src/ui/public/filter_manager/lib/range.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(' ');
Expand Down

0 comments on commit b2a86bb

Please sign in to comment.