Refactors TemplateQueryBuilder and Parser#13253
Conversation
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. |
There was a problem hiding this comment.
template could be made final I think
|
left a few comments, looks good though! |
db005cf to
9461fb1
Compare
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
9461fb1 to
c2ccb21
Compare
Relates to #10217
This PR is against the query-refactoring branch.