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

Should we allow Painless scripts to be longer? #23209

Closed
nik9000 opened this issue Feb 16, 2017 · 11 comments
Closed

Should we allow Painless scripts to be longer? #23209

nik9000 opened this issue Feb 16, 2017 · 11 comments
Assignees
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache

Comments

@nik9000
Copy link
Member

nik9000 commented Feb 16, 2017

Right now we hard code the limit on Painless scripts. I think the limit we set was fairly arbitrary. I've had folks ask about bumping the limit. Maybe we should bump the limit ourselves or make it a part of the script script context when we have a context?

@rjernst
Copy link
Member

rjernst commented Feb 16, 2017

I think it should be part of the context.

@clintongormley
Copy link

The context is hard coded, no? In that case, which contexts would have an increased size limit?

@clintongormley
Copy link

Discussed in FixItFriday - contexts may make sense, eg search scripts should be small and fast, while update scripts might allow for more complexity.

@uschindler
Copy link
Contributor

This is no rwal limitation, it's only for display purposes. You misunderstood that! Scripts can be longer, of course. It is just that the stack trace would not contain the full painless script as source code where normal classes have the file name. This is just a limitation of the class file format, not arbitrary.

@uschindler
Copy link
Contributor

Oh sorry, ignore that. It's not the limit in the class file generator.

@conquel
Copy link

conquel commented Jul 4, 2017

@cbuescher #25527 I have nearly 150 variables which must be passed to the script which handles calculations between these variables and the same number of fields in a document, then documents are scored by the resulting value.
I don't think it is possible to efficiently encourage making faster scripts by limiting their maximum length. One can have long variable names which exhaust the allowed size, but virtually no math. Or you can have a 100 bytes script with a heavy loop. So, I propose to leave performance considerations to the developer.
Thus, @nik9000, it's a great idea to have this limit lifted in the next release, please do so. Otherwise we would have to make a custom build just to change that number.

@morphers82
Copy link

Any possibility of allowing larger painless scripts in 6.0?

@clintongormley clintongormley added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache and removed :Plugin Lang Painless labels Feb 14, 2018
@nerophon
Copy link

nerophon commented Feb 23, 2018

Another user has requested that this hardcoded limit be increased or removed. It is preventing them fulfilling their requirements.

@jdconrad
Copy link
Contributor

Contexts are done so probably worth adding this as a setting at some point.

@jasontedor
Copy link
Member

Closed by #35184

@ypid-geberit
Copy link

ypid-geberit commented Jan 17, 2019

Thanks for fixing!

I found another issue with the size limit which is that source code comments also count to the total script length. Because my scripts are usually well commented, I now hit this limit so I needed to implement a way to minify Painless scripts before sending them to ES as workaround. Refer to --minify-scripts elastic/examples#239

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
Projects
None yet
Development

No branches or pull requests