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

Move score script context from SearchScript to its own class #30816

Merged
merged 6 commits into from
May 25, 2018

Conversation

martijnvg
Copy link
Member

No description provided.

@martijnvg martijnvg added review :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v7.0.0 v6.4.0 labels May 23, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fantastic! I just have one comment.

final CannedScorer scorer = new CannedScorer();
leafScript.setScorer(scorer);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to keep the scorer. Having the script access score through get_score() (which maps to _score) allows score to not be computed when it is not used. needs_score only gets us so far (whether it is used at all). With get_score, we could (although I'm not sure it is actually implemented this way yet) call the method lazily to bind _score.

@martijnvg
Copy link
Member Author

Thanks for reviewing! I've updated the pr and brought back setScorer() and added get_score() which is backed by a supplier that lazily computes the score if needed.

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once the CI issue is addressed (need to add new ScoreScript to MockScriptEngine)

@martijnvg martijnvg changed the title Move score script context from SearchContext to its own class Move score script context from SearchScript to its own class May 24, 2018
@martijnvg martijnvg merged commit ae2f021 into elastic:master May 25, 2018
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request May 25, 2018
martijnvg added a commit that referenced this pull request May 25, 2018
* es/master:
  Move score script context from SearchScript to its own class (#30816)
  Fix bad version check writing Repository nodes (#30846)
  [docs] explainer for java packaging tests (#30825)
  Remove Throwable usage from transport modules (#30845)
  REST high-level client: add put ingest pipeline API (#30793)
  Update the version checks around ip_range bucket keys, now that the change was backported.
  Mute IndexMasterFailoverIT.testMasterFailoverDuringIndexingWithMappingChanges
  Use geohash cell instead of just a corner in geo_bounding_box (#30698)
  Limit user to single concurrent auth per realm (#30794)
  [Tests] Move templated _rank_eval tests (#30679)
  Security: fix dynamic mapping updates with aliases (#30787)
  Ensure that ip_range aggregations always return bucket keys. (#30701)
  Use remote client in TransportFieldCapsAction (#30838)
  Move Watcher versioning setting to meta field (#30832)
  [Docs] Explain incomplete dates in range queries (#30689)
  Move persistent task registrations to core (#30755)
  Decouple ClusterStateTaskListener & ClusterApplier (#30809)
  Send client headers from TransportClient (#30803)
  Packaging: Ensure upgrade_is_oss flag file is always deleted (#30732)
  Force stable file modes for built packages (#30823)
martijnvg added a commit that referenced this pull request May 25, 2018
* es/6.x:
  Move score script context from SearchScript to its own class (#30816)
  Fix bad version check writing Repository nodes (#30846)
  Modify state of VerifyRepositoryResponse for bwc (#30762)
  QA: Fix tribe tests when running default zip
  Use remote client in TransportFieldCapsAction (#30838)
  Mute IndexMasterFailoverIT.testMasterFailoverDuringIndexingWithMappingChanges
  Ensure that ip_range aggregations always return bucket keys. (#30701)
  Limit user to single concurrent auth per realm (#30794)
  Security: fix dynamic mapping updates with aliases (#30787)
  [Tests] Move templated _rank_eval tests (#30679)
  Move Watcher versioning setting to meta field (#30832)
  Restore "Add more yaml tests for get alias API " (#30814)
  Send client headers from TransportClient (#30803)
  [Docs] Explain incomplete dates in range queries (#30689)
  Move persistent task registrations to core (#30755)
  Decouple ClusterStateTaskListener & ClusterApplier (#30809)
  Packaging: Ensure upgrade_is_oss flag file is always deleted (#30732)
  Force stable file modes for built packages (#30823)
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 >non-issue v6.4.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants