Skip to content

Conversation

@jdconrad
Copy link
Contributor

@jdconrad jdconrad commented Jan 6, 2022

This adds the mapped type half float the scripting fields API. This also adds additional methods for asDouble to reach parity with old-style doc access.

@jdconrad jdconrad added >enhancement :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v8.1.0 labels Jan 6, 2022
@jdconrad jdconrad requested a review from stu-elastic January 6, 2022 01:01
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Jan 6, 2022
@elasticmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

Hi @jdconrad, I've created a changelog YAML for you.

}

class org.elasticsearch.script.field.HalfFloatDocValuesField @dynamic_type {
float get(double)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a note explaining why this takes a double but returns a float.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

Copy link
Contributor

@stu-elastic stu-elastic left a comment

Choose a reason for hiding this comment

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

Very nice.

@jdconrad
Copy link
Contributor Author

jdconrad commented Jan 6, 2022

@stu-elastic Thanks for the review. Will commit as soon as CI passes.

@jdconrad
Copy link
Contributor Author

Due to the casting between floats and doubles, I updated the test to use close_to instead of match.

@jdconrad jdconrad merged commit 4f37df1 into elastic:master Jan 10, 2022
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 >enhancement Team:Core/Infra Meta label for core/infra team v8.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants