Query refactoring: MatchQueryBuilder #13402
Conversation
There was a problem hiding this comment.
can we throw an IAE if either is null?
|
this looks great, I put some general comments where I thought it makes sense to be more strict and have less null vaules to signal not set. |
There was a problem hiding this comment.
nit: default constants could be made public?
|
I reviewed this too, looks good, only thing is as Simon said I would move to primitive types whenever possible on the builder, remove null checks and set proper defaults. |
cbuescher
force-pushed
the
feature/query-refactoring-match
branch
2 times, most recently
from
0cbfaaa to
ea5d9b4
Compare
There was a problem hiding this comment.
in other queries we have Fuzziness.AUTO as default, doesn't it apply here too?
There was a problem hiding this comment.
But can you use this if you don't want any fuzzy query? Will check.
There was a problem hiding this comment.
I think AUTO is different from having no fuzziness defined, so I guess this doesn't work.
There was a problem hiding this comment.
weird, I thought this was very similar to query_string where we do have AUTO as default, but eventually it isn't, sorry.
|
left couple of tiny comments, LGTM though |
cbuescher
force-pushed
the
feature/query-refactoring-match
branch
from
e055416 to
4574ee3
Compare
This add equals, hashcode, read/write methods, separates toQuery and JSON parsing and adds tests. Also moving MatchQueryBuilder.Type to MatchQuery to MatchQuery, adding serialization and hashcode, equals there. Relates to elastic#10217
cbuescher
force-pushed
the
feature/query-refactoring-match
branch
from
4574ee3 to
90fac17
Compare
clintongormley
added
:Search/Search
4 participants
This PR separates toQuery and JSON parsing for MatchQueryBuilder and adds equals, hashcode, read/write methods. Also moving MatchQueryBuilder.Type to MatchQuery to MatchQuery, adding serialization and hashcode, equals there.
Relates to #10217