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.

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

Add Fields API to aggregation scripts and field scripts #76325

Merged
merged 9 commits into from
Aug 11, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import org.elasticsearch.repositories.RepositoriesService;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestHandler;
import org.elasticsearch.script.AggregationScript;
import org.elasticsearch.script.FieldScript;
import org.elasticsearch.script.FilterScript;
import org.elasticsearch.script.IngestScript;
import org.elasticsearch.script.NumberSortScript;
Expand All @@ -46,6 +48,7 @@
import org.elasticsearch.script.ScriptEngine;
import org.elasticsearch.script.ScriptModule;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.script.ScriptedMetricAggContexts;
import org.elasticsearch.script.StringSortScript;
import org.elasticsearch.search.aggregations.pipeline.MovingFunctionScript;
import org.elasticsearch.threadpool.ThreadPool;
Expand Down Expand Up @@ -140,6 +143,24 @@ public final class PainlessPlugin extends Plugin implements ScriptPlugin, Extens
filter.add(filterWhitelist);
map.put(FilterScript.CONTEXT, filter);

List<Whitelist> aggregation = new ArrayList<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

At this point we should have something like getRuntimeFieldWhitelist for this API since a lot of this config code is the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I would prefer to do this as a follow up PR. I even wonder if this should maybe be moved elsewhere. The entirety of including these contexts as part of the module is a bit of an afterthought, though I understand doing them through SPI doesn't make sense here given the dependency hierarchy.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well we should cleanup the whitelisting code. Next PR is fine, but if we wanna do a more fundamental change then I'd prefer the wl code cleanup happen first.

Whitelist aggregationWhitelist =
WhitelistLoader.loadFromResourceFiles(PainlessPlugin.class, "org.elasticsearch.script.fields.aggregation.txt");
aggregation.add(aggregationWhitelist);
map.put(AggregationScript.CONTEXT, aggregation);

List<Whitelist> aggmap = new ArrayList<>();
Whitelist aggmapWhitelist =
WhitelistLoader.loadFromResourceFiles(PainlessPlugin.class, "org.elasticsearch.script.fields.aggmap.txt");
aggmap.add(aggmapWhitelist);
map.put(ScriptedMetricAggContexts.MapScript.CONTEXT, aggmap);

List<Whitelist> field = new ArrayList<>();
Whitelist fieldWhitelist =
WhitelistLoader.loadFromResourceFiles(PainlessPlugin.class, "org.elasticsearch.script.fields.field.txt");
field.add(fieldWhitelist);
map.put(FieldScript.CONTEXT, field);

// Execute context gets everything
List<Whitelist> test = new ArrayList<>();
test.add(movFnWhitelist);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#
# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
# or more contributor license agreements. Licensed under the Elastic License
# 2.0 and the Server Side Public License, v 1; you may not use this file except
# in compliance with, at your election, the Elastic License 2.0 or the Server
# Side Public License, v 1.
#

# The whitelist for the fields api

# The scripts must be whitelisted for painless to find the classes for the field API
class org.elasticsearch.script.ScriptedMetricAggContexts$MapScript @no_import {
}
class org.elasticsearch.script.ScriptedMetricAggContexts$MapScript$Factory @no_import {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#
# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
# or more contributor license agreements. Licensed under the Elastic License
# 2.0 and the Server Side Public License, v 1; you may not use this file except
# in compliance with, at your election, the Elastic License 2.0 or the Server
# Side Public License, v 1.
#

# The whitelist for the fields api

# The scripts must be whitelisted for painless to find the classes for the field API
class org.elasticsearch.script.AggregationScript @no_import {
}
class org.elasticsearch.script.AggregationScript$Factory @no_import {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#
# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
# or more contributor license agreements. Licensed under the Elastic License
# 2.0 and the Server Side Public License, v 1; you may not use this file except
# in compliance with, at your election, the Elastic License 2.0 or the Server
# Side Public License, v 1.
#

# The whitelist for the fields api

# The scripts must be whitelisted for painless to find the classes for the field API
class org.elasticsearch.script.FieldScript @no_import {
}
class org.elasticsearch.script.FieldScript @no_import {
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,24 @@ setup:
- is_false: aggregations.str_terms.buckets.1.key_as_string
- match: { aggregations.str_terms.buckets.1.doc_count: 1 }

---
"String Value Script with doc notation (fields api)":

- do:
search:
rest_total_hits_as_int: true
body: { "size" : 0, "aggs" : { "str_terms" : { "terms" : { "field" : "str", "script": { "source": "return field('str').getValue('') + \"1\""} } } } }

- match: { hits.total: 3 }

- length: { aggregations.str_terms.buckets: 2 }
- match: { aggregations.str_terms.buckets.0.key: "abc1" }
- is_false: aggregations.str_terms.buckets.0.key_as_string
- match: { aggregations.str_terms.buckets.0.doc_count: 2 }
- match: { aggregations.str_terms.buckets.1.key: "bcd1" }
- is_false: aggregations.str_terms.buckets.1.key_as_string
- match: { aggregations.str_terms.buckets.1.doc_count: 1 }

---
"Long Value Script with doc notation":

Expand All @@ -84,6 +102,24 @@ setup:
- is_false: aggregations.long_terms.buckets.1.key_as_string
- match: { aggregations.long_terms.buckets.1.doc_count: 1 }

---
"Long Value Script with doc notation (fields api)":

- do:
search:
rest_total_hits_as_int: true
body: { "size" : 0, "aggs" : { "long_terms" : { "terms" : { "field" : "number", "script": { "source": "return field('number').getValue(0L) + 1"} } } } }

- match: { hits.total: 3 }

- length: { aggregations.long_terms.buckets: 2 }
- match: { aggregations.long_terms.buckets.0.key: 2.0 }
- is_false: aggregations.long_terms.buckets.0.key_as_string
- match: { aggregations.long_terms.buckets.0.doc_count: 2 }
- match: { aggregations.long_terms.buckets.1.key: 3.0 }
- is_false: aggregations.long_terms.buckets.1.key_as_string
- match: { aggregations.long_terms.buckets.1.doc_count: 1 }

---
"Double Value Script with doc notation":

Expand All @@ -102,6 +138,24 @@ setup:
- is_false: aggregations.double_terms.buckets.1.key_as_string
- match: { aggregations.double_terms.buckets.1.doc_count: 1 }

---
"Double Value Script with doc notation (fields api)":

- do:
search:
rest_total_hits_as_int: true
body: { "size" : 0, "aggs" : { "double_terms" : { "terms" : { "field" : "double", "script": { "source": "return field('double').getValue(0.0) + 1"} } } } }

- match: { hits.total: 3 }

- length: { aggregations.double_terms.buckets: 2 }
- match: { aggregations.double_terms.buckets.0.key: 2.0 }
- is_false: aggregations.double_terms.buckets.0.key_as_string
- match: { aggregations.double_terms.buckets.0.doc_count: 2 }
- match: { aggregations.double_terms.buckets.1.key: 3.0 }
- is_false: aggregations.double_terms.buckets.1.key_as_string
- match: { aggregations.double_terms.buckets.1.doc_count: 1 }

---
"Bucket script with keys":

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
setup:
- do:
indices.create:
index: test
body:
settings:
number_of_replicas: 0
mappings:
properties:
double:
type: double

- do:
cluster.health:
wait_for_status: green

- do:
index:
index: test
id: 1
body:
double: 1.0

- do:
index:
index: test
id: 2
body:
double: 1.0

- do:
index:
index: test
id: 3
body:
double: 2.0

- do:
indices.refresh: {}

---
"Scripted Metric Agg Total":

- do:
search:
rest_total_hits_as_int: true
body: {
"size": 0,
"aggs": {
"total": {
"scripted_metric": {
"init_script": "state.transactions = []",
"map_script": "state.transactions.add(doc['double'].value)",
"combine_script": "double total = 0.0; for (t in state.transactions) { total += t } return total",
"reduce_script": "double total = 0; for (a in states) { total += a } return total"
}
}
}
}

- match: { hits.total: 3 }
- match: { aggregations.total.value: 4.0 }

---
"Scripted Metric Agg Total (fields api)":

- do:
search:
rest_total_hits_as_int: true
body: {
"size": 0,
"aggs": {
"total": {
"scripted_metric": {
"init_script": "state.transactions = []",
"map_script": "state.transactions.add(field('double').getValue(0.0))",
"combine_script": "double total = 0.0; for (t in state.transactions) { total += t } return total",
"reduce_script": "double total = 0; for (a in states) { total += a } return total"
}
}
}
}

- match: { hits.total: 3 }
- match: { aggregations.total.value: 4.0 }
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,21 @@ setup:

- match: { hits.hits.0.fields.bar.0: "aaabbb"}

---
"Scripted Field (fields api)":
- do:
search:
rest_total_hits_as_int: true
body:
script_fields:
bar:
script:
source: "field('foo').getValue('') + params.x;"
params:
x: "bbb"

- match: { hits.hits.0.fields.bar.0: "aaabbb"}

---
"Scripted Field Doing Compare":
- do:
Expand Down Expand Up @@ -73,6 +88,35 @@ setup:

- match: { hits.hits.0.fields.bar.0: false}

---
"Scripted Field Doing Compare (fields api)":
- do:
search:
rest_total_hits_as_int: true
body:
script_fields:
bar:
script:
source: "boolean compare(Supplier s, def v) {return s.get() == v;}
compare(() -> { return field('foo').getValue('') }, params.x);"
params:
x: "aaa"

- match: { hits.hits.0.fields.bar.0: true}
- do:
search:
rest_total_hits_as_int: true
body:
script_fields:
bar:
script:
source: "boolean compare(Supplier s, def v) {return s.get() == v;}
compare(() -> { return doc['foo'].value }, params.x);"
params:
x: "bbb"

- match: { hits.hits.0.fields.bar.0: false}

---
"Scripted Field with a null safe dereference (non-null)":
- do:
Expand All @@ -88,6 +132,21 @@ setup:

- match: { hits.hits.0.fields.bar.0: 8}

---
"Scripted Field with a null safe dereference (non-null, fields api)":
- do:
search:
rest_total_hits_as_int: true
body:
script_fields:
bar:
script:
source: "(field('foo').getValue('')?.length() ?: 0) + params.x;"
jdconrad marked this conversation as resolved.
Show resolved Hide resolved
params:
x: 5

- match: { hits.hits.0.fields.bar.0: 8}

---
"Scripted Field with a null safe dereference (null)":
- do:
Expand Down Expand Up @@ -116,6 +175,19 @@ setup:

- match: { hits.hits.0.fields.bar.0: 7}

---
"Access a date (fields api)":
- do:
search:
rest_total_hits_as_int: true
body:
script_fields:
bar:
script:
source: "field('date').getValue(0L).dayOfWeekEnum.value"
jdconrad marked this conversation as resolved.
Show resolved Hide resolved

- match: { hits.hits.0.fields.bar.0: 7}

---
"Access many dates":
- do:
Expand All @@ -134,6 +206,24 @@ setup:

- match: { hits.hits.0.fields.bar.0: "7 3 3"}

---
"Access many dates (fields api)":
- do:
search:
rest_total_hits_as_int: true
body:
script_fields:
bar:
script:
source: >
StringBuilder b = new StringBuilder();
for (def date : field('dates').getValues()) {
b.append(" ").append(date.getDayOfWeekEnum().value);
}
return b.toString().trim()

- match: { hits.hits.0.fields.bar.0: "7 3 3"}

---
"Scripted Field with script error":
- do:
Expand Down
Loading