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

Deprecate aggregation ValueScripts in favor of Runtime Fields #69291

Open
not-napoleon opened this issue Feb 19, 2021 · 12 comments
Open

Deprecate aggregation ValueScripts in favor of Runtime Fields #69291

not-napoleon opened this issue Feb 19, 2021 · 12 comments
Labels
:Analytics/Aggregations Aggregations >deprecation :Search Foundations/Search Catch all for Search Foundations Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch

Comments

@not-napoleon
Copy link
Member

Value scripts provide a tool for modifying field values at query time. They can be used for things like applying a correction or normalization to values, or changing the types of a field (e.g. parsing a number out of a string field). They also add a lot of complexity to the type system aggregations use (ValuesSource), and their behavior with missing values is not obvious (e.g. if you specify a value script and a missing value, is the script applied to the missing value?). There are also some tricky issues around value scripts (e.g. #53135)

Enter Runtime Fields. By and large, runtime fields solve the same problem - dynamically modifying a field value at query time. Runtime fields are well integrated into the mapping framework and have much better test coverage than value scripts. They're more centrally documented and present a clear, global abstraction as opposed to an implementation detail of an aggregation.

This issue is to discuss if, how, and when we should drop support for value scripts in favor of runtime fields.

Note: this is only referring to value scripts (e.g. https://www.elastic.co/guide/en/elasticsearch/reference/7.11/search-aggregations-metrics-min-aggregation.html#_value_script_6) not "regular" scripts. Value scripts are intended to operate on a single field, while the more general case operates on a whole document.

@not-napoleon not-napoleon added :Analytics/Aggregations Aggregations >deprecation team-discuss Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) labels Feb 19, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

@javanna
Copy link
Member

javanna commented Mar 3, 2021

Thanks @not-napoleon I can think of a couple more places where scripts could be replaced in the search API in favour of runtime fields. Maybe we should discuss these altogether and come up with a deprecation plan for them all.

@jimczi jimczi added the :Search/Search Search-related issues that do not fall into other categories label Mar 5, 2021
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Mar 5, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

@nik9000
Copy link
Member

nik9000 commented Apr 1, 2021

Here are a few places where we document script parameters that aren't runtime fields. grep ain't great for this, but its what I have at hand.

$ find docs/reference/ -type f | grep -v ingest | grep -v reindex | grep -v /update | grep -v /bulk | grep -v predicate-tokenfilter | grep -v /runtime | xargs grep -n \"script\" | sort | sed 's/^/* [ ] /'

Anyway, I'm going to rewrite a few of these using runtime fields.

nik9000 added a commit to nik9000/elasticsearch that referenced this issue Apr 1, 2021
This replaces the `script` docs for bucket aggregations with runtime
fields. We expect runtime fields to be nicer to work with because you
can also fetch them or filter on them. We expect them to be faster
because their don't need this sort of `instanceof` tree:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/server/src/main/java/org/elasticsearch/search/aggregations/support/values/ScriptDoubleValues.java#L42

Relates to elastic#69291
@javanna javanna self-assigned this Apr 2, 2021
@javanna
Copy link
Member

javanna commented Apr 2, 2021

Thanks Nik! I removed the team-discuss label, as we discussed this with the search team, and decided that we should make this issue broader and list all of the different script contexts that we support in the search API and decide which ones can be replaced by runtime fields. I will work on this.

@nik9000
Copy link
Member

nik9000 commented Apr 2, 2021

Sounds good! Mind if I do the aggs ones? I'm sort of in search of something like this.

@javanna
Copy link
Member

javanna commented Apr 2, 2021

please go ahead ;)

nik9000 added a commit that referenced this issue Apr 2, 2021
This replaces the `script` docs for bucket aggregations with runtime
fields. We expect runtime fields to be nicer to work with because you
can also fetch them or filter on them. We expect them to be faster
because their don't need this sort of `instanceof` tree:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/server/src/main/java/org/elasticsearch/search/aggregations/support/values/ScriptDoubleValues.java#L42

Relates to #69291

Co-authored-by: Adam Locke <[email protected]>
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Apr 2, 2021
This replaces the `script` docs for bucket aggregations with runtime
fields. We expect runtime fields to be nicer to work with because you
can also fetch them or filter on them. We expect them to be faster
because their don't need this sort of `instanceof` tree:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/server/src/main/java/org/elasticsearch/search/aggregations/support/values/ScriptDoubleValues.java#L42

Relates to elastic#69291

Co-authored-by: Adam Locke <[email protected]>
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Apr 2, 2021
This replaces the `script` docs for bucket aggregations with runtime
fields. We expect runtime fields to be nicer to work with because you
can also fetch them or filter on them. We expect them to be faster
because their don't need this sort of `instanceof` tree:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/server/src/main/java/org/elasticsearch/search/aggregations/support/values/ScriptDoubleValues.java#L42

Relates to elastic#69291

Co-authored-by: Adam Locke <[email protected]>
nik9000 added a commit that referenced this issue Apr 2, 2021
This replaces the `script` docs for bucket aggregations with runtime
fields. We expect runtime fields to be nicer to work with because you
can also fetch them or filter on them. We expect them to be faster
because their don't need this sort of `instanceof` tree:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/server/src/main/java/org/elasticsearch/search/aggregations/support/values/ScriptDoubleValues.java#L42

Relates to #69291

Co-authored-by: Adam Locke <[email protected]>
nik9000 added a commit that referenced this issue Apr 2, 2021
This replaces the `script` docs for bucket aggregations with runtime
fields. We expect runtime fields to be nicer to work with because you
can also fetch them or filter on them. We expect them to be faster
because their don't need this sort of `instanceof` tree:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/server/src/main/java/org/elasticsearch/search/aggregations/support/values/ScriptDoubleValues.java#L42

Relates to #69291

Co-authored-by: Adam Locke <[email protected]>
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Apr 2, 2021
This replaces the `script` docs for bucket aggregations with runtime
fields. We expect runtime fields to be nicer to work with because you
can also fetch them or filter on them. We expect them to be faster
because their don't need this sort of `instanceof` tree:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/server/src/main/java/org/elasticsearch/search/aggregations/support/values/ScriptDoubleValues.java#L42

Relates to elastic#69291
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Apr 5, 2021
This adds a "note" on the docs for the script query pointing folks to
runtime fields because they are more flexible. It also translates the
request example into runtime fields.

Relates to elastic#69291
nik9000 added a commit that referenced this issue Apr 5, 2021
This replaces the `script` docs for bucket aggregations with runtime
fields. We expect runtime fields to be nicer to work with because you
can also fetch them or filter on them. We expect them to be faster
because their don't need this sort of `instanceof` tree:
https://github.com/elastic/elasticsearch/blob/a92a647b9f17d1bddf5c707490a19482c273eda3/server/src/main/java/org/elasticsearch/search/aggregations/support/values/ScriptDoubleValues.java#L42

Relates to #69291

Co-authored-by: James Rodewig <[email protected]>
Co-authored-by: Adam Locke <[email protected]>
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Apr 12, 2021
…ic#71351)

Runtime fields are much more flexible than script_fields because you
can filter and aggregate on them so we hope folks use them! This
converts the example of using a `date_nanos` field in a script to a
runtime field so folks get used to seeing them and hopefully using them.

While I was editing this I took the opportunity to replace the script
with a real-ish example. Scripts that just load the field value are nice
and short but I hope no one uses them in real life because they just add
overhead when compared to accessing the field directly. So I made the
script do something.

Relates to elastic#69291

Co-authored-by: Adam Locke <[email protected]>
nik9000 added a commit that referenced this issue Apr 12, 2021
… (#71594)

Runtime fields are much more flexible than script_fields because you
can filter and aggregate on them so we hope folks use them! This
converts the example of using a `date_nanos` field in a script to a
runtime field so folks get used to seeing them and hopefully using them.

While I was editing this I took the opportunity to replace the script
with a real-ish example. Scripts that just load the field value are nice
and short but I hope no one uses them in real life because they just add
overhead when compared to accessing the field directly. So I made the
script do something.

Relates to #69291

Co-authored-by: Adam Locke <[email protected]>
nik9000 added a commit that referenced this issue Apr 12, 2021
… (#71595)

Runtime fields are much more flexible than script_fields because you
can filter and aggregate on them so we hope folks use them! This
converts the example of using a `date_nanos` field in a script to a
runtime field so folks get used to seeing them and hopefully using them.

While I was editing this I took the opportunity to replace the script
with a real-ish example. Scripts that just load the field value are nice
and short but I hope no one uses them in real life because they just add
overhead when compared to accessing the field directly. So I made the
script do something.

Relates to #69291

Co-authored-by: Adam Locke <[email protected]>
nik9000 added a commit that referenced this issue Apr 12, 2021
This shrinks a runtime field definition so that it fits on the screen
without scrolling. It also converts the doc into a test so we can be
sure it continues to work.

Relates to #69291
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Apr 12, 2021
This shrinks a runtime field definition so that it fits on the screen
without scrolling. It also converts the doc into a test so we can be
sure it continues to work.

Relates to elastic#69291
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Apr 12, 2021
This shrinks a runtime field definition so that it fits on the screen
without scrolling. It also converts the doc into a test so we can be
sure it continues to work.

Relates to elastic#69291
nik9000 added a commit that referenced this issue Apr 13, 2021
This shrinks a runtime field definition so that it fits on the screen
without scrolling. It also converts the doc into a test so we can be
sure it continues to work.

Relates to #69291
nik9000 added a commit that referenced this issue Apr 13, 2021
This shrinks a runtime field definition so that it fits on the screen
without scrolling. It also converts the doc into a test so we can be
sure it continues to work.

Relates to #69291
nik9000 added a commit that referenced this issue Apr 13, 2021
Runtime fields are much more flexible than script_fields because you
can filter and aggregate on them so we hope folks use them! This
converts the example of using a `parent_join` field in a script to a
runtime field so folks get used to seeing them and hopefully using them.

While I was editing this I took the opportunity to replace the script
with a real-ish example. Scripts that just load the field value are nice
and short but I hope no one uses them in real life because they just add
overhead when compared to accessing the field directly. So I made the
script do something.

Relates to #69291
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Apr 13, 2021
Runtime fields are much more flexible than script_fields because you
can filter and aggregate on them so we hope folks use them! This
converts the example of using a `parent_join` field in a script to a
runtime field so folks get used to seeing them and hopefully using them.

While I was editing this I took the opportunity to replace the script
with a real-ish example. Scripts that just load the field value are nice
and short but I hope no one uses them in real life because they just add
overhead when compared to accessing the field directly. So I made the
script do something.

Relates to elastic#69291
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Apr 13, 2021
Runtime fields are much more flexible than script_fields because you
can filter and aggregate on them so we hope folks use them! This
converts the example of using a `parent_join` field in a script to a
runtime field so folks get used to seeing them and hopefully using them.

While I was editing this I took the opportunity to replace the script
with a real-ish example. Scripts that just load the field value are nice
and short but I hope no one uses them in real life because they just add
overhead when compared to accessing the field directly. So I made the
script do something.

Relates to elastic#69291
nik9000 added a commit that referenced this issue Apr 13, 2021
…) (#71631)

Runtime fields are much more flexible than script_fields because you
can filter and aggregate on them so we hope folks use them! This
converts the example of using a `parent_join` field in a script to a
runtime field so folks get used to seeing them and hopefully using them.

While I was editing this I took the opportunity to replace the script
with a real-ish example. Scripts that just load the field value are nice
and short but I hope no one uses them in real life because they just add
overhead when compared to accessing the field directly. So I made the
script do something.

Relates to #69291
nik9000 added a commit that referenced this issue Apr 13, 2021
…) (#71632)

Runtime fields are much more flexible than script_fields because you
can filter and aggregate on them so we hope folks use them! This
converts the example of using a `parent_join` field in a script to a
runtime field so folks get used to seeing them and hopefully using them.

While I was editing this I took the opportunity to replace the script
with a real-ish example. Scripts that just load the field value are nice
and short but I hope no one uses them in real life because they just add
overhead when compared to accessing the field directly. So I made the
script do something.

Relates to #69291
@javanna
Copy link
Member

javanna commented Apr 28, 2021

The deprecation of the different flavor of scripts in the search API in favor of runtime fields is under discussion, but it's important to note that Kibana scripted fields rely on them in different places, and are not going to be removed in 8.0, hence we are not ready to deprecate in Elasticsearch. We are working on mentioning runtime fields in as many places as possible in the docs (thanks @nik9000 ) to make sure that users start using runtime fields which are more powerful. On the long run, we do expect runtime fields to replace a lot of script usages in search, but we are not quite ready to do that just now.

@a03nikki
Copy link
Contributor

a03nikki commented Jul 9, 2021

I don't know if this page needs to be updated as well or not, but figured I would mention it as the other doc updates relating to using runtime fields over the (old) scripted fields were done under this ticket.

https://www.elastic.co/guide/en/elasticsearch/reference/current/transform-painless-examples.html

(1) Painless examples for transforms
...
(1.2) Getting time features by using aggregations
...
(1.4) Counting HTTP responses by using scripted metric aggregation
...

(I added the numbering to the section headers to make it a little easier to follow compared to the 7.13 deployed version of the page.)

The example from (1.2) "Getting time features by using aggregations" uses a script in a average aggregation. This lead me to https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-metrics-avg-aggregation.html#_script_2 which uses runtime fields instead of the (older) "script" option on the aggregation. It doesn't mention at all the older (likely deprecated) aggs.{name}.{agg}.script field. The one for the terms agg is also gone as well.

There is also no mention on the runtime field limitations page that they cannot be used with data transforms. Only there is a performance hit for runtime fields for the user-defined time field.

I did not test it, but I would think the two sections (1.2 and 1.4) could use runtime fields instead of an aggregation script.

@javanna javanna removed their assignment Feb 28, 2022
@javanna javanna added :Search Foundations/Search Catch all for Search Foundations and removed :Search/Search Search-related issues that do not fall into other categories labels Jul 17, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@elasticsearchmachine elasticsearchmachine added Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch and removed Team:Search Meta label for search team labels Jul 17, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-foundations (Team:Search Foundations)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Aggregations Aggregations >deprecation :Search Foundations/Search Catch all for Search Foundations Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch
Projects
None yet
Development

No branches or pull requests

7 participants