Query refactoring: NotQueryBuilder and Parser #11823
Conversation
|
|
||
| public NotQueryBuilder(QueryBuilder filter) { | ||
| this.filter = Objects.requireNonNull(filter); |
There was a problem hiding this comment.
shouldn't this convert to an if in validate?
There was a problem hiding this comment.
I don't think so, unfortunately parseInnerFilter/Builder() can legally return null and also does this when the inner query e.g. is { "constant_score" : filter {} }. This is currently legal rest query which parses to null in perseInnerXxx(), this is then passed up by enclosing filters/queries until it gets ignored somewhere.
The former existing null-check was okay for the existing builder-pattern, but now that we use it for intermediate query representation as well, I think this needs to be allowed.
There was a problem hiding this comment.
makes sense to me, thanks for the explanation, I see what you mean
|
left a small comment, LGTM otherwise |
Moving the query building functionality from the parser to the builders new toQuery() method analogous to other recent query refactorings. Relates to elastic#10217 Closes elastic#11823
cbuescher
force-pushed
the
feature/query-refactoring-not
branch
from
2609956 to
682b499
Compare
Query refactoring: NotQueryBuilder and Parser
Moving the query building functionality from the parser to the builders new toQuery() method analogous to other recent query refactorings. Relates to elastic#10217 Closes elastic#11823
…ring-not Query refactoring: NotQueryBuilder and Parser
clintongormley
added
:Search/Search
Moving the query building functionality from the parser to the builders
new toQuery() method analogous to other recent query refactorings.
Relates to #10217
Closes #11823