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

Scripting: Convert script fields to use script context #34164

Merged
merged 13 commits into from
Oct 20, 2018

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Sep 28, 2018

This commit removes the use of SearchScript for script fields and adds
a new FieldScript.

This commit removes the use of SearchScript for script fields and adds
a new FieldScript.
@rjernst rjernst added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v7.0.0 >refactoring v6.5.0 labels Sep 28, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Copy link
Contributor

@jdconrad jdconrad left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for doing this!

@@ -14,18 +14,15 @@ a customized value for each document in the results of a query.
Contains the fields of the specified document where each field is a
`List` of values.

{ref}/mapping-source-field.html[`ctx['_source']`] (`Map`)::
{ref}/mapping-source-field.html[`params['_source']`] (`Map`)::
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mind adding read-only to this, like how _score was? This shouldn't be modified as part of a script field. My mistake from the first iteration of these docs.

Map<String, String> deprecations = new HashMap<>();
deprecations.put(
"doc",
"Accessing variable [doc] via [params.doc] from within a terms-set-query-script " +
Copy link
Contributor

Choose a reason for hiding this comment

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

Think this was a copy and paste issue -- should be field-script and not terms-set-query

@rjernst
Copy link
Member Author

rjernst commented Sep 29, 2018

@jdconrad I addressed your comments. I will wait for #33856 to get in so I can re-use the expression script engine work there to add expression support of the new field script.

@rjernst rjernst merged commit 222652d into elastic:master Oct 20, 2018
@rjernst rjernst deleted the script_field branch October 20, 2018 23:33
@rjernst rjernst removed the v6.5.0 label Oct 20, 2018
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Oct 21, 2018
* master:
  Use trial license in docs tests (elastic#34673)
  Scripting: Convert script fields to use script context (elastic#34164)
  TEST: Mute testDedupByPrimaryTerm
  ingest: processor stats (elastic#34202)
kcm pushed a commit that referenced this pull request Oct 30, 2018
This commit removes the use of SearchScript for script fields and adds
a new FieldScript.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >refactoring v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants