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

Inconsistent usage of ScriptScoreFunction in FiltersFunctionScoreQuery #3464

Closed
brwe opened this issue Aug 8, 2013 · 2 comments
Closed

Inconsistent usage of ScriptScoreFunction in FiltersFunctionScoreQuery #3464

brwe opened this issue Aug 8, 2013 · 2 comments

Comments

@brwe
Copy link
Contributor

brwe commented Aug 8, 2013

Scoring in function_score and filters_function_score queries is potentially inconsistent when using scripts.

Brief overview of related classes

function_score and filters_function_score allow a user to modify the score of a query (referred to as 'subQueryScore' from here on).

In brief, there are two classes that compute scores based on a query and some function: FunctionScoreQuery, which only has one function and FiltersFunctionScoreQuery which combines the result of several functions. For both classes, the function can be a ScriptScoreFunction.

ScoreFunction: Computes a score for a document. The two relevant methods are:

  • score(docId, subQueryScore): computes a new score taking into account the subQueryScore and some other properties of a documents.
  • factor(docId): computes a score solely based on properties of the document.

FunctionScoreQuery.score() computes:

score = subQueryBoost * ScoreFunction.score(docId, subQueryScore)

FiltersFunctionScoreQuery.CustomBoostFactorScorer.score() computes:

score = subQueryScore * subQueryBoost * combine(ScoreFunction1.factor(docId), ScoreFunction2.factor(docId),…)

where combine can mean add, multiply, lowest value, etc.

The problem

ScoreFunctions.factor(docId) implies that the method computes a factor only and does not take into account the subQueryScore. This is the way the ScoreFunctions are used in FiltersFunctionScoreQuery.CustomBoostFactorScorer: The method factor() is called for each score function and the result is than later multiplied to the subQueryScore.

However, the ScriptScoreFunction violates this principle: scripts can use the _score variable which should be initialized with the subQueryScore before the script is run, see ScriptScoreFunction.score(..). If ScriptScoreFunction.factor() is called, then the behavior is undefined, since the _score variable is either wrong or maybe even not initialized.

This might cause unexpected behavior since this inconsistency is not transparent to the user.

@ghost ghost assigned brwe Aug 8, 2013
@brwe
Copy link
Contributor Author

brwe commented Aug 9, 2013

Backported function_score (Issue #3423, commits 720b550..a938bd5, 534299a and 8774c46, 32cdddb) to 0.90.

@s1monw
Copy link
Contributor

s1monw commented Aug 9, 2013

@brwe cool! thanks for recording those commit IDs

brwe added a commit to brwe/elasticsearch that referenced this issue Aug 19, 2013
…Query

This commit fixes inconsistencies in `function_score` and `filters_function_score`
using scripts, see issue elastic#3464

The method 'ScoreFunction.factor(docId)' is removed completely, since the name
suggests that this method actually computes a factor which was not the case.
Multiplying the computed score is now handled by 'FiltersFunctionScoreQuery'
and 'FunctionScoreQuery' and not implicitely performed in
'ScoreFunction.factor(docId, subQueryScore)' as was the case for 'BoostScoreFunction'
and 'DecayScoreFunctions'.

This commit also fixes the explain function for FiltersFunctionScoreQuery. Here,
the influence of the maxBoost was never printed. Furthermore, the queryBoost was
printed as beeing multiplied to the filter score.

Closes elastic#3464
brwe added a commit that referenced this issue Aug 23, 2013
…Query

This commit fixes inconsistencies in `function_score` and `filters_function_score`
using scripts, see issue #3464

The method 'ScoreFunction.factor(docId)' is removed completely, since the name
suggests that this method actually computes a factor which was not the case.
Multiplying the computed score is now handled by 'FiltersFunctionScoreQuery'
and 'FunctionScoreQuery' and not implicitely performed in
'ScoreFunction.factor(docId, subQueryScore)' as was the case for 'BoostScoreFunction'
and 'DecayScoreFunctions'.

This commit also fixes the explain function for FiltersFunctionScoreQuery. Here,
the influence of the maxBoost was never printed. Furthermore, the queryBoost was
printed as beeing multiplied to the filter score.

Closes #3464
@brwe brwe closed this as completed in b007af1 Aug 23, 2013
mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015
…Query

This commit fixes inconsistencies in `function_score` and `filters_function_score`
using scripts, see issue elastic#3464

The method 'ScoreFunction.factor(docId)' is removed completely, since the name
suggests that this method actually computes a factor which was not the case.
Multiplying the computed score is now handled by 'FiltersFunctionScoreQuery'
and 'FunctionScoreQuery' and not implicitely performed in
'ScoreFunction.factor(docId, subQueryScore)' as was the case for 'BoostScoreFunction'
and 'DecayScoreFunctions'.

This commit also fixes the explain function for FiltersFunctionScoreQuery. Here,
the influence of the maxBoost was never printed. Furthermore, the queryBoost was
printed as beeing multiplied to the filter score.

Closes elastic#3464
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants