Refactors TemplateQueryBuilder and Parser #13253
Conversation
|
|
||
| private ScriptService.ScriptType templateType; |
There was a problem hiding this comment.
+1 on removing these as essentially they are encapsulated in the Template above.
|
Left some minor comments. Looks pretty good to me already. |
|
@MaineC thanks for the review. I have addressed the comments. |
| @@ -35,14 +42,8 @@ | |||
|
|
|||
| /** Template to fill. */ | |||
| private Template template; | |||
There was a problem hiding this comment.
template could be made final I think
|
left a few comments, looks good though! |
alexksikes
force-pushed
the
feature/query-refactoring-template
branch
from
db005cf to
9461fb1
Compare
| if (!templateBase.equals(EmptyQueryBuilder.PROTOTYPE)) { | ||
| assertEquals(templateBase.toQuery(createShardContext()).getClass().getName(), query.getClass().getName()); | ||
| } | ||
| } |
There was a problem hiding this comment.
I checked out the branch, testToQuery fails consistently. You have to override setFinalBoost in TemplateQueryBuilder and make it a no-op like we did in WrapperQueryBuilder. Also this conditional in doAssertLuceneQuery shouldn't be needed anymore. Replace with a simple equality check between query and the result ot templateBase#toQuery. The problems you previously encountered around boost should be solved now.
|
I did a final round of review, change looks good, there is a test problem though. LGTM once tests are fixed. |
Relates to elastic#10217 This PR is against the query-refactoring branch. Closes elastic#13253
alexksikes
force-pushed
the
feature/query-refactoring-template
branch
from
9461fb1 to
c2ccb21
Compare
clintongormley
added
:Search/Search
Relates to #10217
This PR is against the query-refactoring branch.