Query refactoring: function_score #13653
Conversation
javanna
force-pushed
the
refactor/function_score
branch
from
90cba95 to
8404a24
Compare
There was a problem hiding this comment.
Would this read better if we used Objects.equals() like we do in other queries?
There was a problem hiding this comment.
yea it would read better, I only added the missing curly brackets while debugging failures till now, can change that too.
|
@javanna I left some comments, but looks really good |
|
@colings86 pushed new commits, added coverage and tests for enums (including operator that was missing tests, my bad) |
There was a problem hiding this comment.
we need a consolidation of all the score mode / type we have at some point, not here though
|
I pushed new commits that address the feedback, ready for another round of review. |
javanna
force-pushed
the
refactor/function_score
branch
2 times, most recently
from
ad31082 to
4c26e7a
Compare
|
@colings86 @s1monw I went ahead and refactored all the functions too. Apologies for the size of the PR, should be close though. |
There was a problem hiding this comment.
Do we not also need a NAME constant here which is the name of this function in the NamedWriteableRegistry? Same with the other Functions
There was a problem hiding this comment.
getWriteableName is implemented once in the base ScoreFunctionBuilder and returns getName which was already there and implemented for all the functions.
|
LGTM |
Refactor the function_score query so it can be parsed on the coordinating node, split parse into fromXContent and toQuery, make FunctionScoreQueryBuilder Writeable. Closes elastic#13653
javanna
force-pushed
the
refactor/function_score
branch
from
4c26e7a to
7eedd84
Compare
clintongormley
added
:Search/Search
4 participants
Refactor the function_score query so it can be parsed on the coordinating node, split parse into fromXContent and toQuery, make the builder writeable.
Found an issue with script_score function in tests, given that it accesses the shardId through SearchContext, but that's something that we don't have available and I am not sure how to make available. Added a TODO there, left out script_score from testing for now.
Simplified code around enums (boost mode and score mode), we now rely on enums for serialization and naming.
This change is breaking for the java api as it modifies FunctionScoreQueryBuilder, removes all the add method from it. Also breaking for plugins that implement custom score functions, see the migrate guide for more info.