Skip to content

Implement needsScore() correctly.#18247

Merged
rmuir merged 4 commits intoelastic:masterfrom
rmuir:needsScore
May 10, 2016
Merged

Implement needsScore() correctly.#18247
rmuir merged 4 commits intoelastic:masterfrom
rmuir:needsScore

Conversation

@rmuir
Copy link
Copy Markdown
Contributor

@rmuir rmuir commented May 10, 2016

Scripts have to report whether or not they use this. If they don't need _score then e.g. more things can be cached, for example queries.

Expressions is currently the only scripting language that doesn't always return true for this method. But because we have special handling for it, we should make us of this info and record if its needed.

@rmuir
Copy link
Copy Markdown
Contributor Author

rmuir commented May 10, 2016

I ported over the test expressions is using here. Basically we just apply a marker interface if scores are needed and then we can check with instanceof. I guess alternatively it could be an annotation or something like that, but this seems simple enough?

Separately we should followup with the special score handling. Maybe the scoreAccessor should not just be shoved into the input map like we do, but instead just passed in as a normal parameter (null if not needed or n/a)? This would avoid unnecessary per-document hashing.

@uschindler
Copy link
Copy Markdown
Contributor

Interface is fine, annotation is IMHO too complicated, but can be done without much changes.

@jdconrad
Copy link
Copy Markdown
Contributor

LGTM.

@rmuir rmuir merged commit 1d80542 into elastic:master May 10, 2016
@jdconrad jdconrad mentioned this pull request May 12, 2016
18 tasks
@clintongormley clintongormley changed the title painless: implement needsScore() correctly. Implement needsScore() correctly. May 17, 2016
@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

:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement v5.0.0-alpha3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants