Can't remove integer element from array with Painless

[ericofusco]

Elasticsearch version: 5.0.0

Plugins installed: []

JVM version: Oracle JRE Server 8u112

OS version: Ubuntu 14.04

Description of the problem including expected versus actual behavior:

Steps to reproduce:

1. Create a document with array of integer values
2. Use .remove() with Integer object

I understand that ElasticSearch uses ArrayList() for arrays, when remove() is used with an integer it removes an element from index position, so it needs to be converted to an Integer object to remove the value, not the position (example: "inline": "ctx._source.field.remove(new Integer(params.value))",) but I get the error:

Unknown new call on type [Integer].

With groovy was easy to do: ctx._source.field -= params.value, now this shortcut doesn't even work with painless.

[clintongormley]

Any ideas @jdconrad ?

[jdconrad]

I'll need to think about the best solution to this. For now a work around looks like ctx._source.field.remove(ctx._source.field.indexOf(params.value))

You can see all of our whitelisted methods here: https://github.com/elastic/elasticsearch/tree/master/modules/lang-painless/src/main/resources/org/elasticsearch/painless

The reason list.remove(def) method was not added is because Painless only supports arity overloading for methods meaning same name, different number of arguments. We considered list.remove(int) to be the more common use case here. This was a deemed a reasonable solution given the added complexity and performance hit at runtime for overloaded method look up.

As for not being allowed to make a new Integer call, Painless attempts to hide boxing from the user if at all possible. If you really need an Integer you can use Integer.valueOf(int), but it should be quite rare if ever necessary.

Lastly, we can add shortcuts, but += and -= are already overloaded for so many different use cases it would be very difficult to add them as those. Maybe something like ++= and --=, but anything that doesn't conflict with existing operators is fine with me.

[ericofusco]

@jdconrad,

Thank you. I went with script below.

ctx._source.field.removeAll(Collections.singleton(params.value))

On my case I want to make sure no elements with same value are left in the array. That's how it was working on groovy with -= params.value. indexOf() only gets the first match.

Should I expect this script using removeAll() and Collections to break on any near ES updates ?

Just a FYI, I found another issue that's related to shortcuts with lists: #21027

[jdconrad]

Got it. removeAll() and Collections should be here to stay, so I would say this solution is a good one. And yes, you are definitely not alone in wanting some shortcuts here!

[nik9000]

I wonder if we can get -= working with lists like groovy has....

[jdconrad]

@nik9000 The answer is no. It would require a significant design change to dynamic typing. The only reasonable way to get this working is to not use an overloaded operator so the List type can inferred.

[nik9000]

@nik9000 The answer is no. It would require a significant design change to dynamic typing. The only reasonable way to get this working is to not use an overloaded operator so the List type can inferred.

Awe well, it could certainly be worse.

[jdconrad]

I'm all for adding this with a non-overloaded operator. I know there will be complaints, but at least it would be available. Maybe something as simple as ++= and --=.

[nik9000]

What about a method on list?

[rjernst]

Maybe something as simple as ++= and --=.

Please let's not add more crazy syntax, let's stick with known syntax that already exists in groovy/java.

I don't see a problem with the workaround proposed: removing an object requires finding the index of that object internally anyways. This just makes it more clear to the user that this is a linear operation.

[jdconrad]

@rjernst Not changing anything here other than docs is fine with me too. It would just be very difficult to implement the += and -= for this since dynamic types doing binary operations expect input, input --> output. There is no effective way to handle this change to pop the value off the stack right now to handle methods. I imagine it would be an ugly hack to get this to work.

[rjernst]

I just don't think we need syntax for this. The workaround is straightforward. We don't need special syntax for every little thing.

[clintongormley]

Agree with @rjernst - let's just document the solution and close

[edalford11]

Is this documented anywhere yet? I'm having trouble finding good documentation on working with painless and having to rely on these issue boards to migrate off of groovy. The current painless docs on the elastic website are pretty limited.

[clintongormley]

@edalford11 @jdconrad is hard at work on writing better docs. it takes time, but we'll get there

[bra-fsn]

It would be so good to have a usable documentation AND REPL somewhere. Writing painless scripts is like pain in the ass now. :(

[elnur]

Damn, Painless is such a pain, indeed.

[jdconrad]

We aren't going to change the behavior here. Documentation work is in progress. Operators section has been added. Closing.

[reswob10]

Similar conversation here: #1583

[hacksdump]

Thanks @jdconrad for
ctx._source.field.remove(ctx._source.field.indexOf(params.value))
I couldn't find a single perfect solution on the internet to remove a string from array.
This one worked like charm for my string type fields.