Update Painless Fields API design to use mapped types by jdconrad · Pull Request #78950 · elastic/elasticsearch

jdconrad · 2021-10-11T23:28:19Z

This change updates the Painless fields API to use mapped types instead of Java types. For this initial release we only support unsigned long field.

As we now intend to do conversions solely on the field classes themselves, converters and field values have been removed. Instead of using field values potentially as source fallback we will now rely on whatever class is passed into the script to support the field method. In the future, this method will need to check if a field exists on doc and if not, then do source fallback itself once that feature is supported.

The path to retrieve a field is also different. Instead of using script doc values we now have our own way to retrieve the fields with a getScriptField method added to LeafDocLookup. This allows us less restrictive design space to generate the field type based the type in the leaf doc lookup; however, this will still require additional information that we need to plumb through for certain field types. Once we support all the field types this makes it easier to deprecate and remove script doc values.

elasticmachine · 2021-10-11T23:28:22Z

Pinging @elastic/es-core-infra (Team:Core/Infra)

server/src/main/java/org/elasticsearch/script/field/Field.java

jdconrad · 2021-10-13T17:28:48Z

@elasticmachine run elasticsearch-ci/part-2

...g-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.fields.txt

modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/100_terms_agg.yml

server/src/main/java/org/elasticsearch/script/DocBasedScript.java

server/src/main/java/org/elasticsearch/script/DocValuesDocReader.java

server/src/main/java/org/elasticsearch/script/field/EmptyField.java

server/src/main/java/org/elasticsearch/search/lookup/LeafDocLookup.java

...pper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongField.java

...unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldTests.java

...ugin/mapper-unsigned-long/src/yamlRestTest/resources/rest-api-spec/test/50_script_values.yml

stu-elastic

We can reuse a lot of the removed tests when we implement basic types in the new design, so let's save those (as requested above).

...g-painless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.fields.txt

stu-elastic

Can you find a way to share the same doc values across ScriptDocValues and and DocValuesField? Having a penalty for using the fields API and doc in the same script is pretty rough for adoption and is surprising behavior.

stu-elastic

lgtm

jdconrad · 2021-10-13T22:49:44Z

@stu-elastic @rjernst Thanks for the in depth reviews! I'm still working toward addressing all the feedback, and will commit once that's done.

elasticsearchmachine · 2021-10-13T23:43:49Z

💔 Backport failed

Status	Branch	Result
❌	7.x	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 78950

This change updates the Painless fields API to use mapped types instead of Java types. For this initial release we only support unsigned long field. As we now intend to do conversions solely on the field classes themselves, converters and field values have been removed. Instead of using field values potentially as source fallback we will now rely on whatever class is passed into the script to support the field method. In the future, this method will need to check if a field exists on doc and if not, then do source fallback itself once that feature is supported. The path to retrieve a field is also different. Instead of using script doc values we now have our own way to retrieve the fields with a getScriptField method added to LeafDocLookup. This allows us less restrictive design space to generate the field type based the type in the leaf doc lookup; however, this will still require additional information that we need to plumb through for certain field types. Once we support all the field types this makes it easier to deprecate and remove script doc values.

…79201) This change updates the Painless fields API to use mapped types instead of Java types. For this initial release we only support unsigned long field. As we now intend to do conversions solely on the field classes themselves, converters and field values have been removed. Instead of using field values potentially as source fallback we will now rely on whatever class is passed into the script to support the field method. In the future, this method will need to check if a field exists on doc and if not, then do source fallback itself once that feature is supported. The path to retrieve a field is also different. Instead of using script doc values we now have our own way to retrieve the fields with a getScriptField method added to LeafDocLookup. This allows us less restrictive design space to generate the field type based the type in the leaf doc lookup; however, this will still require additional information that we need to plumb through for certain field types. Once we support all the field types this makes it easier to deprecate and remove script doc values.

jdconrad added 4 commits October 10, 2021 15:39

start to change painless fields api from java type to mapped type

b314186

ignore bwc tests

b52beeb

add allowed methods for unsigned long field

f647842

move and update yaml tests for new field types

f7c1104

jdconrad added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >refactoring v8.0.0 v7.16.0 labels Oct 11, 2021

jdconrad requested review from rjernst and stu-elastic October 11, 2021 23:28

elasticmachine added the Team:Core/Infra Meta label for core/infra team label Oct 11, 2021

jdconrad added the auto-backport Automatically create backport pull requests when merged label Oct 11, 2021

jdconrad added 2 commits October 13, 2021 09:11

Merge branch 'master' into fields0

f7349a8

add removed tests to restcompat ignore

a229562