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

Painless still cannot understand some overloaded methods #22177

Closed
umnick84 opened this issue Dec 14, 2016 · 7 comments
Closed

Painless still cannot understand some overloaded methods #22177

umnick84 opened this issue Dec 14, 2016 · 7 comments
Assignees
Labels
>bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache

Comments

@umnick84
Copy link

umnick84 commented Dec 14, 2016

Elasticsearch version: 5.0.1 and 5.1.1

Plugins installed: []

JVM version: 1.8.102

OS version: MacOS

Description of the problem including expected versus actual behavior:
When I write a script with code "BitSet.valueOf(some_byte_array)" I get ClassCastException with message byte[] cannot be cast to long[]. I think this happens because method valueOf is overloaded and first method in the overloaded method list uses long[] as method parameter.
I added java.util.BitSet class to whitelist for painless.
Expected: script execute the method so that instance of BitSet is created.

I found the issue #18385 and it should be fixed. I suspect that this fix was added to ES 5.x

Steps to reproduce:

  1. Create a painless script that does "BitSet.valueOf(some_byte_array)"
  2. Run script
  3. Get exception that byte[] cannot be cast to long[]
@clintongormley
Copy link

@jdconrad any thoughts?

@jdconrad
Copy link
Contributor

jdconrad commented Dec 16, 2016

A couple of quick notes:

Edit: Oops! I just realized that BitSet is already part of our whitelist, so I'm editing my comment to make sense.

Overloading has been supported since our initial release of painless in 5.0, but only by arity, not by signature. We decided early on in development to go with arity overloading (meaning methods with the same name must have a different number of arguments) because it turns out it's really difficult to determine method overloading at runtime without at least taking a significant performance hit.

I'll see if I can figure out why we went with valueOf(long[]) instead of valueOf(byte[]). Maybe this is something that should be changed.

@umnick84
Copy link
Author

Thanks for that!
Meanwhile I prepared workaround with custom java plugin implementation. It works fine. However I cannot compare its performance with native painless code. In documentation you said that painless is really fast. Not sure if I can use groovy script for that (as far as I remember it could not access BitSet class at all so I gave up after a while).

@jdconrad
Copy link
Contributor

Groovy has been deprecated in 5.0 and is removed in 6.0, so it's not really an option anymore just in case you were still looking into that.

On a different note, after a bit of investigation, we went with long[] for performance reasons, but this is place where having byte[] also would be good, so I'll evaluate what our options are here to see if we can give you what you need.

@s1monw s1monw added >bug and removed discuss labels Jan 6, 2017
@nezda
Copy link

nezda commented Feb 4, 2017

I got tripped up by Integer.valueOf(String) not working (seems only the int form works so I used Integer.parsInt(String)) - was trying to support input from either ints or string sorta like JavaScript. Guess I can Integer.valueOf(arg.toString())...

@jdconrad
Copy link
Contributor

jdconrad commented Feb 7, 2017

@nezda -- Here is the published Painless API. (https://www.elastic.co/guide/en/elasticsearch/reference/master/painless-api-reference.html)
Please see the reasoning below for why some expected methods may not be defined.

@s1monw I'm not sure I consider this a bug since we claim to support arity overloading, it was a design decision made so method look up (especially for runtime performance) would be fast. Otherwise, we'd have to write our own algorithm for figuring out which method is closest to the passed in arguments which is both very difficult and bad for performance. This means we did have to make decisions around what methods were available that had the same arity within a separate class. I think an appropriate solution to this might be to alias some of the methods that share a name and arity to simply have a different name.

@jdconrad
Copy link
Contributor

jdconrad commented Nov 29, 2017

This is not a bug. Painless uses arity overloading, but it's very unlikely we will ever support full signature overloading. Closing.

@clintongormley clintongormley added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache and removed :Plugin Lang Painless labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache
Projects
None yet
Development

No branches or pull requests

5 participants