[Painless] Partially fixes def boxed types casting#35563
[Painless] Partially fixes def boxed types casting#35563jdconrad wants to merge 4 commits intoelastic:masterfrom
Conversation
jdconrad
added
>bug
:Core/Infra/Scripting
|
Pinging @elastic/es-core-infra |
rjernst
left a comment
rjernst
left a comment
There was a problem hiding this comment.
Looks fine, just one question.
| public static Byte defToByteExplicit(final Object value) { | ||
| if (value == null) { | ||
| return null; | ||
| } else if (value instanceof Character) { |
There was a problem hiding this comment.
I thought byte -> char and char -> byte had to be explicit (in java). Does painless differ here?
There was a problem hiding this comment.
Good catch, you are correct. This is a bug that I'll fix now.
There was a problem hiding this comment.
Note this is the explicit version, though. I corrected the implicit versions.
|
@rjernst Thanks for the review! Will commit as soon as CI passes. |
|
I'm closing this in favor of the quick/easier fix for now (#35653). The casting model in Painless needs to be revisited in a separate follow up as some of the specced out casts do not work as expected, but the fixes need to be given more thought so more problems are not created. |
4 participants
An issue related to implicitly casting to appropriate return types was revealed (#35351). According to the Painless spec (https://www.elastic.co/guide/en/elasticsearch/painless/current/painless-casting.html), def values are allowed to implicitly cast from one boxed type to another boxed type such as
IntegertoDouble. Currently, this not working according to spec. This partially fixes the issue by inserting appropriate methods at compile-time to execute the casts at run-time. The other part of the issue will be a different PR as it needs to fix an issue related to boxing for method parameters at run-time which is trickier due to run-time code path.