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

function_score: undo "Remove explanation of query score from functions" #9826

Closed
wants to merge 3 commits into from

Conversation

brwe
Copy link
Contributor

@brwe brwe commented Feb 23, 2015

This adds the Explanation to the explainScore() again. It is needed
because the explanation of script functions will otherwise not contain
an explanation of _score if boost mode is set to replace.

This adds the Explanation to the explain score again. It is needed
because the explanation of script functions will otherwise not contain
an explanation of _score if boost mode is set to replace.
return new Explanation((float) (runAsDouble()), "This script returned " + runAsDouble());
public Explanation explain(Explanation subQueryScore) throws IOException {
Explanation exp = new Explanation((float) (runAsDouble()), "This script returned " + runAsDouble());
Explanation _scoreExp = new Explanation(subQueryScore.getValue(), "_score: ");
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove the leading underscore? Variables should start with a lowercase letter according to http://www.oracle.com/technetwork/java/codeconventions-135099.html

@jpountz
Copy link
Contributor

jpountz commented Feb 23, 2015

LGTM

@@ -86,6 +88,7 @@ public void testNativeExplainScript() throws InterruptedException, IOException,
for (SearchHit hit : hits.getHits()) {
assertThat(hit.getId(), equalTo(Integer.toString(idCounter)));
assertThat(hit.explanation().toString(), containsString(Double.toString(idCounter) + " = This script returned " + Double.toString(idCounter)));
assertThat(hit.explanation().toString(), containsString("1.0 = tf(freq=1.0), with freq of"));

Choose a reason for hiding this comment

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

To prevent future changes from removing the nesting, we could add for example
assertThat(hit.explanation().getDetails().length, equalTo(2));
or something even more explicit.
@brwe WDYT?

@brwe
Copy link
Contributor Author

brwe commented Feb 25, 2015

@maxjakob @jpountz thanks for the review! Addressed all comments. Can I push?

@maxjakob
Copy link

👍

@maxjakob
Copy link

Can we expect this to be in version 1.4.5?

@s1monw
Copy link
Contributor

s1monw commented Mar 23, 2015

@brwe LGTM

@s1monw
Copy link
Contributor

s1monw commented Mar 24, 2015

@brwe please merge this into 1.x, 1.5 and master

@s1monw s1monw self-assigned this Mar 24, 2015
brwe added a commit that referenced this pull request Mar 24, 2015
This adds the Explanation to the explain score again. It is needed
because the explanation of script functions will otherwise not contain
an explanation of _score if boost mode is set to replace.

closes #9826
brwe added a commit that referenced this pull request Mar 24, 2015
This adds the Explanation to the explain score again. It is needed
because the explanation of script functions will otherwise not contain
an explanation of _score if boost mode is set to replace.

closes #9826
@brwe brwe closed this in 3add12a Mar 24, 2015
@brwe
Copy link
Contributor Author

brwe commented Mar 24, 2015

@maxjakob sorry we did not get this in 1.5 and promised I'll add labels next time to remind me. It will be in 1.5.1. There will be no further 1.4.X release.

@maxjakob
Copy link

@brwe I see, 1.5.1 then, that's fine. Thanks!

mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
This adds the Explanation to the explain score again. It is needed
because the explanation of script functions will otherwise not contain
an explanation of _score if boost mode is set to replace.

closes elastic#9826
@clintongormley clintongormley added :Search/Search Search-related issues that do not fall into other categories and removed :Query DSL labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Search Search-related issues that do not fall into other categories v1.5.1 v1.6.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants