Refactor SimpleQueryStringBuilder #10405
Conversation
SimpleQueryStringBuilder now includes the `fromXContent`, `doXContent`, and `toQuery` methods as well as implementing the Streamable interface. It includes unit tests for the serialization and parsing. Because of testing, I also implemented `hashCode` and `equals` similar to the way that Lucene queries implement these.
There was a problem hiding this comment.
yea I think this validation should be done on the coordinating node ideally, rather sooner than later. That said we might need to do the same on the data node too (toQuery). Shall we have a validate method or something that can be called from both?
|
looks good, I have some comments that are mainly around testing, same as in #10454. I think we should go for a test base class so we trim down the test code required. Define a bunch of tests that need to be present for each query, try and write them in the base class and define abstract methods that we have to go and extend in each specific query test. How does this sound? |
There was a problem hiding this comment.
out of curiosity, what is this class for? :)
|
Closing this, I will re-create based on the new feature branch since we went a different direction. |
clintongormley
added
:Search/Search
SimpleQueryStringBuilder now includes the
fromXContent,doXContent,and
toQuerymethods as well as implementing the Streamable interface.It includes unit tests for the serialization and parsing.
Because of testing, I also implemented
hashCodeandequalssimilarto the way that Lucene queries implement these.
NOTE: this PR is against the
feature/query-parse-refactoringbranch.